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
Enable pure client suspense in blocking rendering #28165
Merged
Merged
Changes from 2 commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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> | ||
) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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' | ||
|
@@ -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' | ||
) | ||
}) | ||
}) | ||
|
||
|
@@ -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', () => { | ||
|
@@ -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)) | ||
) | ||
}) | ||
|
@@ -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 () => { | ||
|
@@ -186,3 +192,8 @@ function runTests(name, mode, fn) { | |
fn(context) | ||
}) | ||
} | ||
|
||
function runTests(name, fn) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. other test changes are just reflected to this simplification |
||
runTest('dev', name, fn) | ||
runTest('prod', name, fn) | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the only core change is here, using suspense + react.lazy won't have preload ids like loadable components.