Skip to content

Fix minimization of bundles #459

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

Merged
merged 1 commit into from
Jul 29, 2018
Merged

Fix minimization of bundles #459

merged 1 commit into from
Jul 29, 2018

Conversation

appden
Copy link
Contributor

@appden appden commented Jul 18, 2018

The default minimizer created by Webpack does not provide a test options, so the default tests just for .js files. Since we rely on a different extension for our output bundles, we need to explicitly use UglifyJsPlugin ourselves.

Fixes #458

The default `minimizer` created by Webpack does not provide a `test` options, so the default tests just for `.js` files. Since we rely on a different extension for our output bundles, we need to explicitly use `UglifyJsPlugin` ourselves.
Copy link
Member

@thymikee thymikee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks ok. @zamotany @satya164?

@satya164
Copy link
Member

Seems the plugin uses uglify-es which is unmaintained and causes bugs with React https://twitter.com/greweb/status/1013093979309727752?s=19

Is there a way to use V2? Maybe it'll be better to switch to babili?

@hedgepigdaniel
Copy link
Contributor

terser is a drop in replacement for uglify (its a fork which is maintained). From what I understand, its faster than babili. Turning off compression makes the process about 3x faster again, with minimal/no increase in filesize.

@appden
Copy link
Contributor Author

appden commented Jul 22, 2018

Not sure which direction you guys would like to go here. Unfortunately, babel-minify-webpack-plugin doesn't look well maintained at this point either.

Terser looks great. We would either recommend users swap it in with the Yarn resolutions hack, or instead depend on this terser-webpack-plugin instead (my personal recommendation).

It looks like this will be temporary dependency since Webpack 5 will include its own MinimizerWebpackPlugin (according to this comment), which will support Terser.

@drgx
Copy link

drgx commented Jul 23, 2018

Hi @appden ,Thanks for you PR but i want to share my finding that related to haul minification:

  • Latest haul version my bundle size was 7MB (minification not working)
  • With you minify changes the bundle size reduce to 4MB
  • Downgrade to haul 1.0.0-beta.13 (before webpack 4) the size reduce to 2.7MB
  • Using metro bundle the bundle size 2.5MB
    After your magnification changes, the bundle size still bigger than pervious haul version and metro. 😢
    Our android team really strict on the RN bundle size. Hopefully we can find a way to reduce the bundle size so it same or smaller than metro version 🙇

Great work btw! 👍

@thymikee thymikee changed the title Fix minimization of bundles (#458) Fix minimization of bundles Jul 29, 2018
@thymikee thymikee merged commit a1d03f5 into callstack:next Jul 29, 2018
@appden appden deleted the minimize-fix branch July 30, 2018 23:42
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.

6 participants