-
Notifications
You must be signed in to change notification settings - Fork 27.8k
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
Make webpack CommonsChunkPlugin chunking default configurable #3600
Make webpack CommonsChunkPlugin chunking default configurable #3600
Conversation
A while ago (in vercel#1644) @arunoda updated the default for minimum refs to chunk a file into commons from the previous (which I believe was a static integer, or at least was at some point), and made it so that the file had to be used in > 50% of the pages. The problem is that people may use pages in different ways, particularly if they're using a custom server instead of the default page routing. For example, I use Styled Components, and put a `styles.js` file alongside my various page JSX files. Others may colocate JSX components inside pages, which could be very reasonable depending on the project. The point is, it's very possible to load up your /pages directory with JS files that add into the default logic's perception of "page count", since there's no singular way to determine what is and is not a page without analyzing the code within. I'd propose that leave this as the default behavior, since that makes sense if you're using Next in the OOTB manner, but allow for configuration if needed. @rauchg suggested as much in vercel#1644 but it seems like that fell out in the final version (vercel#1659)
@@ -5,6 +5,15 @@ const cache = new Map() | |||
const defaultConfig = { | |||
webpack: null, | |||
webpackDevMiddleware: null, | |||
webpackModuleShouldBeCommonInProduction: function ({totalPages, count}) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a pretty long function name and it's not so clear, I understood by the name that I'll be able to define if an individual module should be in the common chunks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hah, fair enough. Technically you could/(should) pass the whole module
into this method if it were to exist, which would make the method name correct (if long).
In any case I think I'll abort this approach per @timneutkens's comment
When you start adding components and styles inside Considering this, the rule is pretty simple: every file inside of pages is a page, and shouldn't be something else 😄 Regarding the added functionality, this can be added using |
All good points @timneutkens.
Hah. I think this is a good basic rule, sure. I have my one use-case (separate Just so I'm gauging the philosophy correctly: the project wants to discourage top-level configuration in Or put another way, if there's a clear way to do it with webpack, we should be doing it with webpack, not config.
Yeah this makes sense. I'll see if there's anything that needs to be changed (passing |
We have a use case that supports this feature as well. Within our app, we have an internal facing portal that is name-spaced within pages |
You can do it like this: https://github.com/zeit/next-plugins/blob/master/packages/next-css/commons-chunk-config.js |
Closing this in favor of #3824 |
A while ago (in #1644) we updated the default for minimum refs to chunk a file into commons from the previous (which I believe was a static integer, or at least was at some point), and made it so that the file had to be used in >50% of the pages to be packaged in the commons.
The problem is that people may use pages in different ways, particularly if they're using a custom server instead of the default page routing. For example, I use Styled Components, and put a
styles.js
file alongside my various page JSX files. Others may colocate JSX components inside pages, which could be very reasonable depending on the project.In any case, it's very possible to load up your /pages directory with JS files that add into the default logic's perception of "page count", since there's no singular way to determine what is and is not a page without analyzing the code within.
I'd propose that leave this as the default behavior, since that makes sense enough if you're using Next in the OOTB manner, but allow for configuration if needed. @rauchg suggested as much in #1644 but it seems like that fell out in the final version (#1659)
If this seems good to all I'll add examples / update docs. LMK, thanks!