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

Add support for webpack's CommonsChunkPlugin #301

Merged
merged 17 commits into from Nov 28, 2016
Merged

Conversation

arunoda
Copy link
Contributor

@arunoda arunoda commented Nov 27, 2016

Fixes #253
Similar to #286 (We didn't take #286 in)

Here now common modules loads once using the webpack's CommonsChunkPlugin. Still, we use Next.js module loading system and SSR features.

As a result of this, we don't need to worry about loading the next-bundle and even creating it.

This example features:

* An app with two pages which has a common Counter component
* That Counter component maintain the counter inside it's module.
Copy link
Member

Choose a reason for hiding this comment

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

its

Copy link
Member

Choose a reason for hiding this comment

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

Maybe reword this sentence? It's not 100% clear to me

Copy link
Member

Choose a reason for hiding this comment

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

Maybe say something along the lines: the common plugin maintains state that is initialized only once.

Copy link
Member

@rauchg rauchg left a comment

Choose a reason for hiding this comment

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

looking great!!!

@nkzawa
Copy link
Contributor

nkzawa commented Nov 28, 2016

👍

@nkzawa nkzawa merged commit fcd59ad into vercel:master Nov 28, 2016
@arunoda
Copy link
Contributor Author

arunoda commented Nov 28, 2016

Superb!

@jonaswindey
Copy link
Contributor

Does this mean no configuration is required to make a component/module "common"?

@arunoda
Copy link
Contributor Author

arunoda commented Nov 28, 2016

@jonaswindey no we don't need.

Currently it uses defaults options for commonChunksPlugin

In a future(near) version of Next.js we'll offer some ways to configure options for commonChunksPlugin and move some specific modules into the common chunk.

@arunoda
Copy link
Contributor Author

arunoda commented Nov 28, 2016

@jonaswindey more info about the current logic of common modules.

If a module used in all of the pages, it will move into the commons JavaScript bundle.
So, individual pages doesn't contain that modules.
If not, individual modules will contain a copy of that module.

But in anyway, a module only loads once even if pages contains multiple copies of it.

@caillou caillou mentioned this pull request Nov 28, 2016
@arunoda arunoda changed the title Add support for webpack's CommonsChunkPlugin and remove next bundle Add support for webpack's CommonsChunkPlugin Nov 28, 2016
@sedubois sedubois mentioned this pull request Nov 29, 2016
@adamrights
Copy link

One thing to consider is how aggressively assets are concatenated in http2 setups:

Concatenation is not necessarily an anti-pattern with HTTP/2. Under HTTP/2, it’s good practice to keep individual files small and ensure that resources are only served when needed. That being said, other factors can affect the the speed tradeoffs of individual resources. For example, when Khan Academy served over 300 individual JavaScript files to HTTP/2 users, they saw a degradation in performance due to less efficient compression over multiple files, and server delays related to reading each file from disk. For more information, see Khan Academy’s article and Smashing Magazine’s HTTP/2 guide.

Generally speaking, organizing your resources into smaller, logical files rather than bundling them one large file offers the best performance over HTTP/2. The number of files that you can serve over HTTP/2 for a single URL without degrading performance depends heavily on the codebase and the server. We suggest keeping the number of files below 50 per URL, as that seems to be the point at which many servers suffer from having to read so many individual files from disk.

source

@lock lock bot locked as resolved and limited conversation to collaborators Feb 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants