vercel / next.js

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

React lazy or next dynamic don't reduce an app route's "First Load JS" #49454

Closed antoineol closed 1 year ago

antoineol commented 1 year ago

Verify canary release

Provide environment information

Operating System:
      Platform: linux
      Arch: x64
      Version: #22 SMP Tue Jan 10 18:39:00 UTC 2023
    Binaries:
      Node: 18.16.0
      npm: 9.5.1
      Yarn: 1.22.19
      pnpm: N/A
    Relevant packages:
      next: 13.4.2-canary.0
      eslint-config-next: 13.4.0
      react: 18.2.0
      react-dom: 18.2.0
      typescript: 5.0.4

Which area(s) of Next.js are affected? (leave empty if unsure)

No response

Link to the code that reproduces this issue

https://codesandbox.io/p/github/antoineol/repro-next-lazy-build-size/main?layout=%257B%2522activeFilepath%2522%253A%2522%252Fapp%252Fpage.tsx%2522%252C%2522openFiles%2522%253A%255B%255D%252C%2522sidebarPanel%2522%253A%2522EXPLORER%2522%252C%2522gitSidebarPanel%2522%253A%2522COMMIT%2522%252C%2522fullScreenDevtools%2522%253Afalse%252C%2522rootPanelGroup%2522%253A%257B%2522type%2522%253A%2522PANEL_GROUP%2522%252C%2522panels%2522%253A%255B%257B%2522type%2522%253A%2522PANEL%2522%252C%2522panelType%2522%253A%2522TABS%2522%252C%2522id%2522%253A%2522clheryg3i00z3356lq12hxizh%2522%257D%255D%252C%2522direction%2522%253A%2522vertical%2522%252C%2522id%2522%253A%2522DEVTOOLS_PANELS%2522%257D%252C%2522tabbedPanels%2522%253A%257B%2522clheryg3i00z3356lq12hxizh%2522%253A%257B%2522id%2522%253A%2522clheryg3i00z3356lq12hxizh%2522%252C%2522tabs%2522%253A%255B%257B%2522type%2522%253A%2522TASK_LOG%2522%252C%2522id%2522%253A%2522clhes0d6l01go356lvx2x3s0w%2522%252C%2522taskId%2522%253A%2522build%2522%257D%255D%252C%2522activeTabId%2522%253A%2522clhes0d6l01go356lvx2x3s0w%2522%257D%257D%252C%2522showSidebar%2522%253Atrue%252C%2522showDevtools%2522%253Atrue%252C%2522sidebarPanelSize%2522%253A15%252C%2522editorPanelSize%2522%253A41.68257051007683%252C%2522devtoolsPanelSize%2522%253A41.442795629349064%257D

To Reproduce

https://github.com/antoineol/repro-next-lazy-build-size

yarn build, you will get:

image

Open app/page.tsx, comment this line:

const DynamicHeavy = lazy(() => import("./DynamicHeavy"));

Then build again. You will get:

image

Same result if using the dynamic wrapper instead of lazy.

Describe the Bug

My understanding is that the "First Load JS" is the minimal bundle size to render the page, excluding lazy-loaded content that are in separate chunks, and counted separately. I expect "First Load JS" to be what matters for a quick initial rendering to improve the lighthouse score.

In this experimentation, lazy does not seem to reduce this initial bundle size. It behaves like a traditional "import" that includes everything in the initial bundle.

Unless I'm missing something?

Expected Behavior

The "First Load JS" remains the same, with or without this line commented:

const DynamicHeavy = lazy(() => import("./DynamicHeavy"));

Since I would expect it to he a separate chunk, not counted in the initial render.

Which browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

ethanryan commented 1 year ago

@antoineol try adding ssr: false to your dynamic import:

const DynamicHeavy = dynamic(() => import("./DynamicHeavy"), {ssr: false});

and make sure you are using a dynamic import: https://nextjs.org/docs/app/building-your-application/optimizing/lazy-loading#nextdynamic

see docs:

Skipping SSR When using React.lazy() and Suspense, client components will be pre-rendered (SSR) by default.

If you want to disable pre-rendering for a client component, you can use the ssr option set to false:

const ComponentC = dynamic(() => import('../components/C'), { ssr: false });

via: https://nextjs.org/docs/app/building-your-application/optimizing/lazy-loading#skipping-ssr


Also, based on your before and after screenshots, your homepage size and first load JS is decreasing.

Before: route: / size: 195 kB first load JS: 272 kB

After:
route: / size: 4.93 kB first load JS: 81.kB

antoineol commented 1 year ago

Using dynamic with ssr: false doesn't seem to change the JS load size in the build :(

image

CodeSandbox link

Am I missing something?

