Semantic-ui-react: RFC: remove production propTypes, allow small custom builds

[layershifter]

I've builded stardust with webpack in my project.

There are two problems:

• [x] unused components in bundle that increases bundle size;
• [x] PropTypes are in production bundle, too.

bundlers TODO

• [x] #1452 test that webpack 1 removes propTypes from production build, also check SUI.js usage
• [ ] #1497 test that webpack 2 treeshaking works
• [ ] test that rollup works

propTypes update

• [x] #1216 Button
• [x] #1159 Container
• [x] #1159 Divider
• [x] #1155 Flag
• [x] #1159 Header
• [x] #1159 Icon
• [x] #1155 Image
• [x] #1167 Input
• [x] #1265 Label
• [x] #1270 List
• [x] #1165 Loader
• [x] #731 Rail
• [x] #1165 Reveal
• [x] #1165 Segment
• [x] #1203 Step
• [x] #1209 Breadcrumb
• [x] #1320 Form
• [x] #1231 Grid
• [x] #1264 Menu
• [x] #1254 Message
• [x] #1200 Table
• [x] #1284 Card
• [x] #1293 Comment
• [x] #1285 Feed
• [x] #1294 Item
• [x] #1232 Statistic
• [x] Accordion
• [x] #1155 Checkbox
• [x] Dimmer
• [x] #1334 Dropdown
• [x] #1217 Embed
• [x] #1331 Modal
• [x] #1322 Popup
• [x] #1269 Progress
• [x] #1253 Rating
• [x] #1150 Search
• [x] #1296 Sidebar
• [x] #1282 Confirm
• [x] #1300 Portal
• [x] #1155 Radio
• [x] Select
• [x] Text Area

[levithomason]

Agreed on both concerns. Removing propTypes in production and allowing smaller builds. Here are my thoughts.

propTypes

There are transforms to remove propTypes in production builds, however, we're currently using them in getUnhandledProps to determine which props the component does not support. Removing them will break things. We'll need to remove library use of propTypes.

Unused Components

In the optimization phase (after getting the bulk of work completed) I plan to publish individual components. This way users can just install what they need.

We should also consider structuring our primary published project to users can do something similar to:

import Button from 'stardust/Button'

If we did this, we could also provide a babel plugin like lodash has done with babel-plugin-lodash. It looks at usages of lodash and transforms the imports to only import what you use, drastically reducing build size. We use it on the doc site.

[layershifter]

propTypes

There are no change that we can remove getUnhandledProps at now and near future, so we need use it :yum: I don't read about Facebook's plans about propTypes, but I think there are no chance for their complete removal.

I'm thinking about Babel plugin that will run following transform:

Component.propTypes = {
  className: PropTypes.string,
  content: PropTypes.string,
  header: customPropTypes.every(),
  icon: customPropTypes.every()
}

Component.handledProps = ['className', 'content', 'header', 'icon']

So, we can modify getUnhandledProps to

if(process.env.NODE_ENV === 'production') {
 const handledProps = Component.handledProps
} else {
 const handledProps = _.union(
    Component.autoControlledProps,
    _.keys(Component.defaultProps),
    _.keys(Component.propTypes),
  )
}

That might be splited on build to:

const handledProps = Component.handledProps

Thoughts?

[levithomason]

If we added this, I think we should use https://nkt.github.io/babel-plugin-react-remove-prop-types. This package strips them entirely. In that case, I think we should update getUnhandledProps to be more functional, and not rely on external references. It should take in an array of 'handledProps'.

I know it would be more code to write, however, the current function is weak in that it has external dependencies and makes assumptions about those dependencies. Currently, it takes in a Component and checks for 3 different props definitions. That would be replaced by a single hard coded array.

This would make it faster, more reliable/reusable, and we could use the community leader for removing propTypes.

Thoughts?

[layershifter]

If we will use handcoded handledProps, it will be easy implement. We can make it after migration to semantic-react, because it looks absurdly in production build.

[levithomason]

We also need to remove the index files from each component "type" folder. Otherwise, we'll automatically bundle extra components. See https://github.com/TechnologyAdvice/stardust/pull/570#discussion_r81445116.

Lastly, the common tests for exported components needs updated. The failure message instructs users to export their components in those index files. Once we remove them, that message is no longer valid. It should instead instruct users to export their components in the main index file.

