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

fix(#39609): warns about suspense and ssr #39676

Merged
merged 5 commits into from Aug 18, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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()
})
})