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

Guides - Tree Shaking and Production Rewrites #1508

Merged
merged 9 commits into from
Aug 26, 2017

Conversation

skipjack
Copy link
Collaborator

@skipjack skipjack commented Aug 11, 2017

This continues our work #1258 and resolves most of the issues mentioned in #1331. It is a WIP though, we still need to populate the TODO code blocks and I'm currently working on updating the
Production guide which may take a bit longer.

Resolves #1331
Related #1258

cc @jakearchibald

@skipjack
Copy link
Collaborator Author

Ok I'm still working on this a little (e.g. some links and examples are missing) but for the most part the outline of these is pretty much good to go in my mind. @TheDutchCoder @jakearchibald if either of you have some time to review and let me know your thoughts I'd appreciate it.

Also anyone whose contributed to this guide in the past...

@henriquea, @rajagopal4890, @markerikson, @simon04, @kisnows, @chrisVillanueva, @swapnilmishra, @bring2dip, @redian, @xgqfrms

Please feel free to review as well. My goal is not to blow away any previous work, just to synchronize the guide with our others (#1258) and make it a bit more approachable.

@sokra I put a little TODO about the LoaderOptionsPlugin at the bottom... this is mentioned in the current one I just wanted to clarify that it is still relevant. This is still included by --optimize-minimize to easily indicate to all loaders that they should minimize their output and not output dev logs (debug)?

The way I broke this down I think should make it easy to add small targeted sections on each configuration change recommended for production. For example, if the LoaderOptionsPlugin is still relevant, I'll just add another small section that introduces it.

@skipjack
Copy link
Collaborator Author

I'm going to work on resolving the last couple TODOs and adding some demos to @TheDutchCoder's webpack-guides-code-examples repository. Once that's done though (probably in the next day or two), I'll merge so if anyone has time to give some feedback please do so as soon as you can! Otherwise just leave your comments after the merge and we can discuss more and resolve any issues in follow-up PRs.

This continues our work #1258 and resolves most of the issues mentioned
in #1331. It is a WIP though, we still need to populate the `TODO` code
blocks.
I've copied the older content to separate files locally so I could start
outlining the structure of the updated guide. I will slowly integrate various
parts back in as this gets more fleshed out.
Adding this is never even mentioned, I think it was just copied over
by accident. It's covered in detail in the hot-module-replacement guide
which comes immediately after this one.

cc @TheDutchCoder
@skipjack skipjack force-pushed the tree-shaking-n-production branch from 257bce1 to 81672d0 Compare August 26, 2017 13:48
skipjack added a commit to TheDutchCoder/webpack-guides-code-examples that referenced this pull request Aug 26, 2017
Add tree shaking example that corresponds with:

webpack/webpack.js.org#1508
skipjack added a commit to TheDutchCoder/webpack-guides-code-examples that referenced this pull request Aug 26, 2017
Add example for the production guide that corresponds to:

webpack/webpack.js.org#1508
@skipjack
Copy link
Collaborator Author

Opened the following example PRs and populated the missing content:

I'm going to merge but still interested in feedback if anyone has ideas.

@sokra I removed this TODO but I'm interested in your thoughts? Is it still worth mentioning it so that people put all loaders into minimize mode with debug turned off?

// TODO: Consider mentioning the `LoaderOptionsPlugin`... is this still relevant?

//  new webpack.LoaderOptionsPlugin({
//    minimize: true,
//    debug: false
//  }),

@skipjack skipjack force-pushed the tree-shaking-n-production branch from a87640d to ed1bc15 Compare August 26, 2017 16:42
@skipjack
Copy link
Collaborator Author

Ok I ran the build/tests locally and everything passed. I'm going to give this one last chance to pass on travis before merging. The terminated issue is happening on other PRs as well (e.g. #1549 and #1547).

@sokra @bebraw @Munter initially I thought it was a hyperlink running too long issue but I tested the lint:links / hyperlink and it ran fine (completed in ~2 minutes). So, I'm not exactly sure what's going on with the terminations and my attempts to resolve it with travis_wait failed. Anyone have thoughts on how to resolve this?

@skipjack
Copy link
Collaborator Author

skipjack commented Aug 26, 2017

Actually, I watched the last build when it hit the end and it does hang on hyperlink for what seems to be much longer than 2 min. @Munter could this be caused by a difference in environment maybe? I'm running Node v6.2.2 and NPM v3.9.5 locally but on Travis it's v6.11.2 and v3.10.10.

@skipjack skipjack merged commit ebcdee0 into master Aug 26, 2017
@skipjack skipjack deleted the tree-shaking-n-production branch August 26, 2017 17:20
TheDutchCoder pushed a commit to TheDutchCoder/webpack-guides-code-examples that referenced this pull request Oct 2, 2017
TheDutchCoder pushed a commit to TheDutchCoder/webpack-guides-code-examples that referenced this pull request Oct 3, 2017
Add example for the production guide that corresponds to:

webpack/webpack.js.org#1508
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Guides - Issues with Tree Shaking & Production
1 participant