vercel / next.js

The React Framework
https://nextjs.org
MIT License
125.25k stars 26.77k forks source link

Serverless, treeshaking #8956

Closed fernandobandeira closed 4 years ago

fernandobandeira commented 4 years ago

Bug report

Describe the bug

Currently, if the next build is targeted to serverless, the resulting files don't go through tree shaking.

To Reproduce

Checkout this repo: https://github.com/fernandobandeira/next-serverless-treeshakingissue Just install and build the project, if you inspect the resulting index.js file, you'll see that it has 1mb and contains all of the icons from font-awesome

Expected behavior

The only icon that should appear there is the one I've imported, it shouldn't include unused code.

Additional context

For me, this is resulting in each page of my app having 16mb, more than 500mb for the entire app and 120mb zipped, which makes it impossible to deploy on aws lambda.

If you remove the target to serverless, the compiled server file only includes the cofee icon, as it should.

timneutkens commented 4 years ago

Note that you shouldn't zip all pages into one lambda. You should create separate lambdas for every page. Pages are standalone lambda functions.

fernandobandeira commented 4 years ago

@timneutkens Cool, we are using a third party lib to help us with the deployment process but it doesn't split the deployments, there's an issue there https://github.com/danielcondemarin/serverless-next.js/issues/49 open for this.

We should be able to deploy our app after the treeshaking but we'll come up with a better solution, hopefully we'll send a PR there to get things done in the right way.

Thanks for the quick comment, ❤️ next.js, cheers.

Enalmada commented 4 years ago

I confirm this. Loading font-awesome is making my serverless files huge. If I do deep imports to avoid tree shaking it helps:

import { faCoffee } from '@fortawesome/free-solid-svg-icons/faCoffee'

@timneutkens If I add these settings from this article, https://wanago.io/2018/08/13/webpack-4-course-part-seven-decreasing-the-bundle-size-with-tree-shaking, to my webpack config I get quite a bit smaller serverless file sizes. Is there something unsafe about them or could whatever Next is missing be added to the defaults so everyone could get smaller sizes?

  webpack: (config, options) => {
        config.optimization.minimize = true;
        config.optimization.providedExports = true;
        config.optimization.usedExports = true;
        config.optimization.sideEffects = true;
       return config;
}
timneutkens commented 4 years ago
      config.optimization.providedExports = true;
      config.optimization.usedExports = true;
      config.optimization.sideEffects = true;

These don't do anything, they're enabled by default.

      config.optimization.minimize = true;

Minification is disabled by default for serverless bundles as it significantly increases build times and doesn't give any significant benefits when deploying serverless bundles.

Enalmada commented 4 years ago

For me, the serverless files generated by Next go from about 5.5m to 3.4m with minimize=true. When having tons of routes and then sourcemaps this could add up. Though it seems like treeshaking not working with serverless build is the real issue.

sdornan commented 4 years ago

I'm also running into this issue too. Only setting config.optimization.minimize = true; makes my files small enough to deploy to Lambda, but as stated this setting causes builds to take forever.

timneutkens commented 4 years ago

@sdornan that would mean you're deploying wrong (all in one lambda I guess). A page is generally under 5MB by itself and when using the serverless target every page should be a separate lambda.

timneutkens commented 4 years ago

Closing this as we're currently not investigating this and no PR was opened.

nodabladam commented 4 years ago

@timneutkens Your feedback about why you intentionally don't minimize serverless files was very helpful. Is there any background you could provide on why Next.js doesn't feel it is worth the effort to investigate turning on treeshaking? The bloated code from huge libraries and font packs in everyones builds must be impacting every single next.js serverless users performance and most users probably don't even realize they lost treeshaking when moving to serverless so I am curious if this is just an oversight or if there is some big known issue. Some pointers on how one might attempt to fix could make it more likely for a PR to be opened.

timneutkens commented 4 years ago