I remember I read that lazy() was the recommended approach over dynamic, but I can't find the mention. Was it removed?

Assuming the SSR is what would make the trick, does activating the SSR have any noticeable negative impact, e.g. on lighthouse score, initial bundle size, user experience...? It sounds like doing as much as possible in SSR is the Next.js way, so I'm wondering if it is at the cost of a bigger initial bundle size. Or if it is just the build tool which prints harmless sizes (for other needs).

Also, based on your before and after screenshots, your homepage size and first load JS is decreasing.

Before = with lazy import, after = without any import. So yes, it's aligned with the issue reported (to clarify, I should have reverted the order).

ethanryan commented 1 year ago

does the "first load JS shared by all" number change? i see above, it is 76.9 kB.

is it smaller when using a dynamic import?

next/dynamic is what you should be using, and add ssr: false if what is being imported is a component that isn't needed for server-side rendering, for example, something that won't help with SEO.

https://nextjs.org/docs/pages/building-your-application/optimizing/lazy-loading#with-no-ssr

import dynamic from 'next/dynamic';

const DynamicHeader = dynamic(() => import('../components/header'), {
  ssr: false,
});
ethanryan commented 1 year ago

also I checked your CodeSandbox @antoineol, you're not rendering that imported component anywhere, try adding this:

        <DynamicHeavy />

within the page's return statement, for example:

        <p>
          Get started by editing&nbsp;
          <code className={styles.code}>app/page.tsx</code>
        </p>
        <DynamicHeavy />

that may make a difference, compared to rendering that component and importing it normally, without the dynamic render:

no dynamic import:

import DynamicHeavy from './DynamicHeavy'

versus dynamic, no srr:

import dynamic from 'next/dynamic';
const DynamicHeader = dynamic(() => import('../components/header'), {
  ssr: false,
});
antoineol commented 1 year ago

does the "first load JS shared by all" number change?

No, "first load JS shared by all" does not change. Only the "First Load JS" of the page, depending on what's imported.

you're not rendering that imported component anywhere

True, for the minimal reproduction, just importing (lazily) was enough to increase the "First Load JS", even without any usage. Using the component doesn't change it.


It sounds like the issue is still there:

Using dynamic with ssr: false doesn't seem to change the JS load size in the build :(

image

Any idea about what could be wrong?

Thanks for your help!

pewallin commented 1 year ago

We're seeing something similar. We are consuming our svg lib with a custom Icon-component where we pass name and then use an object built up by next/dynamic values for the actual svg icon. With the app dir enabled these svgs end up as part of one of the main bundles, without it into separate bundles per svg file (as intended). I've experimented a bit and it seems like next/dynamic loaded components referenced from a sever component will find it's way into one of the client js bundles.

ivan-kleshnin commented 1 year ago

