From d71d84c438f587a26b02b9973f030e98e9890194 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Mon, 7 Nov 2022 16:41:23 +0100 Subject: [PATCH 01/15] Implement loadable with lazy and suspense for next dynamic remove dynamic alias for app dir remove legacy test --- packages/next/build/webpack-config.ts | 18 ----- packages/next/client/components/dynamic.tsx | 19 ----- packages/next/shared/lib/dynamic.tsx | 76 ++++++------------- packages/next/shared/lib/loadable-context.ts | 2 + packages/next/shared/lib/loadable.js | 61 +++++---------- .../test/index.test.js | 8 +- .../next-dynamic-with-suspense/pages/index.js | 19 ----- .../next-dynamic-with-suspense/pages/thing.js | 3 - .../test/index.test.ts | 41 ---------- .../next-dynamic/test/index.test.js | 4 +- 10 files changed, 52 insertions(+), 199 deletions(-) delete mode 100644 packages/next/client/components/dynamic.tsx delete mode 100644 test/integration/next-dynamic-with-suspense/pages/index.js delete mode 100644 test/integration/next-dynamic-with-suspense/pages/thing.js delete mode 100644 test/integration/next-dynamic-with-suspense/test/index.test.ts diff --git a/packages/next/build/webpack-config.ts b/packages/next/build/webpack-config.ts index 650a6b31d4468bb..fac7fe4b5a787c1 100644 --- a/packages/next/build/webpack-config.ts +++ b/packages/next/build/webpack-config.ts @@ -1707,26 +1707,8 @@ export default async function getBaseWebpackConfig( }, ] : []), - // Alias `next/dynamic` to React.lazy implementation for RSC ...(hasServerComponents ? [ - { - test: codeCondition.test, - issuerLayer(layer: string) { - return ( - layer === WEBPACK_LAYERS.client || - layer === WEBPACK_LAYERS.server - ) - }, - resolve: { - alias: { - // Alias `next/dynamic` to React.lazy implementation for RSC - [require.resolve('next/dynamic')]: require.resolve( - 'next/dist/client/components/dynamic' - ), - }, - }, - }, { // Alias react-dom for ReactDOM.preload usage. // Alias react for switching between default set and share subset. diff --git a/packages/next/client/components/dynamic.tsx b/packages/next/client/components/dynamic.tsx deleted file mode 100644 index 4c3f9c82781984a..000000000000000 --- a/packages/next/client/components/dynamic.tsx +++ /dev/null @@ -1,19 +0,0 @@ -import React from 'react' - -export type LoaderComponent

= Promise<{ - default: React.ComponentType

-}> - -export type Loader

= () => LoaderComponent

- -export type DynamicOptions

= { - loader?: Loader

-} - -export type LoadableComponent

= React.ComponentType

- -export default function dynamic

( - loader: Loader

-): React.ComponentType

{ - return React.lazy(loader) -} diff --git a/packages/next/shared/lib/dynamic.tsx b/packages/next/shared/lib/dynamic.tsx index 1cf5d213b73d1ef..84d3853eb09a649 100644 --- a/packages/next/shared/lib/dynamic.tsx +++ b/packages/next/shared/lib/dynamic.tsx @@ -29,6 +29,9 @@ export type DynamicOptions

= LoadableGeneratedOptions & { loader?: Loader

| LoaderMap loadableGenerated?: LoadableGeneratedOptions ssr?: boolean + /** + * @deprecated `suspense` prop is not required any more + */ suspense?: boolean } @@ -66,31 +69,27 @@ export default function dynamic

( ): React.ComponentType

{ let loadableFn: LoadableFn

= Loadable - let loadableOptions: LoadableOptions

= options?.suspense - ? {} - : // only provide a default loading component when suspense is disabled - { - // A loading component is not required, so we default it - loading: ({ error, isLoading, pastDelay }) => { - if (!pastDelay) return null - if (process.env.NODE_ENV === 'development') { - if (isLoading) { - return null - } - if (error) { - return ( -

- {error.message} -
- {error.stack} -

- ) - } - } - + let loadableOptions: LoadableOptions