Hey Adam, based on how I'm reading your message it sounds like you're confusing client-side tree shaking with serverless / what we disabled (actually we didn't disable anything new). Maybe you're not confusing it, but either way it might be useful to type this out:

Next.js has 2 compilations, client and server. A lot of the "tree shaking" is actually just minification (webpack inlines constants, terser removes the code block).

The bloated code from huge libraries and font packs in everyones builds must be impacting every single next.js serverless users performance

The thing with the serverless target is that you have to bundle every page as a separate serverless function. If you compare this to having to ship the full node_modules + all pages in a container you'll get that the size differences are tremendous. One example is how zeit.co had a 300-400Mb docker image. Now it has page render functions that are always around 500Kb or less zipped (more so because we have a lot of custom code in them, not because of Next.js).

Anyway, to recap. Next.js does do tree shaking, just not on the server compilation, which is fine.

nodabladam commented 4 years ago

So I am using several huge font packs and when doing a next.js non-serverless build, I load them like this import { faCoffee } from '@fortawesome/free-solid-svg-icons' and only the ones I reference are included in my bundle. It feels like a bug that when I switch to serverless, I see every font awesome 5 icon in every serverless function bundle rather than just the ones I reference in the page. The workaround is to do the "tree shaking" myself by doing this import { faCoffee } from '@fortawesome/free-solid-svg-icons/faCoffee' but I feel like this should be unnecessary for every user trying to move from server to serverless to do manually.

I guess when in serverless mode, I expected each page to be built individually and only have the externals it needed because each page is essentially a unique independent build run through the same processes (minus terser) that non-serverless is.

timneutkens commented 4 years ago

On ZEIT Now Next.js already uses this target instead of the current serverless one: https://github.com/zeit/next.js/pull/8246

trace has a better result for the case that you're describing but you have even more implementation steps (all taken care of automatically on ZEIT Now).

nodabladam commented 4 years ago

Enabling experimental-serverless-trace makes my average file size go from image to image ...so yes, it does indeed seem like that is the solution to this issue. (FYI minimize cuts that in half again and the build time isn't that much slower vs before but obviously trace is the big deal) image

I would strongly recommend everyone use ZEIT Now for serverless but for those already running on AWS in target serverless and not yet able to move, what implementation steps need to be handled manually?

daniel-minchev commented 4 years ago

Anyone with any solution? My pages are like 10MB each, and I have a lot of pages.

ahmadi-akbar commented 3 years ago

Anyone with any solution? My pages are like 10MB each, and I have a lot of pages.

I have same issue and my out dir size had 150MB!!!

Try this add "rimraf" package to your project,

yarn add -D rimraf
or
npm i rimraf --save-dev

add this command to your package.json

"preexport": "rimraf out .next"

this snippet run before your "export" command and delete all cached builds

patroza commented 3 years ago

@timneutkens

Minification is disabled by default for serverless bundles as it significantly increases build times and doesn't give any significant benefits when deploying serverless bundles.

We've had serverless function cold boots from about 3-10s on Vercel with serverless nextjs.. Which brought us rather close to ditching serverless and going classical.

In the meantime figured out a bunch of ways to optimise and reduce bundle sizes etc. but to me, every few ms count here. imo, serverless functions have a similar performance importance as browser - not server. The server can keep all in memory and keep running, so you don't need to care, but at serverless where cold starts are the name of the game it seems, it really does matter.

So we're happy with setting minimize = true, and paying the cost at build-time ;-)

Perhaps minification should be auto-enabled after certain sizes, or we should at least have a prominent report of these sizes and their impacts, as much as the current browser payload informations and warnings at next build.

leerob commented 3 years ago

Hey @patroza, would you mind including a reproduction of this cold boot? Happy to help assist and look into it further. 3-10s is definitely unexpected and not what we normally see.

patroza commented 3 years ago

Hey @patroza, would you mind including a reproduction of this cold boot? Happy to help assist and look into it further. 3-10s is definitely unexpected and not what we normally see.

hey @leerob thanks for the response! It's a private app hosted on Vercel. I'm not sure what would be the easiest way for reproduction, debug access, some dump? thanks!

leerob commented 3 years ago

In that case, please contact Vercel support 🙏

balazsorban44 commented 2 years ago

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.