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 some good defaults to code-splitting #1644

Closed
wants to merge 2 commits into from

Conversation

arunoda
Copy link
Contributor

@arunoda arunoda commented Apr 6, 2017

Move any module in 0.5 of total pages into the commons bundle.

As the pages count increases, it's hard to use a module on all the
pages.
So, now we'll pick a module which is used at-least in half of the pages.
This gives us dramatic space save in individual pages.

Here's the Zeit.co's page sizes without this fix:
without

Here's with this fix:
with

As the pages count increases, it's hard to use a module on all the
pages.
So, now we'll pick a module which is used at-least in half of the pages.
This gives us dramatic space save in individual pages.
@rauchg
Copy link
Member

rauchg commented Apr 6, 2017

I'm 100% ok with this, but I think we should have an example in the examples dir where people can supply their own common chunks logic (by extending webpack and replacing the common chunks plugin for example)

@arunoda
Copy link
Contributor Author

arunoda commented Apr 6, 2017

How about if we expose a function to do it in next.config.js?

@rauchg
Copy link
Member

rauchg commented Apr 6, 2017

Yeah, my point was, before we make that decision (which is not an easy one), I'd like to have a escape hatch for people to revert back or change to whatever constant they want.

@arunoda
Copy link
Contributor Author

arunoda commented Apr 6, 2017

@rauchg Okay sure. I'll play with it and make it configurable.
First thing up for tomorrow morning.

@arunoda
Copy link
Contributor Author

arunoda commented Apr 7, 2017

@rauchg check now.

@arunoda
Copy link
Contributor Author

arunoda commented Apr 7, 2017

We took #1659 (comment) which is pretty similar but without the public API.

@arunoda arunoda closed this Apr 7, 2017
@arunoda arunoda deleted the code-split-good-defaults branch April 7, 2017 22:54
gcpantazis pushed a commit to gcpantazis/next.js that referenced this pull request Jan 22, 2018
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)
@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 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.

2 participants