From 5b12dd53d9b404c55b193da32b498ab87b8980e2 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Sat, 14 Aug 2021 23:40:33 +0800 Subject: [PATCH 1/2] Error on suspense in blocking rendering mode --- packages/next/shared/lib/dynamic.tsx | 14 +++------ .../react-18/app/pages/suspense/unwrapped.js | 10 ------ test/integration/react-18/test/basics.js | 31 ------------------- test/integration/react-18/test/blocking.js | 27 ---------------- test/integration/react-18/test/index.test.js | 28 +++-------------- 5 files changed, 9 insertions(+), 101 deletions(-) delete mode 100644 test/integration/react-18/app/pages/suspense/unwrapped.js delete mode 100644 test/integration/react-18/test/basics.js delete mode 100644 test/integration/react-18/test/blocking.js diff --git a/packages/next/shared/lib/dynamic.tsx b/packages/next/shared/lib/dynamic.tsx index ba5871238998..967901839267 100644 --- a/packages/next/shared/lib/dynamic.tsx +++ b/packages/next/shared/lib/dynamic.tsx @@ -114,15 +114,11 @@ export default function dynamic

( loadableOptions = { ...loadableOptions, ...options } const suspenseOptions = loadableOptions as LoadableSuspenseOptions

- if (!process.env.__NEXT_CONCURRENT_FEATURES) { - // Error if react root is not enabled and `suspense` option is set to true - if (!process.env.__NEXT_REACT_ROOT && suspenseOptions.suspense) { - // TODO: add error doc when this feature is stable - throw new Error( - `Disallowed suspense option usage with next/dynamic in blocking mode` - ) - } - suspenseOptions.suspense = false + if (!process.env.__NEXT_CONCURRENT_FEATURES && suspenseOptions.suspense) { + // TODO: add error doc when this feature is stable + throw new Error( + `Disallowed suspense option usage with next/dynamic in blocking mode` + ) } if (suspenseOptions.suspense) { return loadableFn(suspenseOptions) diff --git a/test/integration/react-18/app/pages/suspense/unwrapped.js b/test/integration/react-18/app/pages/suspense/unwrapped.js deleted file mode 100644 index 3909e80679a6..000000000000 --- a/test/integration/react-18/app/pages/suspense/unwrapped.js +++ /dev/null @@ -1,10 +0,0 @@ -import React from 'react' -import dynamic from 'next/dynamic' - -const Hello = dynamic(() => import('../../components/hello'), { - suspense: true, -}) - -export default function Unwrapped() { - return -} diff --git a/test/integration/react-18/test/basics.js b/test/integration/react-18/test/basics.js deleted file mode 100644 index bb3c4ad99ced..000000000000 --- a/test/integration/react-18/test/basics.js +++ /dev/null @@ -1,31 +0,0 @@ -/* eslint-env jest */ - -import webdriver from 'next-webdriver' -import cheerio from 'cheerio' -import { fetchViaHTTP, renderViaHTTP } from 'next-test-utils' - -export default (context) => { - it('hydrates correctly for normal page', async () => { - const browser = await webdriver(context.appPort, '/') - expect(await browser.eval('window.didHydrate')).toBe(true) - expect(await browser.elementById('react-dom-version').text()).toMatch(/18/) - }) - - it('should works with suspense in ssg', async () => { - const res1 = await fetchViaHTTP(context.appPort, '/suspense/thrown') - const res2 = await fetchViaHTTP(context.appPort, '/suspense/no-thrown') - - expect(res1.status).toBe(200) - expect(res2.status).toBe(200) - }) - - it('should render fallback without preloads on server side', async () => { - const html = await renderViaHTTP(context.appPort, '/suspense/no-preload') - const $ = cheerio.load(html) - const nextData = JSON.parse($('#__NEXT_DATA__').text()) - const content = $('#__next').text() - // is suspended - expect(content).toBe('rab') - expect(nextData.dynamicIds).toBeUndefined() - }) -} diff --git a/test/integration/react-18/test/blocking.js b/test/integration/react-18/test/blocking.js deleted file mode 100644 index 3adb476c5a01..000000000000 --- a/test/integration/react-18/test/blocking.js +++ /dev/null @@ -1,27 +0,0 @@ -/* eslint-env jest */ - -import cheerio from 'cheerio' - -export default (context, render) => { - async function get$(path, query) { - const html = await render(path, query) - return cheerio.load(html) - } - - it('should render fallback on server side if suspense without preload', async () => { - const $ = await get$('/suspense/no-preload') - const nextData = JSON.parse($('#__NEXT_DATA__').text()) - const content = $('#__next').text() - expect(content).toBe('rab') - expect(nextData.dynamicIds).toBeUndefined() - }) - - it('should render fallback on server side if suspended on server with preload', async () => { - const $ = await get$('/suspense/thrown') - const html = $('body').html() - expect(html).toContain('loading') - expect( - JSON.parse($('#__NEXT_DATA__').text()).dynamicIds - ).not.toBeUndefined() - }) -} diff --git a/test/integration/react-18/test/index.test.js b/test/integration/react-18/test/index.test.js index d0f889adcccd..e4cd2225bf52 100644 --- a/test/integration/react-18/test/index.test.js +++ b/test/integration/react-18/test/index.test.js @@ -12,9 +12,7 @@ import { nextStart, renderViaHTTP, } from 'next-test-utils' -import blocking from './blocking' import concurrent from './concurrent' -import basics from './basics' jest.setTimeout(1000 * 60 * 5) @@ -80,14 +78,6 @@ describe('React 18 Support', () => { expect(output).not.toMatch(USING_CREATE_ROOT) 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) - }) }) describe('warns with stable supported version of react-dom', () => { @@ -117,13 +107,6 @@ 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) - ) -}) - describe('Blocking mode', () => { beforeAll(() => { dynamicHello.replace('suspense = false', `suspense = true`) @@ -132,13 +115,10 @@ 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) => - blocking(context, (p, q) => renderViaHTTP(context.appPort, p, q)) - ) + it('suspense is not allowed in blocking rendering mode (prod)', async () => { + const output = await getBuildOutput(appDir) + expect(output).toContain(SUSPENSE_ERROR_MESSAGE) + }) }) describe('Concurrent mode', () => { From 3fa0a5233c194c4191472ca9ad716fc2dc62304f Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Sun, 15 Aug 2021 11:50:59 +0800 Subject: [PATCH 2/2] simplify loadable component --- packages/next/shared/lib/loadable.js | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/packages/next/shared/lib/loadable.js b/packages/next/shared/lib/loadable.js index fbeaf9480f6f..6e3fee37504c 100644 --- a/packages/next/shared/lib/loadable.js +++ b/packages/next/shared/lib/loadable.js @@ -71,8 +71,14 @@ function createLoadableComponent(loadFn, options) { options ) + let lazyComponent if (opts.suspense) { - opts.lazy = React.lazy(opts.loader) + lazyComponent = React.lazy(opts.loader) + return LazyImpl + } + + function LazyImpl(props) { + return React.createElement(lazyComponent, props) } let subscription = null @@ -90,7 +96,7 @@ function createLoadableComponent(loadFn, options) { } // Server only - if (typeof window === 'undefined' && !opts.suspense) { + if (typeof window === 'undefined') { ALL_INITIALIZERS.push(init) } @@ -99,8 +105,7 @@ function createLoadableComponent(loadFn, options) { !initialized && typeof window !== 'undefined' && typeof opts.webpack === 'function' && - typeof require.resolveWeak === 'function' && - !opts.suspense + typeof require.resolveWeak === 'function' ) { const moduleIds = opts.webpack() READY_INITIALIZERS.push((ids) => { @@ -148,12 +153,8 @@ function createLoadableComponent(loadFn, options) { }, [props, state]) } - function LazyImpl(props, ref) { - return React.createElement(opts.lazy, { ...props, ref }) - } - - const LoadableComponent = opts.suspense ? LazyImpl : LoadableImpl - LoadableComponent.preload = () => !opts.suspense && init() + const LoadableComponent = LoadableImpl + LoadableComponent.preload = () => init() LoadableComponent.displayName = 'LoadableComponent' return React.forwardRef(LoadableComponent)