Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid including full Lodash build #45

Merged
merged 5 commits into from
Nov 17, 2016
Merged

Avoid including full Lodash build #45

merged 5 commits into from
Nov 17, 2016

Conversation

nylen
Copy link
Contributor

@nylen nylen commented Nov 15, 2016

Currently Lodash is a significant portion of the build size (source):

In Automattic/wp-calypso#6539 this approach was removed from Calypso in favor of a Babel plugin which automatically transforms imports as follows:

- import { map } from 'lodash';
+ import map from 'lodash/map';

I've opted for the older approach here (transforming imports manually + forbidding "root" lodash imports) because it would be quite difficult to add a new babel plugin without ejecting from create-react-app. (We could do it, but we would have to override npm run build with a custom command, require the webpack config from react-scripts ourselves, and make our changes to the webpack config object before the build runs.)

See also Automattic/eslint-plugin-wpcalypso@02a997a.

With the changes here, the minified build is down from 513kB to 450kB. (The graphic above shows only unminified sizes; Lodash unminified size is down from 608kB to 119kB).

@nylen
Copy link
Contributor Author

nylen commented Nov 15, 2016

Including custom Webpack config / Babel plugins with create-react-app appears to be discouraged. Some more info:

@nylen
Copy link
Contributor Author

nylen commented Nov 15, 2016

Since today is hack day at Automattic, let's try it anyway. 6838226 overrides the Webpack config from create-react-app via a small custom build script.

@youknowriad
Copy link
Collaborator

Well! this does not seem so violent, so I would agree to merge it.

I wanted to note also that CRA is waiting to Webpack 2 to go out of beta to integrate it. I think we'll have tree shaking out of the box then.

@nylen
Copy link
Contributor Author

nylen commented Nov 16, 2016

Yeah, I think this turned out pretty nicely, and it's a good example of how to override CRA configs (but this is definitely unsupported and something to be careful with).

@timarney
Copy link

Again

"Definitely unsupported and something to be careful with"

https://github.com/timarney/react-app-rewired

@nylen
Copy link
Contributor Author

nylen commented Nov 17, 2016

Configure the unconfigurable

💥

@timarney
Copy link

I don't have time at the moment to test but if your using CRA I would be curious to see assuming it ended up being compatible (it should) what the file size would be for you if you flipped to use Preact see this example https://github.com/timarney/react-app-rewired/blob/master/examples/preact.md

@gaearon
Copy link

gaearon commented Nov 20, 2016

because it would be quite difficult to add a new babel plugin without ejecting from create-react-app

Have you considered filing an issue in CRA describing your use case? Just because we don't allow configuration doesn't mean we won't be happy to merge a plugin that reduces bundle size for users of one of the most popular libraries on npm.

@gaearon
Copy link

gaearon commented Nov 20, 2016

I'd totally take a PR adding lodash plugin to our Babel config.

@youknowriad
Copy link
Collaborator

@gaearon Thanks for the heads up. You're right, we should have thought about raising this in CRA. I guess we just thought this was specific to a library.

I see that it's been worked on. That's awesome, we'll drop the custom script once released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants