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

Serverless, treeshaking #8956

Closed
fernandobandeira opened this issue Oct 3, 2019 · 20 comments
Closed

Serverless, treeshaking #8956

fernandobandeira opened this issue Oct 3, 2019 · 20 comments
Labels
good first issue Easy to fix issues, good for newcomers

Comments

@fernandobandeira
Copy link

fernandobandeira commented Oct 3, 2019

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 timneutkens added good first issue Easy to fix issues, good for newcomers help wanted labels Oct 3, 2019
@timneutkens
Copy link
Member

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
Copy link
Author

fernandobandeira commented Oct 3, 2019

@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 serverless-nextjs/serverless-next.js#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
Copy link
Contributor

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
Copy link
Member

      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
Copy link
Contributor

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
Copy link
Contributor

sdornan commented Dec 14, 2019

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
Copy link
Member

@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
Copy link
Member

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

@nodabladam
Copy link

nodabladam commented Jan 3, 2020

@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
Copy link
Member

timneutkens commented Jan 3, 2020

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).

  • Next.js by default does not run terser on the server-compilation (not by default, not when serverless)
  • When not using serverless all node_modules are externals in webpack, meaning they're not touched at all by webpack and you actually need to install all dependencies in order for the app to run correctly. When using serverless deps are not marked external and are bundled by webpack.

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
Copy link

nodabladam commented Jan 3, 2020

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
Copy link
Member

On ZEIT Now Next.js already uses this target instead of the current serverless one: #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
Copy link

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
Copy link

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

@ahmadi-akbar
Copy link

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
Copy link

patroza commented Jun 27, 2021

@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
Copy link
Member

leerob commented Jun 28, 2021

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
Copy link

patroza commented Jul 3, 2021

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
Copy link
Member

leerob commented Jul 4, 2021

In that case, please contact Vercel support 🙏

@balazsorban44
Copy link
Member

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.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
good first issue Easy to fix issues, good for newcomers
Projects
None yet
Development

No branches or pull requests

10 participants