Skip to content

Commit

Permalink
fix(#39609): warns about suspense and ssr (#39676)
Browse files Browse the repository at this point in the history
Currently, `next/dynamic` will opt-in to `React.lazy` if `{ suspense: true }` is used. And React 18 will always resolve the `Suspense` boundary on the server-side, effectively ignoring the `ssr` option.

The PR fixes #39609 by showing a warning message when `{ suspense: true, ssr: false }` is detected. The error documentation and the corresponding test case has also been updated.

In the future, Next.js could implement a custom version of `React.lazy` that could suspense without executing the lazy-loaded component on the server-side.

cc @huozhi 

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [x] Errors have helpful link attached, see `contributing.md`
  • Loading branch information
SukkaW committed Aug 18, 2022
1 parent 78aefee commit 5360440
Show file tree
Hide file tree
Showing 6 changed files with 108 additions and 57 deletions.
22 changes: 20 additions & 2 deletions errors/invalid-dynamic-suspense.md
Expand Up @@ -2,11 +2,29 @@

#### Why This Error Occurred

`<Suspense>` is not allowed under legacy render mode when using React older than v18.
- You are using `{ suspense: true }` with React version older than 18.
- You are using `{ suspense: true, ssr: false }`.
- You are using `{ suspense: true, loading }`.

#### Possible Ways to Fix It

Remove `suspense: true` option in `next/dynamic` usages.
**If you are using `{ suspense: true }` with React version older than 18**

- You can try upgrading to React 18 or newer
- If upgrading React is not an option, remove `{ suspense: true }` from `next/dynamic` usages.

**If you are using `{ suspense: true, ssr: false }`**

Next.js will use `React.lazy` when `suspense` is set to true. React 18 or newer will always try to resolve the Suspense boundary on the server. This behavior can not be disabled, thus the `ssr: false` is ignored with `suspense: true`.

- You should write code that works in both client-side and server-side.
- If rewriting the code is not an option, remove `{ suspense: true }` from `next/dynamic` usages.

**If you are using `{ suspense: true, loading }`**

Next.js will use `React.lazy` when `suspense` is set to true, when your dynamic-imported component is loading, React will use the closest suspense boundary's fallback.

You should remove `loading` from `next/dynamic` usages, and use `<Suspense />`'s `fallback` prop.

### Useful Links

Expand Down
25 changes: 25 additions & 0 deletions packages/next/shared/lib/dynamic.tsx
Expand Up @@ -112,6 +112,31 @@ export default function dynamic<P = {}>(
)
}

if (process.env.NODE_ENV !== 'production') {
if (loadableOptions.suspense) {
/**
* TODO: Currently, next/dynamic will opt-in to React.lazy if { suspense: true } is used
* React 18 will always resolve the Suspense boundary on the server-side, effectively ignoring the ssr option
*
* In the future, when React Suspense with third-party libraries is stable, we can implement a custom version of
* React.lazy that can suspense on the server-side while only loading the component on the client-side
*/
if (loadableOptions.ssr === false) {
loadableOptions.ssr = true
console.warn(
`"ssr: false" is ignored by next/dynamic because you can not enable "suspense" while disabling "ssr" at the same time. Read more: https://nextjs.org/docs/messages/invalid-dynamic-suspense`
)
}

if (loadableOptions.loading != null) {
loadableOptions.loading = undefined
console.warn(
`"loading" is ignored by next/dynamic because you have enabled "suspense". Place your loading element in your suspense boundary's "fallback" prop instead. Read more: https://nextjs.org/docs/messages/invalid-dynamic-suspense`
)
}
}
}

// coming from build/babel/plugins/react-loadable-plugin.js
if (loadableOptions.loadableGenerated) {
loadableOptions = {
Expand Down
55 changes: 0 additions & 55 deletions test/e2e/dynamic-with-suspense/index.test.ts

This file was deleted.

19 changes: 19 additions & 0 deletions test/integration/next-dynamic-with-suspense/pages/index.js
@@ -0,0 +1,19 @@
import { Suspense } from 'react'
import dynamic from 'next/dynamic'

const Thing = dynamic(() => import('./thing'), {
ssr: false,
suspense: true,
loading: () => 'Loading...',
})

export default function IndexPage() {
return (
<div>
<p>Next.js Example</p>
<Suspense fallback="Loading...">
<Thing />
</Suspense>
</div>
)
}
3 changes: 3 additions & 0 deletions test/integration/next-dynamic-with-suspense/pages/thing.js
@@ -0,0 +1,3 @@
export default function Thing() {
return 'Thing'
}
41 changes: 41 additions & 0 deletions test/integration/next-dynamic-with-suspense/test/index.test.ts
@@ -0,0 +1,41 @@
/* eslint-env jest */

import webdriver from 'next-webdriver'
import { join } from 'path'
import {
renderViaHTTP,
findPort,
launchApp,
killApp,
hasRedbox,
} from 'next-test-utils'

let app
let appPort: number
const appDir = join(__dirname, '../')

describe('next/dynamic with suspense', () => {
beforeAll(async () => {
appPort = await findPort()
app = await launchApp(appDir, appPort)
})
afterAll(() => killApp(app))

it('should render server-side', async () => {
const html = await renderViaHTTP(appPort, '/')
expect(html).toContain('Next.js Example')
expect(html).toContain('Thing')
})

it('should render client-side', async () => {
const browser = await webdriver(appPort, '/')
const warnings = (await browser.log()).map((log) => log.message).join('\n')

expect(await hasRedbox(browser)).toBe(false)
expect(warnings).toMatch(
/"ssr: false" is ignored by next\/dynamic because you can not enable "suspense" while disabling "ssr" at the same time/gim
)

await browser.close()
})
})

0 comments on commit 5360440

Please sign in to comment.