Skip to content

Commit

Permalink
Enable pure client suspense in blocking rendering (#28165)
Browse files Browse the repository at this point in the history
Do not fallback to loadable component when `reactRoot` is enabled but without `concurrentFeatures`. Thus we can enable pure client suspense:

`fallback` is always rendered on server side, but client side can work with fully functional suspense.

Closes #28117
  • Loading branch information
huozhi committed Aug 16, 2021
1 parent 49c3e0a commit 5313426
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 28 deletions.
1 change: 0 additions & 1 deletion packages/next/shared/lib/dynamic.tsx
Expand Up @@ -122,7 +122,6 @@ export default function dynamic<P = {}>(
`Disallowed suspense option usage with next/dynamic in blocking mode`
)
}
suspenseOptions.suspense = false
}
if (suspenseOptions.suspense) {
return loadableFn(suspenseOptions)
Expand Down
13 changes: 11 additions & 2 deletions test/integration/react-18/app/pages/suspense/unwrapped.js
@@ -1,10 +1,19 @@
import React from 'react'
import React, { Suspense } from 'react'
import dynamic from 'next/dynamic'

// flag for testing
const wrapped = true

const Hello = dynamic(() => import('../../components/hello'), {
suspense: true,
})

export default function Unwrapped() {
return <Hello />
if (!wrapped) return <Hello />

return (
<Suspense fallback={`loading`}>
<Hello />
</Suspense>
)
}
4 changes: 1 addition & 3 deletions test/integration/react-18/test/blocking.js
Expand Up @@ -20,8 +20,6 @@ export default (context, render) => {
const $ = await get$('/suspense/thrown')
const html = $('body').html()
expect(html).toContain('loading')
expect(
JSON.parse($('#__NEXT_DATA__').text()).dynamicIds
).not.toBeUndefined()
expect(JSON.parse($('#__NEXT_DATA__').text()).dynamicIds).toBeUndefined()
})
}
55 changes: 33 additions & 22 deletions test/integration/react-18/test/index.test.js
Expand Up @@ -23,9 +23,8 @@ const nodeArgs = ['-r', join(__dirname, 'require-hook.js')]
const appDir = join(__dirname, '../app')
const nextConfig = new File(join(appDir, 'next.config.js'))
const dynamicHello = new File(join(appDir, 'components/dynamic-hello.js'))
const unwrappedPage = new File(join(appDir, 'pages/suspense/unwrapped.js'))

const SUSPENSE_ERROR_MESSAGE =
'Disallowed suspense option usage with next/dynamic'
const UNSUPPORTED_PRERELEASE =
"You are using an unsupported prerelease of 'react-dom'"
const USING_CREATE_ROOT = 'Using the createRoot API for React'
Expand Down Expand Up @@ -81,12 +80,15 @@ describe('React 18 Support', () => {
expect(output).not.toMatch(UNSUPPORTED_PRERELEASE)
})

it('suspense is not allowed in blocking rendering mode', async () => {
const appPort = await findPort()
const app = await launchApp(appDir, appPort)
const html = await renderViaHTTP(appPort, '/suspense/unwrapped')
await killApp(app)
expect(html).toContain(SUSPENSE_ERROR_MESSAGE)
test('suspense is not allowed in blocking rendering mode (prod)', async () => {
const { stderr, code } = await nextBuild(appDir, [], {
nodeArgs,
stderr: true,
})
expect(code).toBe(1)
expect(stderr).toContain(
'Disallowed suspense option usage with next/dynamic'
)
})
})

Expand Down Expand Up @@ -118,10 +120,21 @@ describe('React 18 Support', () => {
})

describe('Basics', () => {
runTests('default setting with react 18', 'dev', (context) => basics(context))
runTests('default setting with react 18', 'prod', (context) =>
basics(context)
)
runTests('default setting with react 18', (context) => basics(context))

it('suspense is not allowed in blocking rendering mode (dev)', async () => {
// set dynamic.suspense = true but not wrapping with <Suspense>
unwrappedPage.replace('wrapped = true', 'wrapped = false')
const appPort = await findPort()
const app = await launchApp(appDir, appPort, { nodeArgs })
const html = await renderViaHTTP(appPort, '/suspense/unwrapped')
unwrappedPage.restore()
await killApp(app)
// expect(html).toContain('Disallowed suspense option usage with next/dynamic')
expect(html).toContain(
'A React component suspended while rendering, but no fallback UI was specified'
)
})
})

describe('Blocking mode', () => {
Expand All @@ -132,11 +145,7 @@ describe('Blocking mode', () => {
dynamicHello.restore()
})

runTests('concurrentFeatures is disabled', 'dev', (context) =>
blocking(context, (p, q) => renderViaHTTP(context.appPort, p, q))
)

runTests('concurrentFeatures is disabled', 'prod', (context) =>
runTests('concurrentFeatures is disabled', (context) =>
blocking(context, (p, q) => renderViaHTTP(context.appPort, p, q))
)
})
Expand All @@ -156,15 +165,12 @@ describe('Concurrent mode', () => {
dynamicHello.restore()
})

runTests('concurrentFeatures is enabled', 'dev', (context) =>
concurrent(context, (p, q) => renderViaHTTP(context.appPort, p, q))
)
runTests('concurrentFeatures is enabled', 'prod', (context) =>
runTests('concurrentFeatures is enabled', (context) =>
concurrent(context, (p, q) => renderViaHTTP(context.appPort, p, q))
)
})

function runTests(name, mode, fn) {
function runTest(mode, name, fn) {
const context = { appDir }
describe(`${name} (${mode})`, () => {
beforeAll(async () => {
Expand All @@ -186,3 +192,8 @@ function runTests(name, mode, fn) {
fn(context)
})
}

function runTests(name, fn) {
runTest('dev', name, fn)
runTest('prod', name, fn)
}

0 comments on commit 5313426

Please sign in to comment.