Skip to content
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

fix(#40388): next/dynamic should only add default loading without suspense #40397

Merged
merged 8 commits into from Sep 9, 2022
Merged
44 changes: 24 additions & 20 deletions packages/next/shared/lib/dynamic.tsx
Expand Up @@ -65,29 +65,33 @@ export default function dynamic<P = {}>(
options?: DynamicOptions<P>
): React.ComponentType<P> {
let loadableFn: LoadableFn<P> = Loadable
let loadableOptions: LoadableOptions<P> = {
// 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<P> = options?.suspense
huozhi marked this conversation as resolved.
Show resolved Hide resolved
? {}
: // 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 (
<p>
{error.message}
<br />
{error.stack}
</p>
)
}
}

return null
}
if (error) {
return (
<p>
{error.message}
<br />
{error.stack}
</p>
)
}
},
}

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'))
Expand Down
6 changes: 6 additions & 0 deletions test/integration/next-dynamic/test/index.test.js
Expand Up @@ -31,6 +31,12 @@ function runTests() {
// Failure case is 'Index<!-- -->3<!-- --><!-- -->'
expect(text).toBe('Index<!-- -->1<!-- -->2<!-- -->3<!-- -->4<!-- -->4')
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'
)
})
}

Expand Down