[hankthewhale]

Do you guys see anything wrong with doing this in a lib/stardust file in our project?

export { default as Button } from 'stardust/dist/commonjs/elements/Button/Button';

Then when required, we can just do

import { Button } from 'lib/stardust';

This works for our needs but I wasn't sure if there were any issues we might encounter doing this (other than breaking changes as you guys make updates)?

[layershifter]

PropTypes

As we decided in #684:

• we need separate enums from Component _meta;
• _meta.props is array of handled props by Component.

I've sketched some code there, if we approve it I'll open new PR for this.

Stateless component

function SUIComponent () {}

SUIComponent._meta = {
  // ...
  props: [
    'as',
    'children',
    'size',
    // etc.
  ]
}

if (process.env.NODE_ENV !== 'production') {
  SUIComponent.propValues = {
    size: SUI.SIZES,
  }

  SUIComponent.propTypes = {
    as: customProptypes.as,
    size: PropTypes.oneOf(SUIComponent.propValues.size),
  }
}

propValues - I really don't like this idea, but it's simpliest way to get values for commentTest.

[levithomason]

To leave room for future expansion of our prop definition (component explorer), we can use an object for each prop. I've also renamed it from propValues to props:

function SUIComponent() {}

SUIComponent._meta = {
  props: ['as', 'children', 'size']
}

if (process.env.NODE_ENV !== 'production') {
  SUIComponent.props = {
    size: {
      values: SUI.SIZES,
    },
  }

  SUIComponent.propTypes = {
    as: customProptypes.as,
    size: PropTypes.oneOf(SUIComponent.props.size.values),
  }
}

It is worth noting that we'll need to pull propTypes out of classes so we can wrap them in the production check as well:

class SUIComponent extends Component {
  _meta = {
    props: ['as', 'children', 'size']
  }
}

if (process.env.NODE_ENV !== 'production') {
  SUIComponent.props = {
    size: {
      values: SUI.SIZES,
    },
  }

  SUIComponent.propTypes = {
    as: customProptypes.as,
    size: PropTypes.oneOf(SUIComponent.props.size.values),
  }
}

Lastly, for the first PR can we update the tests, utilities, contributing guide, and one component only? This way it is a smaller PR that we can review quickly.

[layershifter]

Okay, #731 was merged, wow :v: However, we need to do cleanup work to make it work properly. You can follow #1155 as example.

[layershifter]

For information: #1210 was merged, so ES6 modules for Webpack 2 and Rollup are now supported.

[levithomason]

I've added good first contribution here so folks can assist with the propTypes task list above. They can reference any of the linked PRs in the already completed items.

[layershifter]

Okay, all components where updated 🖖

---

Also, @levithomason I think that we need to add page called Optimization to docs where will be descibed hot to optimize bundle size:

• remove propTypes in production build;
• how use babel-plugin-lodash for commonjs modules;
• notes about es modules support.

[levithomason]

Regarding the docs page, I completely agree. I'd like to have some docs on basic setup as well. This has been discussed in another issue that I cannot find ATM. Starting build/optimize docs anywhere we can for now is good by me. We can iterate afterward.

[layershifter]

I've played with rollup, treeshaking doesn't work there.
I'll try webpack 2 today.

[layershifter]

Same problem with webpack :confused:

[levithomason]

Depends on how they are being imported as well. Could you open a PR and add your setup to the repo for review? I'd like to start some minimal project setup examples in our repo anyhow. Let's put them in /examples in the root of the repo. This way we can link to them from the docs, too.

[pke]

Appreciate the efforts to reduce build sizes.
How about removing the full lodash imports if all u need from the lib is _.isNil()? This also would dramatically reduce build times&sizes.

[jeffcarbs]

@pke - See this comment: https://github.com/Semantic-Org/Semantic-UI-React/issues/830#issuecomment-259785072

We have this done automatically by babel using https://github.com/lodash/babel-plugin-lodash. Compare the src and dist directories to see it in action.

[pke]

@jcarbo Ah I see. The bundle is still a heavy 500kb. I am afraid to use it in my app which is already over 1MB. And with webpack 1 there is no tree shaking.

[jeffcarbs]

