Skip to content

Commit

Permalink
Remove extra suspense boundary for default next/dynamic (#64716)
Browse files Browse the repository at this point in the history
### What

Removing the Suspense boundary on top of `next/dynamic` by default, make
it as `React.lazy` component with preloading CSS feature.

### Why

Extra Suspense boundary is causing extra useless rendering. For SSR, it
shouldn't render `loading` by default

Related: #64060
Related: #64687
Closes
[NEXT-3074](https://linear.app/vercel/issue/NEXT-3074/app-router-content-flickering-with-reactcreatecontext-and-nextdynamic)

This is sort of a breaking change, since removing the Suspense boundary
on top of `next/dynamic` by default. If there's error happening in side
the dynamic component you need to wrap an extra Suspense boundary on top
of it
  • Loading branch information
huozhi committed Jun 17, 2024
1 parent 13f7ad3 commit 3a67a07
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 4 deletions.
1 change: 1 addition & 0 deletions packages/next/src/shared/lib/dynamic.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export type DynamicOptions<P = {}> = LoadableGeneratedOptions & {
loadableGenerated?: LoadableGeneratedOptions
ssr?: boolean
/**
* TODO: remove this in next major version v15
* @deprecated `suspense` prop is not required anymore
*/
suspense?: boolean
Expand Down
9 changes: 6 additions & 3 deletions packages/next/src/shared/lib/lazy-dynamic/loadable.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Suspense, lazy } from 'react'
import { Suspense, Fragment, lazy } from 'react'
import { BailoutToCSR } from './dynamic-bailout-to-csr'
import type { ComponentModule } from './types'
import { PreloadChunks } from './preload-chunks'
Expand Down Expand Up @@ -48,7 +48,10 @@ function Loadable(options: LoadableOptions) {
<Loading isLoading={true} pastDelay={true} error={null} />
) : null

const children = opts.ssr ? (
const isSSR = opts.ssr
const Wrap = isSSR ? Fragment : Suspense
const wrapProps = isSSR ? {} : { fallback: fallbackElement }
const children = isSSR ? (
<>
{/* During SSR, we need to preload the CSS from the dynamic component to avoid flash of unstyled content */}
{typeof window === 'undefined' ? (
Expand All @@ -62,7 +65,7 @@ function Loadable(options: LoadableOptions) {
</BailoutToCSR>
)

return <Suspense fallback={fallbackElement}>{children}</Suspense>
return <Wrap {...wrapProps}>{children}</Wrap>
}

LoadableComponent.displayName = 'LoadableComponent'
Expand Down
12 changes: 12 additions & 0 deletions test/e2e/app-dir/dynamic/app/default/dynamic-component.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
const isDevTest = false

const DynamicImportComponent = () => {
if (isDevTest && typeof window === 'undefined') {
throw new Error('This component should only be rendered on the client side')
}
return (
<div id="dynamic-component">This is a dynamically imported component</div>
)
}

export default DynamicImportComponent
17 changes: 17 additions & 0 deletions test/e2e/app-dir/dynamic/app/default/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
'use client'

import dynamic from 'next/dynamic'

const DynamicHeader = dynamic(() => import('./dynamic-component'), {
loading: () => <p>Loading...</p>,
})

const ClientWrapper = () => {
return (
<div>
<DynamicHeader />
</div>
)
}

export default ClientWrapper
23 changes: 22 additions & 1 deletion test/e2e/app-dir/dynamic/dynamic.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { nextTestSetup } from 'e2e-utils'
import { retry } from 'next-test-utils'

describe('app dir - next/dynamic', () => {
const { next, isNextStart, skipped } = nextTestSetup({
const { next, isNextStart, isNextDev, skipped } = nextTestSetup({
files: __dirname,
skipDeployment: true,
})
Expand Down Expand Up @@ -57,6 +58,26 @@ describe('app dir - next/dynamic', () => {
expect($('h1').text()).toBe('hello')
})

it('should not render loading by default', async () => {
const $ = await next.render$('/default')
expect($('#dynamic-component').text()).not.toContain('loading')
})

if (isNextDev) {
it('should directly raise error when dynamic component error on server', async () => {
const pagePath = 'app/default/dynamic-component.js'
const page = await next.readFile(pagePath)
await next.patchFile(
pagePath,
page.replace('const isDevTest = false', 'const isDevTest = true')
)
await retry(async () => {
const { status } = await next.fetch('/default')
expect(status).toBe(500)
})
})
}

describe('no SSR', () => {
it('should not render client component imported through ssr: false in client components in edge runtime', async () => {
// noSSR should not show up in html
Expand Down

0 comments on commit 3a67a07

Please sign in to comment.