= { + // A loading component is not required, so we default it + loading: ({ error, isLoading, pastDelay }) => { + if (!pastDelay) return null + if (process.env.NODE_ENV === 'development') { + if (isLoading) { return null - }, + } + if (error) { + return ( +

+ {error.message} +
+ {error.stack} +

+ ) + } } + return null + }, + } // Support for direct import(), eg: dynamic(import('../hello-world')) // Note that this is only kept for the edge case where someone is passing in a promise as first argument @@ -109,32 +108,6 @@ export default function dynamic

( // Support for passing options, eg: dynamic(import('../hello-world'), {loading: () =>

Loading something

}) loadableOptions = { ...loadableOptions, ...options } - if (loadableOptions.suspense) { - if (process.env.NODE_ENV !== 'production') { - /** - * 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) { - 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) { - 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` - ) - } - } - - delete loadableOptions.ssr - delete loadableOptions.loading - } - // coming from build/babel/plugins/react-loadable-plugin.js if (loadableOptions.loadableGenerated) { loadableOptions = { @@ -144,9 +117,8 @@ export default function dynamic

( delete loadableOptions.loadableGenerated } - // support for disabling server side rendering, eg: dynamic(import('../hello-world'), {ssr: false}). - // skip `ssr` for suspense mode and opt-in React.lazy directly - if (typeof loadableOptions.ssr === 'boolean' && !loadableOptions.suspense) { + // support for disabling server side rendering, eg: dynamic(() => import('../hello-world'), {ssr: false}). + if (typeof loadableOptions.ssr === 'boolean') { if (!loadableOptions.ssr) { delete loadableOptions.ssr return noSSR(loadableFn, loadableOptions) diff --git a/packages/next/shared/lib/loadable-context.ts b/packages/next/shared/lib/loadable-context.ts index 914b64939d38676..c449b61d3ee962f 100644 --- a/packages/next/shared/lib/loadable-context.ts +++ b/packages/next/shared/lib/loadable-context.ts @@ -1,3 +1,5 @@ +'use client' + import React from 'react' type CaptureFn = (moduleName: string) => void diff --git a/packages/next/shared/lib/loadable.js b/packages/next/shared/lib/loadable.js index 9b8d7a73ddb3bc3..404cc1b3826418d 100644 --- a/packages/next/shared/lib/loadable.js +++ b/packages/next/shared/lib/loadable.js @@ -21,7 +21,7 @@ WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE // https://github.com/jamiebuilds/react-loadable/blob/v5.5.0/src/index.js // Modified to be compatible with webpack 4 / Next.js -import React, { useSyncExternalStore } from 'react' +import React from 'react' import { LoadableContext } from './loadable-context' const ALL_INITIALIZERS = [] @@ -52,10 +52,6 @@ function load(loader) { return state } -function resolve(obj) { - return obj && obj.__esModule ? obj.default : obj -} - function createLoadableComponent(loadFn, options) { let opts = Object.assign( { @@ -65,14 +61,14 @@ function createLoadableComponent(loadFn, options) { timeout: null, webpack: null, modules: null, - suspense: false, + // suspense: false, }, options ) - if (opts.suspense) { - opts.lazy = React.lazy(opts.loader) - } + opts.lazy = React.lazy(opts.loader) + + console.log('opts', opts) /** @type LoadableSubscription */ let subscription = null @@ -123,47 +119,24 @@ function createLoadableComponent(loadFn, options) { } } - function LoadableImpl(props, ref) { + function LoadableComponent(props, ref) { useLoadableModule() - const state = useSyncExternalStore( - subscription.subscribe, - subscription.getCurrentValue, - subscription.getCurrentValue - ) + const fallbackElement = React.createElement(opts.loading, { + isLoading: true, + pastDelay: true, + error: null, + }) - React.useImperativeHandle( - ref, - () => ({ - retry: subscription.retry, - }), - [] + return React.createElement( + React.Suspense, + { + fallback: fallbackElement, + }, + React.createElement(opts.lazy, { ...props, ref }) ) - - return React.useMemo(() => { - if (state.loading || state.error) { - return React.createElement(opts.loading, { - isLoading: state.loading, - pastDelay: state.pastDelay, - timedOut: state.timedOut, - error: state.error, - retry: subscription.retry, - }) - } else if (state.loaded) { - return React.createElement(resolve(state.loaded), props) - } else { - return null - } - }, [props, state]) - } - - function LazyImpl(props, ref) { - useLoadableModule() - - return React.createElement(opts.lazy, { ...props, ref }) } - const LoadableComponent = opts.suspense ? LazyImpl : LoadableImpl LoadableComponent.preload = () => init() LoadableComponent.displayName = 'LoadableComponent' diff --git a/test/integration/next-dynamic-lazy-compilation/test/index.test.js b/test/integration/next-dynamic-lazy-compilation/test/index.test.js index e76b1176b91ee68..1711b20a795bba8 100644 --- a/test/integration/next-dynamic-lazy-compilation/test/index.test.js +++ b/test/integration/next-dynamic-lazy-compilation/test/index.test.js @@ -29,7 +29,9 @@ function runTests() { const browser = await webdriver(appPort, '/') const text = await browser.elementByCss('#before-hydration').text() - expect(text).toBe('Index12344') + expect(text).toMatch( + /^Index1()+2()+3()+4()+4$/ + ) expect(await browser.eval('window.caughtErrors')).toBe('') }) @@ -37,7 +39,9 @@ function runTests() { const browser = await webdriver(appPort, '/') const text = await browser.elementByCss('#first-render').text() - expect(text).toBe('Index12344') + expect(text).toMatch( + /^Index1()+2()+3()+4()+4$/ + ) expect(await browser.eval('window.caughtErrors')).toBe('') }) } diff --git a/test/integration/next-dynamic-with-suspense/pages/index.js b/test/integration/next-dynamic-with-suspense/pages/index.js deleted file mode 100644 index 284fcc7f7f8f177..000000000000000 --- a/test/integration/next-dynamic-with-suspense/pages/index.js +++ /dev/null @@ -1,19 +0,0 @@ -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 ( -

-

Next.js Example

- - - -
- ) -} diff --git a/test/integration/next-dynamic-with-suspense/pages/thing.js b/test/integration/next-dynamic-with-suspense/pages/thing.js deleted file mode 100644 index 561df9831e5b220..000000000000000 --- a/test/integration/next-dynamic-with-suspense/pages/thing.js +++ /dev/null @@ -1,3 +0,0 @@ -export default function Thing() { - return 'Thing' -} diff --git a/test/integration/next-dynamic-with-suspense/test/index.test.ts b/test/integration/next-dynamic-with-suspense/test/index.test.ts deleted file mode 100644 index 625fdde999d507b..000000000000000 --- a/test/integration/next-dynamic-with-suspense/test/index.test.ts +++ /dev/null @@ -1,41 +0,0 @@ -/* 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() - }) -}) diff --git a/test/integration/next-dynamic/test/index.test.js b/test/integration/next-dynamic/test/index.test.js index b926fdfca98aa1b..5207e55b64c02cb 100644 --- a/test/integration/next-dynamic/test/index.test.js +++ b/test/integration/next-dynamic/test/index.test.js @@ -29,7 +29,9 @@ function runTests() { const text = await browser.elementByCss('#first-render').text() // Failure case is 'Index3' - expect(text).toBe('Index12344') + expect(text).toMatch( + /^Index1()+2()+3()+4()+4$/ + ) expect(await browser.eval('window.caughtErrors')).toBe('') // should not print "invalid-dynamic-suspense" warning in browser's console From 800e88f87653b4e5375cb2f99c0dd138363a5e11 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Tue, 8 Nov 2022 13:23:06 +0100 Subject: [PATCH 02/15] use lazy to load inline component fix tests update tests --- packages/next/shared/lib/dynamic.tsx | 28 ++++++++----- packages/next/shared/lib/loadable.js | 10 ++--- .../next-dynamic/components/nested2.js | 9 +++-- .../next-dynamic/pages/dynamic/head.js | 40 ++++++++++--------- .../basic/next-dynamic/components/nested2.js | 10 +++-- .../basic/next-dynamic/pages/dynamic/head.js | 40 ++++++++++--------- 6 files changed, 77 insertions(+), 60 deletions(-) diff --git a/packages/next/shared/lib/dynamic.tsx b/packages/next/shared/lib/dynamic.tsx index 84d3853eb09a649..b3a5adeb253c667 100644 --- a/packages/next/shared/lib/dynamic.tsx +++ b/packages/next/shared/lib/dynamic.tsx @@ -1,4 +1,4 @@ -import React from 'react' +import React, { Suspense } from 'react' import Loadable from './loadable' const isServerSide = typeof window === 'undefined' @@ -9,7 +9,7 @@ export type LoaderComponent

= Promise< export type Loader

= (() => LoaderComponent

) | LoaderComponent

-export type LoaderMap = { [mdule: string]: () => Loader } +export type LoaderMap = { [module: string]: () => Loader } export type LoadableGeneratedOptions = { webpack?(): any @@ -44,22 +44,32 @@ export type LoadableFn

= ( export type LoadableComponent

= React.ComponentType

export function noSSR

( - LoadableInitializer: LoadableFn

, + _LoadableInitializer: LoadableFn

, loadableOptions: DynamicOptions

): React.ComponentType

{ // Removing webpack and modules means react-loadable won't try preloading delete loadableOptions.webpack delete loadableOptions.modules - // This check is necessary to prevent react-loadable from initializing on the server - if (!isServerSide) { - return LoadableInitializer(loadableOptions) - } + const NoSSRComponent = React.lazy( + (isServerSide + ? async () => ({ default: () => null }) + : loadableOptions.loader) as () => Promise<{ + default: React.ComponentType

+ }> + ) const Loading = loadableOptions.loading! - // This will only be rendered on the server side + return () => ( - + + } + > + {/* @ts-ignore */} + + ) } diff --git a/packages/next/shared/lib/loadable.js b/packages/next/shared/lib/loadable.js index 404cc1b3826418d..4df5d11263e7019 100644 --- a/packages/next/shared/lib/loadable.js +++ b/packages/next/shared/lib/loadable.js @@ -61,15 +61,12 @@ function createLoadableComponent(loadFn, options) { timeout: null, webpack: null, modules: null, - // suspense: false, }, options ) opts.lazy = React.lazy(opts.loader) - console.log('opts', opts) - /** @type LoadableSubscription */ let subscription = null function init() { @@ -118,8 +115,7 @@ function createLoadableComponent(loadFn, options) { }) } } - - function LoadableComponent(props, ref) { + function LoadableComponent(props) { useLoadableModule() const fallbackElement = React.createElement(opts.loading, { @@ -133,14 +129,14 @@ function createLoadableComponent(loadFn, options) { { fallback: fallbackElement, }, - React.createElement(opts.lazy, { ...props, ref }) + React.createElement(opts.lazy, props) ) } LoadableComponent.preload = () => init() LoadableComponent.displayName = 'LoadableComponent' - return React.forwardRef(LoadableComponent) + return LoadableComponent } class LoadableSubscription { diff --git a/test/development/basic-basepath/next-dynamic/components/nested2.js b/test/development/basic-basepath/next-dynamic/components/nested2.js index 8fe2f1ce0a6842a..b9b2c982b108a8d 100644 --- a/test/development/basic-basepath/next-dynamic/components/nested2.js +++ b/test/development/basic-basepath/next-dynamic/components/nested2.js @@ -1,8 +1,11 @@ import dynamic from 'next/dynamic' -const BrowserLoaded = dynamic(async () => () =>

Browser hydrated
, { - ssr: false, -}) +const BrowserLoaded = dynamic( + async () => ({ default: () =>
Browser hydrated
}), + { + ssr: false, + } +) export default () => (
diff --git a/test/development/basic-basepath/next-dynamic/pages/dynamic/head.js b/test/development/basic-basepath/next-dynamic/pages/dynamic/head.js index 4fe4224787a6426..3d1535a164eb577 100644 --- a/test/development/basic-basepath/next-dynamic/pages/dynamic/head.js +++ b/test/development/basic-basepath/next-dynamic/pages/dynamic/head.js @@ -3,25 +3,27 @@ import Head from 'next/head' const Test = dynamic({ loader: async () => { - // component - return () => { - return ( -
- -