@pke - Despite the name, you can actually use the babel-plugin-lodash package to cherry-pick _any_ library. See this comment (and subsequent discussion) where I explained how to do it with SUI-React: https://github.com/Semantic-Org/Semantic-UI-React/pull/731#issuecomment-258729112

It will essentially transform:

// from
import { Button } from 'semantic-ui-react'

// to
import Button from 'semantic-ui-react/dist/commonjs/elements/Button'

Bundle size is definitely something we're trying to solve with issues/PRs like these.

[levithomason]

@pke

The bundle is still a heavy 500kb.

The full library in UMD format is only 81.8 kB minified and gzipped. You can see bundle sizes for the full library, as well as every component individually, in #1233.

You should have slightly smaller builds assuming you aren't building to UMD and likely do not need to include the UMD checks for all environments.

How about removing the full lodash imports if all u need from the lib is _.isNil()?

To clarify, the full list of lodash methods currently used is quite large. It also spans lodash and lodash/fp. I'd liket to eventually migrate everything to lodash/fp, though, for now, these are the methods in use:

_.assign
_.camelCase
_.capitalize
_.castArray
_.clamp
_.compact
_.curry
_.debounce
_.difference
_.dropRight
_.each
_.endsWith
_.eq
_.escapeRegExp
_.every
_.filter
_.find
_.findIndex
_.flatMap
_.flow
_.get
_.has
_.head
_.inRange
_.includes
_.indexOf
_.intersection
_.invoke
_.isArray
_.isEmpty
_.isEqual
_.isFunction
_.isNil
_.isNumber
_.isObject
_.isPlainObject
_.isString
_.isUndefined
_.kebabCase
_.keys
_.last
_.map
_.mapValues
_.max
_.min
_.noop
_.omit
_.partialRight
_.pick
_.random
_.range
_.reduce
_.round
_.sample
_.snakeCase
_.some
_.sortBy
_.startCase
_.startsWith
_.sum
_.tail
_.take
_.times
_.transform
_.trim
_.union
_.uniq
_.values
_.without

[pke]

Wow, thanks for the the thorough responses, guys.
Maybe I'll give it another try. Gzipped sizes don't matter for Cordova apps though. I am not worried about data that goes over the wire, since the script bundle is already on the mobile device. However I want to load and parse it as fast as possible. And for that an additional gzip deflate increases load time.

But minified might help to decrease load time and parsing.

The additional transformation @jcarbo mentioned might also help. I just wish that we would be able to import Button from 'semantic-ui-react/lib/elements/Button right in my ES6 code.

[levithomason]

I just wish that we would be able to import Button from 'semantic-ui-react/lib/elements/Button right in my ES6 code.

You certainly can if you compile it yourself, just pull it form our src 😄

import Button from 'semantic-ui-react/src/elements/Button'

Or, you can use the precompiled commonjs module:

import Button from 'semantic-ui-react/dist/commonjs/elements/Button'

Or, you can use the precompiled ES build, which preserves import statements for tree shaking:

import Button from 'semantic-ui-react/dist/es/elements/Button'

[levithomason]

@layershifter is this issue actually complete now? The last check list item is:

• [ ] unused components (I'm realy don't use Rail in app) in bundle that increases bundle size;

However, we offer several ways to do this now with babel-plugin-lodash, tree shaking with the ES build, and direct component imports (as above).

Let me know if you had any other needs or ideas in mind for this.

[layershifter]

No, I want to check this TODO (#524 (comment)) before we finally close this.

[levithomason]

Sounds good, although, it is not clear to me what other ways of removing unused components we could offer, short of #1443.

[henri-hulski]

Tree-shaking with webpack2 doesn't work for me.
Do I need babel-plugin-lodash also for webpack2?

[levithomason]

You do not. Are you importing from the ES build target?

import { Button} from 'semantic-ui-react/dist/es'

Tree shaking can only work with ES import statements, the default import location provides commonjs modules.

[henri-hulski]

No from semantic-ui-react.
But I changed it now and nothing changed.

[henri-hulski]

I check it again, when I import from semantic-ui-react it automatically imports from semantic-ui-react/dist/es.
This is how it looks:

[levithomason]

We're building through a webpack 2 and tree example. Let's address any issues there, #1497.

[layershifter]

We have separate issues for webpack 2 and rollup, so I'm closing this issue for housekeeping because technically we reached its goal.