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

next v14.2.8 with next/dynamic component breaks app #69720

Closed
ovidiuwin opened this issue Sep 5, 2024 · 10 comments
Closed

next v14.2.8 with next/dynamic component breaks app #69720

ovidiuwin opened this issue Sep 5, 2024 · 10 comments
Assignees
Labels
bug Issue was opened via the bug report template. create-next-app Related to our CLI tool for quickly starting a new Next.js application. Developer Experience Issues related to Next.js logs, Error overlay, etc. locked Module Resolution Module resolution (CJS / ESM, module resolving). Runtime Related to Node.js or Edge Runtime with Next.js.

Comments

@ovidiuwin
Copy link

ovidiuwin commented Sep 5, 2024

Link to the code that reproduces this issue

https://codesandbox.io/p/devbox/j6yc57

To Reproduce

  1. Open the codesandbox link
  2. Go to page.tsx file and check the next/dynamic import
  3. Go to the actual component, you will see that it's marked as 'use client' and it uses a simple useState hook
  4. Check the error on the screen
  5. Remove next/dynamic from page.tsx and import it as regular component, error is gone

Current vs. Expected behavior

I expected that next/dynamic component would work since prior to this nextjs version it did

Provide environment information

Operating System:
  Platform: linux
  Arch: x64
  Version: #1 SMP PREEMPT_DYNAMIC Sun Aug  6 20:05:33 UTC 2023
  Available memory (MB): 4102
  Available CPU cores: 2
Binaries:
  Node: 20.9.0
  npm: 9.8.1
  Yarn: 1.22.19
  pnpm: 8.10.2
Relevant Packages:
  next: 14.2.8 // Latest available version is detected (14.2.8).
  eslint-config-next: 14.2.1
  react: 18.3.1
  react-dom: 18.3.1
  typescript: 5.5.4
Next.js Config:
  output: N/A

Which area(s) are affected? (Select all that apply)

create-next-app, Developer Experience, Module Resolution, Runtime

Which stage(s) are affected? (Select all that apply)

next dev (local), next start (local)

Additional context

No response

@ovidiuwin ovidiuwin added the bug Issue was opened via the bug report template. label Sep 5, 2024
@github-actions github-actions bot added create-next-app Related to our CLI tool for quickly starting a new Next.js application. Developer Experience Issues related to Next.js logs, Error overlay, etc. Module Resolution Module resolution (CJS / ESM, module resolving). Runtime Related to Node.js or Edge Runtime with Next.js. labels Sep 5, 2024
@AlexHashgraph
Copy link

Encountered this too. Looking for a resolution

@lubieowoce lubieowoce self-assigned this Sep 5, 2024
@lubieowoce
Copy link
Member

lubieowoce commented Sep 5, 2024

Hi! Sorry about that -- definitely shouldn't break things like this in a patch release! I'm looking into what's causing this. AFAICT, it's specific to a server component (e.g. page.js) wrapping a client module in dynamic(), but i'm not 100% sure yet.

In the meantime, as a workaround, try the following:

If your module has a default export, there's no need to do import("../components/Test").then((module) => module.default), it's enough to just do import("../components/Test"):

// default export - no need to `.then` at all
const TestDynamicComp = dynamic(() => import("../components/Test"), {
  ssr: false,
});

(Technically, import("../components/Test").then((module) => ({ default: module.default }) works too, it's just redundant)

You only need to do something with the module if you're trying to use a named export:

// named export - need to wrap in `{ default: }`
const TestDynamicComp = dynamic(
  () =>
    import("../components/Test").then((module) => {
      return { default: module.TestComp };
    }),
  {
    ssr: false,
  }
);

I've tested these in your repro and both forms work for me on 14.2.8.

@huozhi
Copy link
Member

huozhi commented Sep 5, 2024

Technically it's not really doing ssr: false dynamic import of client components in server components. We're thinking to disable the usage for it as it's not really doing the code splitting like it's in pages router or app router client components (only SSR).

If you're looking better code splitting with next/dynamic, you can move it into a client component and call next/dynamic there. Then import that client component into the server component page or layout.

In your case, you might want to move the dynamic component into another client component file.

// client-dynamic-compo.js
'use client' 
export const TestDynamicComp = dynamic(
  () => import("../components/Test").then((module) => module.default),
  {
    ssr: false,
  }
);

Then use the dynamic-comp.js into the server component.

// page.js

import { TestDynamicComp } from './client-dynamic-comp'

<TestDynamicComp />

This will have less impact for the future updates and really using the code splitting ability of next/dynamic

@a88zach
Copy link

a88zach commented Sep 5, 2024

This solution worked for me: #47316

lubieowoce pushed a commit that referenced this issue Sep 6, 2024
This reverts commit 9bb06c5.

This was a sort breaking change that could effect using `next/dynamic`
in server component, when the imported one is client component. Ideally
you shouldn't directly return it due to the property access restriction
for client reference. We've already provided related codemod for v15 but
this might be a bit surprise for users upgrading to the new patch.

Reverted as it's not necessarily part of ESM externals fixes.

Fixes #69720
@borisghidaglia
Copy link

Hey @huozhi 👋

Technically it's not really doing ssr: false dynamic import of client components in server components. We're thinking to disable the usage for it as it's not really doing the code splitting like it's in pages router or app router client components (only SSR).

I'm not sure I understand this correctly.
Do you mean that doing the following does not in fact disable SSR if Component is a client component and that we are doing this from a server component?

const DynamicComponent = dynamic(() => import('./Component'), {
  ssr: false,
});

I had some issues with dynamic.
I couldn't build my project because of ReferenceError: window is not defined .

When running the project in dev, I had a 500 with the same error, but the page rendered fine.
I think its because in dev Next.js falls back to "client rendering only" just like when there is a hydration mismatch, but I'm not sure.

When I implemented what your message suggests with TestDynamicComp, the build worked and dev wasn't throwing the error anymore.

@Stewart86
Copy link

Because I have used dynamic imports to reduce bundle size to the entire project previously, this 'bug' means I have to almost entirely overhaul my entire project server / client structure. Can there be a clear guideline on how it should be done moving forward? Not looking at quick fixes or workaround here, as I would have to edit a lot of files.

@huozhi
Copy link
Member

huozhi commented Sep 9, 2024

@borisghidaglia when you use next/dynamic with ssr: false option to only load a component in browser, you need to put the next/dynamic call in a client components, otherwise it will work do SSR. We're considering in the next major version, disable ssr: false for server components to help users to render non-SSR component correctly. It's a different case from this issue but sth we'd love to improve on the DX.

@huozhi
Copy link
Member

huozhi commented Sep 9, 2024

Regarding to this issue, I had a fix #69749 to change the behavior back as and release a patch soon. Since the pattern of "using next/dynamic to import a client component in a server component" is not recommended, we'll recommend you to migrate later. The behavior will be changed in v15 but not on v14.2.x.

@huozhi
Copy link
Member

huozhi commented Sep 9, 2024

This is fixed in patch release 14.2.9, please upgrade Next.js to that version, thanks 🙏

Copy link
Contributor

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue was opened via the bug report template. create-next-app Related to our CLI tool for quickly starting a new Next.js application. Developer Experience Issues related to Next.js logs, Error overlay, etc. locked Module Resolution Module resolution (CJS / ESM, module resolving). Runtime Related to Node.js or Edge Runtime with Next.js.
Projects
None yet
Development

No branches or pull requests

7 participants