feat(Proptypes): remove from production build

[layershifter]

WIP

This PR is first part for #524.
What will we do there?

Props

First variant

Component._meta.props will be renamed to Component._meta.values while Component._meta.props will contain array of all handled props, (look to Loader.js in PR).

Second variant

We will leave Component._meta.props as it as and add all left props to it:

props: {
  as: true,
  active: true,
  inline: ['centered'],
  size: SUI.SIZES,
}

I think that first variant is sensible, any better suggestions there?

getUnhandledProps

This function will be more simple as you can see.

Tests

I've added test cases for isConformant that checks that Component._meta.props and is equal to union of autoControlledProps, defaultProps, propTypes like it was in getUnhandledProps.

---

Thoughts?
/cc @jcarbo @levithomason

[levithomason]

Here are the thoughts that come to mind:

1. autoControlledProps must remain a static in order for AutoControlledComponent to work.
2. We should list the prop name in one place, rather than props and values. I'd propose the keys of _meta.props as the SSOT.
3. I don't think we should remove propTypes since the user needs them during dev. We can however point users to babel-plugin-remove-proptypes to strip propTypes in their own build.
4. If we do modify _meta.props, we should consider the needs of the Component Explorer. It needs to know at least the prop name, type, and possible values. If we made each value in props an object that described these things, then it would be more useful in the Component Explorer. That said, all this is documentation information, so I'd like to strive to move it all into the doc block instead. We create docgenInfo.json from the doc block, so we could then import any doc block info into the docs. This would keep it out of the source code as well.

Apologies as my thoughts are a little scattered. I haven't reached a clear resolve on this yet so I gave a brain dump for now.

In summary, I think user's should remove propTypes themselves using the babel-plugin mentioned. I think we should consider moving _meta.props into the doc block and parsing that for documentation purposes instead of having it live in the source code.

[layershifter]

autoControlledProps must remain a static in order for AutoControlledComponent to work.

Yes, defaultProps must retain too.

We can however point users to babel-plugin-remove-proptypes to strip propTypes in their own build.

It sounds reasonable.

I think we should consider moving _meta.props into the doc block and parsing that for documentation purposes instead of having it live in the source code.

This idea will not solve problem with propTypes:

• we will still need array of handled props for getUnhandledProps;
• we will still need list of possible values for propTypes.

So, I don't see there any way how it can moved in doc block.

/**
 * A loader is magic.
 * 
 * @property as type.as As i magic 
 * @property active bool Marks loader active
 * @property size array['small', 'medium', 'large', 'huge'] Defines loader size
 */
function Loader () {}

docgen will parse and create props (['as', 'active', 'size']) for each component that will be used for getUnhandledProps, but it looks like black magic.

---

If we will keep props as SSOT, it will looks strange:

props: {
  as: true,
  active: true,
  inline: ['centered'],
  size: SUI.SIZES,
}

We can move there own constants for Component Explorer, but I think it's beyond good and evil, it will duplicate functionality of propTypes.

props: {
  as: META.PROPS.BOOL,
  active: META.PROPS.BOOL,
  inline: ['centered'],
  size: SUI.SIZES,
}

props: {
  as: { type: META.PROPS.BOOL },
  active: { type: META.PROPS.BOOL },
  inline: {
    type: META.PROPS.ARRAY,
    values: ['centered'],
  },
  size: {
    type: META.PROPS.ARRAY,
    values: SUI.SIZES,
  },
}

[levithomason]

I had a lengthy reply with code examples written out, however, after reviewing it I'm thinking this might not be worth the effort. In the end, we must have an array of handled props. If we want to keep builds small, that array needs to be separate from the propTypes definition. This means we'll have to manage two lists. The gain is also minimal but it adds lots of overhead and potential for bugs. So, I'm hesitant to merge anything that optimizes propTypes, for now.

I think I'd rather focus on publishing a babel plugin to transform import statements:

import { Button, Card } from 'semantic-ui-react'

Becomes:

import Button from 'semantic-ui-react/Button'
import Card from 'semantic-ui-react/Card'

This would require a PR to refactor how we publish so that the components are published as a flat directory. This avoids rewriting the imports to:

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

[jeffcarbs]

Agreed with that last comment. I don't think the benefits of removing propTypes is worth all the added complexity/brittleness. I think we'd find ourselves reimplementing the propTypes functionality that's already built into React.

[layershifter]

Seems, I'm in the minority 😄

Anyway, I'm still thinking that storing array of handled props and removing propTypes from user's production build is good idea. It's easy testable and doesn't add any magic except small overhead with duplicating propTypes keys.

Benefits aren't so great, but it's really beastly to see (Icon, Flag) this in production build. Yep, gzip will compress all, but...

[levithomason]

If we kept the focus on the props only and forgo my previous considerations (Component Explorer, doc blocks, etc.) then we could simply do this to remove propTypes in production:

function Flag() {...}

if (process.env.NODE_ENV !== 'production' {
  Flag.propTypes = {}
}

The downside is that we'd then have to move all current _meta.props values into the actual propTypes as well. Otherwise, the _meta.props object would still be massive for icon names and flag names.

This would also require common test updates. There are tests that assert that _meta.props includes enums for value only props which would need to be removed / updated. We'd then need to add a common test that ensured the propType keys matched the hard list of prop names that getUnhandledProps would use (probably _.map(_meta.props, 'key') or similar).

These are doable and not difficult, however, it means refactoring every component. We can and probably should do this at some point. I just don't personally have the bandwidth to embark on this at the moment.