I observe the same behavior in newer NextJS with pages (not using app directory and server components). dynamic components are bundled (regardless of {ssr: true} which is not related to bundling – not sure why it's mentioned as a potential solution...)

Which means dynamic does not actually work as a code splitter anymore 😨

Is it normal?

No, it worked properly in NextJS v12, at least. Was one of the best way to reduce a bundle size. Likely a regression after a rewrite to TurboPack. But it's kinda weird that nobody else has noticed it. Maybe it's conditional on something 🤔

Note: the only imports of forementioned components are dynamic, no silly mistakes like static + dynamic imports or reexports from index.ts...

alex-krasikau commented 1 year ago

Want to share my observations on this topic.

I created a demo using the latest 13.5.2 Next.js and App Router:

The application has a dynamic route segment that renders pages based on the configuration from the JSON file. In the application, I implemented 5 base sections used as building blocks for the pages:

  1. Light Server Component - plain text
  2. Heavy Server Component - receives a large payload
  3. Light Client Component - server + client component that renders plain text
  4. Heavy Client Component - server + client component that renders react-select
  5. Heavy Client Component 2 - server + client component that renders @dotlottie/react-player

I implemented lazy loading for both Heavy Components:

image

I tested different configurations of the pages, and I found that bundle splitting works only when dynamically loading the components from the client:

image

qqpann commented 1 year ago

having the same problem with app router

qqpann commented 1 year ago

It's a wired behavior (and not well documented), but I stopped dynamic import from ssr and extracted to a client side file it worked.

Before (600KB first load js):

1. app/specific/page.tsx # "use server". dynamic import form from here.
2. components/form.tsx # "use client"

After (80KB first load js):

1. app/specific/page.tsx # "use server". import dynamic-wrapper.tsx
2. components/dynamic-wrapper.tsx # "use client". dynamic import form.tsx from here.
3. components/form.tsx # "use client"
SushantLoop commented 1 year ago

Experiencing the same issue in next 13 using app router

jacekkuczynski commented 1 year ago

It's a wired behavior (and not well documented), but I stopped dynamic import from ssr and extracted to a client side file it worked.

Before (600KB first load js):

1. app/specific/page.tsx # "use server". dynamic import form from here.
2. components/form.tsx # "use client"

After (80KB first load js):

1. app/specific/page.tsx # "use server". import dynamic-wrapper.tsx
2. components/dynamic-wrapper.tsx # "use client". dynamic import form.tsx from here.
3. components/form.tsx # "use client"

that works for me; however, there is still an issue with PSI (google pagespeed insights or lighthouse):

using wrapper with a dynamic inside client component with ssr:false

Route (app) Size First Load JS ┌ λ / 36.3 kB 152 kB └ λ /_not-found 883 B 81.5 kB

Screenshot from 2023-10-08 16-45-42

without an element at all:

Route (app) Size First Load JS ┌ λ / 35.8 kB 152 kB └ λ /_not-found 883 B 81.3 kB

Screenshot from 2023-10-08 16-53-22

without wrapper - using a dynamic in server component with ssr:false: Route (app) Size First Load JS ┌ λ / 244 kB 360 kB └ λ /_not-found 883 B 81.4 kB

Screenshot from 2023-10-08 16-29-27

marekmiotelka commented 1 year ago

it's kind of a joke that at least since May code splitting doesn't work in NextJS app directory. We are now on version 14.0.1, app router is recommended for all new applications since May, (v13.4.0 right?) and core features don't work at all (code splitting, cache & revalidation (fixed in 14.0.2-canary.3 tho, finally)). 2.4k open issues and Vercel is focusing on server actions that are just a sugar layer on top of the api routes. Like... seriously?

Let's imagine any app that has a CMS - currently EVERY SINGLE PAGE, no matter if renders 1 component or 100, gets the same, huge bundle with all components included.

Like seriously, using Next stopped to be fun with version 13, as most of the development time is spend on debugging the newly introduced bugs or discovering that core features that worked since v1.0.0 now just don't and no ones gives a s**t about it for half of the year. I get that you want to introduce new features, Server Actions looks nice, but for God's sake - first make sure the core features are working properly and issues relating core features are not left open for half of the year.

I don't know what are your processes of development, but would be nice to revisit them, as almost every new update introduces more bugs than it fixes.

App router is not stable and shouldn't be used by any project larger than a landing page.

PS: code split works fine with pages directory (v14.0.1).

SergiuGrisca commented 1 year ago

For me in the pages dir is not working even, or I don't understand something

how it is possible that lighthouse is complaining about not used js, if that component is with dynamic import and ssr: false ?

manuelseisl commented 1 year ago

Experiencing the same issue in app router. It's not possible to use app router for large applications, because pagespeed performance is really bad, when using many components.

levipadre commented 1 year ago

Same issue here. CMS, all content is dynamic, and the page is slow (not just page speed numbers, but visible too for the eyes). We need a solution for dynamic import, suspense, lazy load, file sizes

rharrisuk commented 1 year ago

Exact same issue here. This is preventing me from using Next in any production site and as I see it, is not fit for purpose.

This issue has been open over 6 months now and it would be nice to know if it isn't being worked on so I can look for an alternative framework. If it is then why the lack of communication?

huozhi commented 1 year ago

Hi, sorry for the late response, definitely missed the discussion here as there're too many issues atm. Wanna share a bit more that how can you do code-splitting better.

Solution

If we look at the reproduction case that shared in the issue description, it's using next/dynamic call in the server components, loading a client component. The quick way to make code splitting work is to use next/dynamic in client components.

page.js -> client component using `next/dynamic` -> module to load in splitting chunk

So you could either add a client component in between the module you're loading or mark the page.js as client component if that works for you.

Why

Calling next/dynamic with client component in server component is so far not able to do code splitting. It's like you're trying to code splitting in server components where you might not worry about the chunks size especially when you want those async loaded chunks to be loaded on client. So you need to move them to a client component and call next/dynamic there to split the module you'd like to separate into other chunks.

Using next/dynamic in server components that loading a client component is still including everything in server side rendering, and that client component doesn't know it needs to be split into another chunk in browser chunks.

We'll improve the education resource to make it more clear and any feedback to it is welcomed, thanks

manuelseisl commented 11 months ago

When server components are imported via next/dynamic, shouldn't the CSS be split into chunks?

The issue is that the CSS of all components is bundled into one CSS file. Thia leads to performance losses in Google PageSpeed Insights, especially in larger projects.

github-actions[bot] commented 11 months ago

This closed issue has been automatically locked because it had no new activity for 2 weeks. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.