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

[appDir] Dynamically importing a Client Component using the 'use client' directive from a Server Component causes "Error: object is not a function" #43147

Closed
1 task done
brvnonascimento opened this issue Nov 20, 2022 · 6 comments · Fixed by #42589
Assignees
Labels
area: app App directory (appDir: true) Lazy Loading Related to Next.js Lazy Loading (e.g., `next/dynamic` or `React.lazy`).

Comments

@brvnonascimento
Copy link
Contributor

Verify canary release

  • I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
  Platform: darwin
  Arch: arm64
  Version: Darwin Kernel Version 22.1.0: Sun Oct  9 20:14:30 PDT 2022; root:xnu-8792.41.9~2/RELEASE_ARM64_T8103
Binaries:
  Node: 14.18.0
  npm: 6.14.15
  Yarn: 1.22.17
  pnpm: 7.14.1
Relevant packages:
  next: 13.0.5-canary.3
  eslint-config-next: 13.0.5-canary.3
  react: 18.2.0
  react-dom: 18.2.0

What browser are you using? (if relevant)

Firefox 107.0

How are you deploying your application? (if relevant)

Vercel

Describe the Bug

Dynamically importing with next/dynamic a Client Component that uses a use/client directive will cause an Error: object is not a function error in development. The production build runs fine though.

Also noticed that adding ssr: false won't prevent the component from getting prerendered when running next build. Adding suspense: true or suspense: false won't make a difference either.

Expected Behavior

We should be able to dynamically import a component that makes heavy use of JavaScript in the client from a Server Component and use a custom Suspense-based fallback.

Link to reproduction - Issues with a link to complete (but minimal) reproduction code will be addressed faster

https://github.com/brvnonascimento/use-client-dynamic-bug

To Reproduce

  1. Run yarn create next-app --experimental-app
  2. Create a Client Component by adding the 'use client' directive
  3. Use next/dynamic to import the given component
  4. See Error: object is not a function in development
import dynamic from "next/dynamic";
import { Suspense } from "react";

const ClientTest = dynamic(() => import("./ClientTest"));

export default function Home() {
  return (
    <div>
      <Suspense fallback={<h1>Loading...</h1>}>
        <ClientTest />
      </Suspense>
    </div>
  );
}
Screen.Recording.2022-11-20.at.04.16.21.mov
@brvnonascimento brvnonascimento added the bug Issue was opened via the bug report template. label Nov 20, 2022
@brvnonascimento
Copy link
Contributor Author

Also the link of deployed build in Vercel proving that it works in production and also that is still prerendering even though ssr: false is being set.

https://use-client-dynamic-bug.vercel.app/

@brvnonascimento
Copy link
Contributor Author

After reading a bit more about this section of the React Server Components RFC I could check that this is possibly by design and that dynamic is not needed as bundle splitting is simplified.

I still think we could improve the error messaging here, will open a PR for this ASAP.

@balazsorban44 balazsorban44 added area: app App directory (appDir: true) Lazy Loading Related to Next.js Lazy Loading (e.g., `next/dynamic` or `React.lazy`). labels Nov 21, 2022
@balazsorban44
Copy link
Member

Correct, you won't need next/dynamic in this case, as splitting is going to happen automatically.

Although the error message is a bit unclear and we could improve it.

See also: https://beta.nextjs.org/docs/routing/loading-ui#manually-defining-suspense-boundaries

@balazsorban44 balazsorban44 added Webpack Related to Webpack with Next.js. bug Issue was opened via the bug report template. Developer Experience Issues related to Next.js logs, Error overlay, etc. and removed bug Issue was opened via the bug report template. labels Nov 21, 2022
@huozhi huozhi self-assigned this Nov 23, 2022
@huozhi
Copy link
Member

huozhi commented Nov 23, 2022

Hi @brvnonascimento , when you import a client component in server component, it will become a module reference which is the "object" that marked in the error. But as @balazsorban44 mentioned above, you don't actually need to do in that way you can just move the part you don't want to have in client bundle to server components then leave the rest to client components, and directly importing them will be fine.

Will improve the error for it

@huozhi
Copy link
Member

huozhi commented Dec 6, 2022

After discussion, this case should be able to support, the effect is client will be split into different chunk but still be loadable throught this way. I'm addressing the support for this in #42589

huozhi added a commit that referenced this issue Dec 7, 2022
### Summary

Migrate `next/dynamic` to implementation based on `React.lazy` and
`Suspense`. Then it becomes easier to migrate the existing code in pages
to layouts. Then we can support both `ssr` and `loading` option for
`next/dynamic`.

For `loading` option, it will work like `Suspense`'s `fallback` property

```js
<Suspense fallback={loading}>
  <DynamicComponent />
 </Suspense>
```

For `ssr` option, by default `React.lazy` supports SSR, but we'll
disable the `ssr: false` case for dynamic import in server components
since there's no client side involved.

Then we don't need `suspense` option anymore as react >= 18 is always
required. Mark it as deprecated.

It also supports to load client component dynamically in server
components now.

#### Code code changes
* switch loadable component to `lazy` + `Suspense`
* will make sure it's retuning a module from `loader()` to
`loader().then(mod => ({ default: mod.default || mod }))` since `lazy()`
only accepts loader returning a module
* Inside suspense boundary, throwing an error for ssr: false, catch the
error on server and client side and ignore it.
* Ignore options like ssr: false for server components since they're on
server, doesn't make sense
* Remove legacy dynamic related transform
#### Feature changes
* `next/dynamic` will work in the same way across the board (appDir and
pages)
* For the throwing error, will make it become a API that throws error
later in the future, so users can customize more with `Suspense`
* You can load client components now in server components with dynamic.
Resolves #43147

#### Tests
* existing dynamic tests all work
* add case: import client component and load through next/dynamic in
server components

### Issues
@github-actions
Copy link
Contributor

github-actions bot commented Jan 7, 2023

This closed issue has been automatically locked because it had no new activity for a month. 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 Jan 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: app App directory (appDir: true) Lazy Loading Related to Next.js Lazy Loading (e.g., `next/dynamic` or `React.lazy`).
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants