Enhancement for best practice prop types. #70
Conversation
… size and allow users that import this package to include prop types in development and optionally remove them using Webpacks UglifyJS + Define plugin.
|
@jquense Let me know your thoughts. If we can get this merged and a future build cut relatively soon that'd be great. Thanks! |
|
Ping @jquense |
src/CSSTransitionGroup.js
Outdated
| @@ -53,7 +37,21 @@ class CSSTransitionGroup extends React.Component { | |||
| } | |||
| } | |||
|
|
|||
| CSSTransitionGroup.propTypes = propTypes; | |||
| CSSTransitionGroup.defaultProps = defaultProps; | |||
| CSSTransitionGroup.propTypes = { | |||
There was a problem hiding this comment.
I don't really like the propTypes defined down here, is the plugin really not smart enough to extract them as they were?
There was a problem hiding this comment.
The plugin traverses the AST using the default parser... I think it's Babylon under the hood. It wraps the direct node and can't reverse engineer because it would require to find all references I believe which is very complicated.
There was a problem hiding this comment.
its not that complicated, i've written that babel plugin before.
There was a problem hiding this comment.
I could investigate to see whether uglify will find the dead code and resulting file size is same if you'd prefer to keep these as separate variables?
| @@ -57,6 +57,7 @@ | |||
| "babel-plugin-add-module-exports": "^0.2.1", | |||
| "babel-plugin-dev-expression": "^0.2.1", | |||
| "babel-plugin-transform-object-assign": "^6.22.0", | |||
| "babel-plugin-transform-react-remove-prop-types": "^0.4.5", | |||
There was a problem hiding this comment.
why is this here? you aren't using it anywhere
There was a problem hiding this comment.
Good catch, forgot to add plugin wrap mode to .babelrc, Updated.
| @@ -9,6 +9,7 @@ | |||
| "plugins": [ | |||
| "dev-expression", | |||
| "add-module-exports", | |||
| "transform-object-assign" | |||
| "transform-object-assign", | |||
| ["transform-react-remove-prop-types", { mode: "wrap" }] | |||
There was a problem hiding this comment.
is the idea that wrapping will work with uglify to remove dead code?
There was a problem hiding this comment.
Yes, correct. Sample output bundle from development build would look as follows:
TransitionGroup.propTypes = process.env.NODE_ENV !== "production" ? {
component: _propTypes2.default.any,
childFactory: _propTypes2.default.func,
children: _propTypes2.default.node
} : {};
In the now production build for this app (as well as end-users who import and build production bundles using either Webpack -p or equivalently Define + Uglify plugins) will result in:
After Define
TransitionGroup.propTypes = false ? {
component: _propTypes2.default.any,
childFactory: _propTypes2.default.func,
children: _propTypes2.default.node
} : {};
After Uglify
TransitionGroup.propTypes = {};
So small changes; however, allows for smaller code base in this production bundle -- which is a good thing... And optionally allows for end-users to perform the same optimizations when using bundlers like Webpack, browserify, etc, when not using external distribution production build.
This is how a lot of packages help reduce filesize (e.g. material-ui)
does find dead code and remove it.
|
@jquense The uglify plugin does remove the dead code for prop types when defined in the original location, so I went ahead and restored those. So essentially, this saves a few hundred bytes in production now with the following changes :) Let me know if you need anything else as the sooner we can get the next release cut we can use production dist minified from unpkg. |
virgofx
force-pushed
the
master
branch
3 times, most recently
from
04bf0af
to
bba5d44
Compare
|
ping @jquense |
|
thanks! |
* Fixes #69: Adding capability to reduce production build output size and allow users that import this package to include prop types in development and optionally remove them using Webpacks UglifyJS + Define plugin. * Updated .babelrc that properly wraps prop-types. * Restoring the original constant declaration for prop types as uglify does find dead code and remove it. * Updating changelog.
* Fixes #69: Adding capability to reduce production build output size and allow users that import this package to include prop types in development and optionally remove them using Webpacks UglifyJS + Define plugin. * Updated .babelrc that properly wraps prop-types. * Restoring the original constant declaration for prop types as uglify does find dead code and remove it. * Updating changelog.
* Initial * clean up prop names * more exploration * fun children * fix tests * some comment docs * Incorporated development + production builds into /dist (#64) (#66) * Incorporated development + production builds into /dist (#64) and updated Readme to include CDN/External information. * Dropping references to Webpack specific build configurations in Readme and adding clarity for CDN/external. * Enhancement for best practice prop types. (#70) * Fixes #69: Adding capability to reduce production build output size and allow users that import this package to include prop types in development and optionally remove them using Webpacks UglifyJS + Define plugin. * Updated .babelrc that properly wraps prop-types. * Restoring the original constant declaration for prop types as uglify does find dead code and remove it. * Updating changelog.
Fixes #69: Reduces output bundles by wrapping prop types allowing for end users to further remove them using the babel plugin in combination with webpack define plugin. Production build file reduced by removing propTypes for static dist build.