From 75c9a2cd3dbb0ddb41377c37d9ef3c24042e4398 Mon Sep 17 00:00:00 2001 From: SukkaW Date: Fri, 9 Sep 2022 21:24:13 +0800 Subject: [PATCH 1/4] fix(#40388): only add default loading for non-suspense --- packages/next/shared/lib/dynamic.tsx | 44 +++++++++++++++------------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/packages/next/shared/lib/dynamic.tsx b/packages/next/shared/lib/dynamic.tsx index 6e7dc8fa2290cb3..094e11faf0fdd39 100644 --- a/packages/next/shared/lib/dynamic.tsx +++ b/packages/next/shared/lib/dynamic.tsx @@ -65,29 +65,33 @@ export default function dynamic

( options?: DynamicOptions

): React.ComponentType

{ let loadableFn: LoadableFn

= Loadable - 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) { + + 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} +

+ ) + } + } + 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 // The react-loadable babel plugin will turn dynamic(import('../hello-world')) into dynamic(() => import('../hello-world')) From 7cf54dc47ddaee892d4028ce4cce5de02768f90d Mon Sep 17 00:00:00 2001 From: SukkaW Date: Fri, 9 Sep 2022 21:30:58 +0800 Subject: [PATCH 2/4] test(#40388): validate the fix --- test/integration/next-dynamic/test/index.test.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/integration/next-dynamic/test/index.test.js b/test/integration/next-dynamic/test/index.test.js index e0c8f9be9ae73cd..b926fdfca98aa1b 100644 --- a/test/integration/next-dynamic/test/index.test.js +++ b/test/integration/next-dynamic/test/index.test.js @@ -31,6 +31,12 @@ function runTests() { // Failure case is 'Index3' expect(text).toBe('Index12344') expect(await browser.eval('window.caughtErrors')).toBe('') + + // should not print "invalid-dynamic-suspense" warning in browser's console + const logs = (await browser.log()).map((log) => log.message).join('\n') + expect(logs).not.toContain( + 'https://nextjs.org/docs/messages/invalid-dynamic-suspense' + ) }) } From 69f8a9cfbd9103a49cbed10ce03b1abcf9015555 Mon Sep 17 00:00:00 2001 From: SukkaW Date: Fri, 9 Sep 2022 23:02:44 +0800 Subject: [PATCH 3/4] apply code review suggestions by @huozhi Co-authored-by: huozhi --- packages/next/shared/lib/dynamic.tsx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/next/shared/lib/dynamic.tsx b/packages/next/shared/lib/dynamic.tsx index 094e11faf0fdd39..bd92a78ec0a47a6 100644 --- a/packages/next/shared/lib/dynamic.tsx +++ b/packages/next/shared/lib/dynamic.tsx @@ -116,8 +116,8 @@ export default function dynamic

( ) } - if (process.env.NODE_ENV !== 'production') { - if (loadableOptions.suspense) { + 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 @@ -126,19 +126,20 @@ export default function dynamic

( * 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` ) } } + + loadableOptions.ssr = true + loadableOptions.loading = undefined } // coming from build/babel/plugins/react-loadable-plugin.js From 0df82559ad8d5d5e6dd703acb9fd2e43e79f8cdf Mon Sep 17 00:00:00 2001 From: Sukka Date: Fri, 9 Sep 2022 23:24:05 +0800 Subject: [PATCH 4/4] apply code review suggestions by @huozhi Co-authored-by: Jiachi Liu --- packages/next/shared/lib/dynamic.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/next/shared/lib/dynamic.tsx b/packages/next/shared/lib/dynamic.tsx index bd92a78ec0a47a6..b79caf271dc4fca 100644 --- a/packages/next/shared/lib/dynamic.tsx +++ b/packages/next/shared/lib/dynamic.tsx @@ -138,8 +138,8 @@ export default function dynamic

( } } - loadableOptions.ssr = true - loadableOptions.loading = undefined + delete loadableOptions.ssr + delete loadableOptions.loading } // coming from build/babel/plugins/react-loadable-plugin.js