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

Be more aggressive about minification #310

Merged
merged 1 commit into from Oct 16, 2015
Merged

Be more aggressive about minification #310

merged 1 commit into from Oct 16, 2015

Conversation

danvk
Copy link
Collaborator

@danvk danvk commented Oct 16, 2015

There are two things going on here:

  • Using -g [ envify ] instead of -t [envify] applies the transform globally (i.e. including to code in node_modules), rather than just to our code. This means that it sets the NODE_ENV variable inside of React.
  • The uglifyify transform minifies each module as browserify sees it. This means that dead require statements are removed.

These combine to shrink React pretty dramatically, since all the dev-mode code can be stripped.

452,690 pileup.min.js before
394,079 pileup.min.js after

∆ = 58,611 bytes (13%)

Looking at the source map, most of the difference is coming from React:

207,354 /react before (172 files in bundle)
139,935 /react after (167* files in bundle)

∆ = 67,419 bytes (33%)

Closes #307

* the dropped files are:

node_modules/react/lib/ReactDefaultPerf.js
node_modules/react/lib/ReactDefaultPerfAnalysis.js
node_modules/react/lib/ReactTestUtils.js
node_modules/react/lib/performance.js
node_modules/react/lib/performanceNow.js

Review on Reviewable

@danvk
Copy link
Collaborator Author

danvk commented Oct 16, 2015

cc @ihodes

@@ -29,10 +29,10 @@
"jstransform": "./scripts/jstransform.sh",
"jstransform-watch": "./scripts/jstransform.sh --watch",
"browserify-all": "npm run browserify && npm run browserify-test",
"browserify": "browserify -r .:pileup -v -t [ envify --NODE_ENV production ] --debug -o dist/pileup.js",
"browserify": "browserify -r .:pileup -v -g [ envify --NODE_ENV production ] -g uglifyify --debug -o dist/pileup.js",
Copy link
Member

Choose a reason for hiding this comment

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

does this lead to double uglification (first via uglifyify and then directly via uglify)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes -- though this is actually recommended!

When using uglifyify you should generally also use Uglify, to achieve the smallest output. Uglifyify provides an additional optimization when used with Uglify, but does not provide all of the optimization that using Uglify on its own does, so it's not a replacement.

Copy link
Member

Choose a reason for hiding this comment

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

aha, magic 🔮

@armish
Copy link
Member

armish commented Oct 16, 2015

File size looks remarkably small now! Thanks for testing these options out, @danvk.

danvk added a commit that referenced this pull request Oct 16, 2015
Be more aggressive about minification
@danvk danvk merged commit 5558445 into master Oct 16, 2015
@danvk danvk deleted the uglifyify branch November 17, 2015 16:59
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.

None yet

2 participants