From 600287838c17dd5ed6c3ac1753efd0985302d68d Mon Sep 17 00:00:00 2001
From: Jiachi Liu <inbox@huozhi.im>
Date: Tue, 30 Apr 2024 22:40:11 +0200
Subject: [PATCH 1/5] Remove extra suspense boundary for default next/dynamic
 (#64716)

### What

Removing the Suspense boundary on top of `next/dynamic` by default, make
it as `React.lazy` component with preloading CSS feature.

### Why

Extra Suspense boundary is causing extra useless rendering. For SSR, it
shouldn't render `loading` by default

Related: #64060
Related: #64687
Closes
[NEXT-3074](https://linear.app/vercel/issue/NEXT-3074/app-router-content-flickering-with-reactcreatecontext-and-nextdynamic)

This is sort of a breaking change, since removing the Suspense boundary
on top of `next/dynamic` by default. If there's error happening in side
the dynamic component you need to wrap an extra Suspense boundary on top
of it
---
 packages/next/src/shared/lib/dynamic.tsx      |  1 +
 .../src/shared/lib/lazy-dynamic/loadable.tsx  |  9 +++++---
 .../dynamic/app/default/dynamic-component.js  | 12 ++++++++++
 test/e2e/app-dir/dynamic/app/default/page.js  | 17 ++++++++++++++
 test/e2e/app-dir/dynamic/dynamic.test.ts      | 23 ++++++++++++++++++-
 5 files changed, 58 insertions(+), 4 deletions(-)
 create mode 100644 test/e2e/app-dir/dynamic/app/default/dynamic-component.js
 create mode 100644 test/e2e/app-dir/dynamic/app/default/page.js

diff --git a/packages/next/src/shared/lib/dynamic.tsx b/packages/next/src/shared/lib/dynamic.tsx
index 52f68be1f73ec..64b10dd837e49 100644
--- a/packages/next/src/shared/lib/dynamic.tsx
+++ b/packages/next/src/shared/lib/dynamic.tsx
@@ -41,6 +41,7 @@ export type DynamicOptions<P = {}> = LoadableGeneratedOptions & {
   loadableGenerated?: LoadableGeneratedOptions
   ssr?: boolean
   /**
+   * TODO: remove this in next major version v15
    * @deprecated `suspense` prop is not required anymore
    */
   suspense?: boolean
diff --git a/packages/next/src/shared/lib/lazy-dynamic/loadable.tsx b/packages/next/src/shared/lib/lazy-dynamic/loadable.tsx
index 5950b1d82ce2f..d3ec03d14f856 100644
--- a/packages/next/src/shared/lib/lazy-dynamic/loadable.tsx
+++ b/packages/next/src/shared/lib/lazy-dynamic/loadable.tsx
@@ -1,4 +1,4 @@
-import { Suspense, lazy } from 'react'
+import { Suspense, Fragment, lazy } from 'react'
 import { BailoutToCSR } from './dynamic-bailout-to-csr'
 import type { ComponentModule } from './types'
 import { PreloadChunks } from './preload-chunks'
@@ -48,7 +48,10 @@ function Loadable(options: LoadableOptions) {
       <Loading isLoading={true} pastDelay={true} error={null} />
     ) : null
 
-    const children = opts.ssr ? (
+    const isSSR = opts.ssr
+    const Wrap = isSSR ? Fragment : Suspense
+    const wrapProps = isSSR ? {} : { fallback: fallbackElement }
+    const children = isSSR ? (
       <>
         {/* During SSR, we need to preload the CSS from the dynamic component to avoid flash of unstyled content */}
         {typeof window === 'undefined' ? (
@@ -62,7 +65,7 @@ function Loadable(options: LoadableOptions) {
       </BailoutToCSR>
     )
 
-    return <Suspense fallback={fallbackElement}>{children}</Suspense>
+    return <Wrap {...wrapProps}>{children}</Wrap>
   }
 
   LoadableComponent.displayName = 'LoadableComponent'
diff --git a/test/e2e/app-dir/dynamic/app/default/dynamic-component.js b/test/e2e/app-dir/dynamic/app/default/dynamic-component.js
new file mode 100644
index 0000000000000..27d3b267db32d
--- /dev/null
+++ b/test/e2e/app-dir/dynamic/app/default/dynamic-component.js
@@ -0,0 +1,12 @@
+const isDevTest = false
+
+const DynamicImportComponent = () => {
+  if (isDevTest && typeof window === 'undefined') {
+    throw new Error('This component should only be rendered on the client side')
+  }
+  return (
+    <div id="dynamic-component">This is a dynamically imported component</div>
+  )
+}
+
+export default DynamicImportComponent
diff --git a/test/e2e/app-dir/dynamic/app/default/page.js b/test/e2e/app-dir/dynamic/app/default/page.js
new file mode 100644
index 0000000000000..b40d356d55955
--- /dev/null
+++ b/test/e2e/app-dir/dynamic/app/default/page.js
@@ -0,0 +1,17 @@
+'use client'
+
+import dynamic from 'next/dynamic'
+
+const DynamicHeader = dynamic(() => import('./dynamic-component'), {
+  loading: () => <p>Loading...</p>,
+})
+
+const ClientWrapper = () => {
+  return (
+    <div>
+      <DynamicHeader />
+    </div>
+  )
+}
+
+export default ClientWrapper
diff --git a/test/e2e/app-dir/dynamic/dynamic.test.ts b/test/e2e/app-dir/dynamic/dynamic.test.ts
index 9322396bf405c..590ae9509e054 100644
--- a/test/e2e/app-dir/dynamic/dynamic.test.ts
+++ b/test/e2e/app-dir/dynamic/dynamic.test.ts
@@ -1,7 +1,8 @@
 import { nextTestSetup } from 'e2e-utils'
+import { retry } from 'next-test-utils'
 
 describe('app dir - next/dynamic', () => {
-  const { next, isNextStart, skipped } = nextTestSetup({
+  const { next, isNextStart, isNextDev, skipped } = nextTestSetup({
     files: __dirname,
     skipDeployment: true,
   })
@@ -57,6 +58,26 @@ describe('app dir - next/dynamic', () => {
     expect($('h1').text()).toBe('hello')
   })
 
+  it('should not render loading by default', async () => {
+    const $ = await next.render$('/default')
+    expect($('#dynamic-component').text()).not.toContain('loading')
+  })
+
+  if (isNextDev) {
+    it('should directly raise error when dynamic component error on server', async () => {
+      const pagePath = 'app/default/dynamic-component.js'
+      const page = await next.readFile(pagePath)
+      await next.patchFile(
+        pagePath,
+        page.replace('const isDevTest = false', 'const isDevTest = true')
+      )
+      await retry(async () => {
+        const { status } = await next.fetch('/default')
+        expect(status).toBe(500)
+      })
+    })
+  }
+
   describe('no SSR', () => {
     it('should not render client component imported through ssr: false in client components in edge runtime', async () => {
       // noSSR should not show up in html

From 240fde922d95c34a0838b2c6cd5d191a1490b946 Mon Sep 17 00:00:00 2001
From: Jiachi Liu <inbox@huozhi.im>
Date: Mon, 17 Jun 2024 22:39:10 +0200
Subject: [PATCH 2/5] no suspense without loading

---
 packages/next/src/shared/lib/dynamic.tsx      |  5 ----
 .../src/shared/lib/lazy-dynamic/loadable.tsx  |  9 ++++---
 .../app/default-loading/dynamic-component.js  | 12 +++++++++
 .../dynamic/app/default-loading/page.js       | 26 +++++++++++++++++++
 .../dynamic/app/default/dynamic-component.js  |  5 ----
 test/e2e/app-dir/dynamic/app/default/page.js  | 12 ++++++---
 test/e2e/app-dir/dynamic/dynamic.test.ts      | 15 ++++++++---
 7 files changed, 63 insertions(+), 21 deletions(-)
 create mode 100644 test/e2e/app-dir/dynamic/app/default-loading/dynamic-component.js
 create mode 100644 test/e2e/app-dir/dynamic/app/default-loading/page.js

diff --git a/packages/next/src/shared/lib/dynamic.tsx b/packages/next/src/shared/lib/dynamic.tsx
index 64b10dd837e49..3dd72ada2eca2 100644
--- a/packages/next/src/shared/lib/dynamic.tsx
+++ b/packages/next/src/shared/lib/dynamic.tsx
@@ -40,11 +40,6 @@ export type DynamicOptions<P = {}> = LoadableGeneratedOptions & {
   loader?: Loader<P> | LoaderMap
   loadableGenerated?: LoadableGeneratedOptions
   ssr?: boolean
-  /**
-   * TODO: remove this in next major version v15
-   * @deprecated `suspense` prop is not required anymore
-   */
-  suspense?: boolean
 }
 
 export type LoadableOptions<P = {}> = DynamicOptions<P>
diff --git a/packages/next/src/shared/lib/lazy-dynamic/loadable.tsx b/packages/next/src/shared/lib/lazy-dynamic/loadable.tsx
index d3ec03d14f856..3f74a077e54c5 100644
--- a/packages/next/src/shared/lib/lazy-dynamic/loadable.tsx
+++ b/packages/next/src/shared/lib/lazy-dynamic/loadable.tsx
@@ -48,10 +48,11 @@ function Loadable(options: LoadableOptions) {
       <Loading isLoading={true} pastDelay={true} error={null} />
     ) : null
 
-    const isSSR = opts.ssr
-    const Wrap = isSSR ? Fragment : Suspense
-    const wrapProps = isSSR ? {} : { fallback: fallbackElement }
-    const children = isSSR ? (
+    // If it's non-SSR or provided a loading component, wrap it in a suspense boundary
+    const hasSuspenseBoundary = !opts.ssr || !!opts.loading
+    const Wrap = hasSuspenseBoundary ? Suspense : Fragment
+    const wrapProps = hasSuspenseBoundary ? { fallback: fallbackElement } : {}
+    const children = opts.ssr ? (
       <>
         {/* During SSR, we need to preload the CSS from the dynamic component to avoid flash of unstyled content */}
         {typeof window === 'undefined' ? (
diff --git a/test/e2e/app-dir/dynamic/app/default-loading/dynamic-component.js b/test/e2e/app-dir/dynamic/app/default-loading/dynamic-component.js
new file mode 100644
index 0000000000000..27d3b267db32d
--- /dev/null
+++ b/test/e2e/app-dir/dynamic/app/default-loading/dynamic-component.js
@@ -0,0 +1,12 @@
+const isDevTest = false
+
+const DynamicImportComponent = () => {
+  if (isDevTest && typeof window === 'undefined') {
+    throw new Error('This component should only be rendered on the client side')
+  }
+  return (
+    <div id="dynamic-component">This is a dynamically imported component</div>
+  )
+}
+
+export default DynamicImportComponent
diff --git a/test/e2e/app-dir/dynamic/app/default-loading/page.js b/test/e2e/app-dir/dynamic/app/default-loading/page.js
new file mode 100644
index 0000000000000..fff76ebc5e2e4
--- /dev/null
+++ b/test/e2e/app-dir/dynamic/app/default-loading/page.js
@@ -0,0 +1,26 @@
+'use client'
+
+import dynamic from 'next/dynamic'
+
+const DynamicHeader = dynamic(
+  () => {
+    return new Promise((resolve) => {
+      setTimeout(() => {
+        resolve(import('./dynamic-component'))
+      }, 1000)
+    })
+  },
+  {
+    loading: () => <p>Loading...</p>,
+  }
+)
+
+const Page = () => {
+  return (
+    <div>
+      <DynamicHeader />
+    </div>
+  )
+}
+
+export default Page
diff --git a/test/e2e/app-dir/dynamic/app/default/dynamic-component.js b/test/e2e/app-dir/dynamic/app/default/dynamic-component.js
index 27d3b267db32d..f23de50e36b12 100644
--- a/test/e2e/app-dir/dynamic/app/default/dynamic-component.js
+++ b/test/e2e/app-dir/dynamic/app/default/dynamic-component.js
@@ -1,9 +1,4 @@
-const isDevTest = false
-
 const DynamicImportComponent = () => {
-  if (isDevTest && typeof window === 'undefined') {
-    throw new Error('This component should only be rendered on the client side')
-  }
   return (
     <div id="dynamic-component">This is a dynamically imported component</div>
   )
diff --git a/test/e2e/app-dir/dynamic/app/default/page.js b/test/e2e/app-dir/dynamic/app/default/page.js
index b40d356d55955..21986bd0e17ef 100644
--- a/test/e2e/app-dir/dynamic/app/default/page.js
+++ b/test/e2e/app-dir/dynamic/app/default/page.js
@@ -2,11 +2,15 @@
 
 import dynamic from 'next/dynamic'
 
-const DynamicHeader = dynamic(() => import('./dynamic-component'), {
-  loading: () => <p>Loading...</p>,
+const DynamicHeader = dynamic(() => {
+  return new Promise((resolve) => {
+    setTimeout(() => {
+      resolve(import('./dynamic-component'))
+    }, 1000)
+  })
 })
 
-const ClientWrapper = () => {
+const Page = () => {
   return (
     <div>
       <DynamicHeader />
@@ -14,4 +18,4 @@ const ClientWrapper = () => {
   )
 }
 
-export default ClientWrapper
+export default Page
diff --git a/test/e2e/app-dir/dynamic/dynamic.test.ts b/test/e2e/app-dir/dynamic/dynamic.test.ts
index 590ae9509e054..1946cbf67fa71 100644
--- a/test/e2e/app-dir/dynamic/dynamic.test.ts
+++ b/test/e2e/app-dir/dynamic/dynamic.test.ts
@@ -58,6 +58,15 @@ describe('app dir - next/dynamic', () => {
     expect($('h1').text()).toBe('hello')
   })
 
+  it('should render loading by default if loading is specified and loader is slow', async () => {
+    const $ = await next.render$('/default-loading')
+
+    // First render in dev should show loading, production build will resolve the content.
+    expect($('body').text()).toContain(
+      isNextDev ? 'Loading...' : 'This is a dynamically imported component'
+    )
+  })
+
   it('should not render loading by default', async () => {
     const $ = await next.render$('/default')
     expect($('#dynamic-component').text()).not.toContain('loading')
@@ -65,15 +74,15 @@ describe('app dir - next/dynamic', () => {
 
   if (isNextDev) {
     it('should directly raise error when dynamic component error on server', async () => {
-      const pagePath = 'app/default/dynamic-component.js'
+      const pagePath = 'app/default-loading/dynamic-component.js'
       const page = await next.readFile(pagePath)
       await next.patchFile(
         pagePath,
         page.replace('const isDevTest = false', 'const isDevTest = true')
       )
       await retry(async () => {
-        const { status } = await next.fetch('/default')
-        expect(status).toBe(500)
+        const { status } = await next.fetch('/default-loading')
+        expect(status).toBe(200)
       })
     })
   }

From 45baf6276020cba4a9656f4eabf3c936eebc51d6 Mon Sep 17 00:00:00 2001
From: Jiachi Liu <inbox@huozhi.im>
Date: Tue, 25 Jun 2024 14:20:02 +0200
Subject: [PATCH 3/5] fix test

---
 .../app/pages/dynamic-suspense.js                    | 12 ------------
 .../app/pages/use-flush-effect/styled-jsx.tsx        |  4 +---
 .../react-current-version/test/index.test.js         |  1 -
 3 files changed, 1 insertion(+), 16 deletions(-)
 delete mode 100644 test/integration/react-current-version/app/pages/dynamic-suspense.js

diff --git a/test/integration/react-current-version/app/pages/dynamic-suspense.js b/test/integration/react-current-version/app/pages/dynamic-suspense.js
deleted file mode 100644
index e576f788e1352..0000000000000
--- a/test/integration/react-current-version/app/pages/dynamic-suspense.js
+++ /dev/null
@@ -1,12 +0,0 @@
-import dynamic from 'next/dynamic'
-import { Suspense } from 'react'
-
-const Foo = dynamic(() => import('../components/foo'), { suspense: true })
-
-export default () => (
-  <div>
-    <Suspense fallback="fallback">
-      <Foo />
-    </Suspense>
-  </div>
-)
diff --git a/test/integration/react-current-version/app/pages/use-flush-effect/styled-jsx.tsx b/test/integration/react-current-version/app/pages/use-flush-effect/styled-jsx.tsx
index 039f554337c3e..d24fc947ffd82 100644
--- a/test/integration/react-current-version/app/pages/use-flush-effect/styled-jsx.tsx
+++ b/test/integration/react-current-version/app/pages/use-flush-effect/styled-jsx.tsx
@@ -3,9 +3,7 @@
 import React from 'react'
 import dynamic from 'next/dynamic'
 
-const Red = dynamic(() => import('../../components/red'), {
-  suspense: true,
-})
+const Red = dynamic(() => import('../../components/red'))
 
 function Blue() {
   return (
diff --git a/test/integration/react-current-version/test/index.test.js b/test/integration/react-current-version/test/index.test.js
index db686913f2944..50638dcb928e5 100644
--- a/test/integration/react-current-version/test/index.test.js
+++ b/test/integration/react-current-version/test/index.test.js
@@ -66,7 +66,6 @@ describe('Basics', () => {
         }
       }
       await expectToContainPreload('dynamic')
-      await expectToContainPreload('dynamic-suspense')
     })
   })
 })

From ab8036b6fa4bfa079bd352bad85aa2c29c7e4cbd Mon Sep 17 00:00:00 2001
From: Jiachi Liu <inbox@huozhi.im>
Date: Tue, 25 Jun 2024 17:30:11 +0200
Subject: [PATCH 4/5] Remove default loading

---
 packages/next/src/shared/lib/app-dynamic.tsx | 27 +++-----------------
 1 file changed, 4 insertions(+), 23 deletions(-)

diff --git a/packages/next/src/shared/lib/app-dynamic.tsx b/packages/next/src/shared/lib/app-dynamic.tsx
index 94da53ad04ca4..3ac8389091e03 100644
--- a/packages/next/src/shared/lib/app-dynamic.tsx
+++ b/packages/next/src/shared/lib/app-dynamic.tsx
@@ -1,4 +1,5 @@
-import React, { type JSX } from 'react'
+import type React from 'react';
+import type { JSX } from 'react'
 import Loadable from './lazy-dynamic/loadable'
 
 import type {
@@ -16,7 +17,7 @@ export {
 }
 
 export type DynamicOptions<P = {}> = LoadableGeneratedOptions & {
-  loading?: (loadingProps: DynamicOptionsLoadingProps) => JSX.Element | null
+  loading?: () => JSX.Element | null
   loader?: Loader<P>
   loadableGenerated?: LoadableGeneratedOptions
   modules?: string[]
@@ -35,27 +36,7 @@ export default function dynamic<P = {}>(
   dynamicOptions: DynamicOptions<P> | Loader<P>,
   options?: DynamicOptions<P>
 ): React.ComponentType<P> {
-  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 !== 'production') {
-        if (isLoading) {
-          return null
-        }
-        if (error) {
-          return (
-            <p>
-              {error.message}
-              <br />
-              {error.stack}
-            </p>
-          )
-        }
-      }
-      return null
-    },
-  }
+  const loadableOptions: LoadableOptions<P> = {}
 
   if (typeof dynamicOptions === 'function') {
     loadableOptions.loader = dynamicOptions

From 32d6423a261ee5e690b13a7d1f228348752de185 Mon Sep 17 00:00:00 2001
From: Jiachi Liu <inbox@huozhi.im>
Date: Tue, 25 Jun 2024 19:57:29 +0200
Subject: [PATCH 5/5] fix test

---
 packages/next/src/shared/lib/app-dynamic.tsx               | 2 +-
 test/e2e/app-dir/next-dynamic-css/app/page/inner.tsx       | 7 ++++++-
 test/e2e/app-dir/next-dynamic-css/next-dynamic-css.test.ts | 2 +-
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/packages/next/src/shared/lib/app-dynamic.tsx b/packages/next/src/shared/lib/app-dynamic.tsx
index 3ac8389091e03..9b03073ff0011 100644
--- a/packages/next/src/shared/lib/app-dynamic.tsx
+++ b/packages/next/src/shared/lib/app-dynamic.tsx
@@ -1,4 +1,4 @@
-import type React from 'react';
+import type React from 'react'
 import type { JSX } from 'react'
 import Loadable from './lazy-dynamic/loadable'
 
diff --git a/test/e2e/app-dir/next-dynamic-css/app/page/inner.tsx b/test/e2e/app-dir/next-dynamic-css/app/page/inner.tsx
index b0588afc687ca..e25d002efe339 100644
--- a/test/e2e/app-dir/next-dynamic-css/app/page/inner.tsx
+++ b/test/e2e/app-dir/next-dynamic-css/app/page/inner.tsx
@@ -1,8 +1,13 @@
 'use client'
 
 import dynamic from 'next/dynamic'
+import { Suspense } from 'react'
 const Component = dynamic(() => import('./component'))
 
 export default async function Inner() {
-  return <Component />
+  return (
+    <Suspense>
+      <Component />
+    </Suspense>
+  )
 }
diff --git a/test/e2e/app-dir/next-dynamic-css/next-dynamic-css.test.ts b/test/e2e/app-dir/next-dynamic-css/next-dynamic-css.test.ts
index ecb596dea017b..52fd51cecd95f 100644
--- a/test/e2e/app-dir/next-dynamic-css/next-dynamic-css.test.ts
+++ b/test/e2e/app-dir/next-dynamic-css/next-dynamic-css.test.ts
@@ -40,7 +40,7 @@ describe('next-dynamic-css', () => {
     })
   }
 
-  it('should have correct order of styles on next/dymamic loaded component', async () => {
+  it('should have correct order of styles on next/dynamic loaded component', async () => {
     const browser = await next.browser('/page')
     expect(await browser.waitForElementByCss('#component').text()).toBe(
       'Hello Component'