Alias next/dynamic to lazy impl for appDir (#41216)

Since `next/dynamic` has client hooks that not compatible with server
components, and app renderer doesn't provide context (such as
`LoadableContext`) for it to use. Previously we provided a simple
replacement using `React.lazy` for `next/dynamic` if you want to use it
in appDir.

This PR always alias it to the `React.lazy ` implementation for appDir
so that user won't need to worry about the dynamic options. They can
only use `dynamic()` without 2nd options arg

```js
import dynamic from 'next/dynamic'

const Dynamic = dynamic(() => import('./dynamic-component'))
```

packages/next/build/webpack-config.ts

@@ -1586,6 +1586,21 @@ export default async function getBaseWebpackConfig(
               } as any,
             ]
           : []),
+        // Alias `next/dynamic` to React.lazy implementation for RSC
+        ...(hasServerComponents && appDir
+          ? [
+              {
+                test: codeCondition.test,
+                include: [appDir],
+                resolve: {
+                  alias: {
+                    [require.resolve('next/dynamic')]:
+                      'next/dist/client/components/dynamic',
+                  },
+                },
+              },
+            ]
+          : []),
         ...(hasServerComponents && (isNodeServer || isEdgeServer)
           ? [
               // RSC server compilation loaders

packages/next/client/components/shared/dynamic/index.tsx → packages/next/client/components/dynamic.tsx

@@ -15,7 +15,5 @@ export type LoadableComponent<P = {}> = React.ComponentType<P>
 export default function dynamic<P = {}>(
   loader: Loader<P>
 ): React.ComponentType<P> {
-  const LazyLoadable = React.lazy(loader)
-
-  return LazyLoadable
+  return React.lazy(loader)
 }

packages/next/shared/lib/dynamic.tsx

@@ -1,5 +1,3 @@
-'client'
-
 import React from 'react'
 import Loadable from './loadable'
 

test/e2e/app-dir/app/app/dashboard/index/dynamic-imports/dynamic-client.js

@@ -1,6 +1,6 @@
 'client'
 
-import dynamic from 'next/dist/client/components/shared/dynamic'
+import dynamic from 'next/dynamic'
 
 const Dynamic = dynamic(() => import('../text-dynamic-client'))
 

test/e2e/app-dir/app/app/dashboard/index/dynamic-imports/dynamic-server.js

@@ -1,4 +1,4 @@
-import dynamic from 'next/dist/client/components/shared/dynamic'
+import dynamic from 'next/dynamic'
 
 const Dynamic = dynamic(() => import('../text-dynamic-server'))
 

test/e2e/app-dir/rsc-basic.test.ts

@@ -25,7 +25,7 @@ async function resolveStreamResponse(response: any, onData?: any) {
   return result
 }
 
-describe('app dir - react server components', () => {
+describe('app dir - rsc basics', () => {
   let next: NextInstance
   let distDir: string
 
@@ -44,9 +44,8 @@ describe('app dir - react server components', () => {
       },
       packageJson: {
         scripts: {
-          setup: `cp -r ./node_modules_bak/* ./node_modules`,
-          build: 'yarn setup && next build',
-          dev: 'yarn setup && next dev',
+          build: 'next build',
+          dev: 'next dev',
           start: 'next start',
         },
       },
@@ -79,7 +78,7 @@ describe('app dir - react server components', () => {
 
     const inlineFlightContents = []
     const $ = cheerio.load(homeHTML)
-    $('script').each((index, tag) => {
+    $('script').each((_index, tag) => {
       const content = $(tag).text()
       if (content) inlineFlightContents.push(content)
     })
@@ -244,20 +243,6 @@ describe('app dir - react server components', () => {
     expect(imageTag.attr('src')).toContain('data:image')
   })
 
-  it('should handle external async module libraries correctly', async () => {
-    const clientHtml = await renderViaHTTP(next.url, '/external-imports/client')
-    const serverHtml = await renderViaHTTP(next.url, '/external-imports/server')
-
-    expect(clientHtml).toContain('module type:esm-export')
-    expect(clientHtml).toContain('export named:named')
-    expect(clientHtml).toContain('export value:123')
-    expect(clientHtml).toContain('export array:4,5,6')
-    expect(clientHtml).toContain('export object:{x:1}')
-
-    // support esm module on server side
-    expect(serverHtml).toContain('random-module-instance')
-  })
-
   it('should handle various kinds of exports correctly', async () => {
     const html = await renderViaHTTP(next.url, '/various-exports')
     const content = getNodeBySelector(html, 'body').text()
@@ -399,45 +384,6 @@ describe('app dir - react server components', () => {
     )
   })
 
-  it('should resolve the subset react in server components based on the react-server condition', async () => {
-    await fetchViaHTTP(next.url, '/react-server').then(async (response) => {
-      const result = await resolveStreamResponse(response)
-      expect(result).toContain('Server: <!-- -->subset')
-      expect(result).toContain('Client: <!-- -->full')
-    })
-  })
-
-  it('should resolve 3rd party package exports based on the react-server condition', async () => {
-    await fetchViaHTTP(next.url, '/react-server/3rd-party-package').then(
-      async (response) => {
-        const result = await resolveStreamResponse(response)
-
-        // Package should be resolved based on the react-server condition,
-        // as well as package's internal & external dependencies.
-        expect(result).toContain(
-          'Server: index.react-server:react.subset:dep.server'
-        )
-        expect(result).toContain('Client: index.default:react.full:dep.default')
-
-        // Subpath exports should be resolved based on the condition too.
-        expect(result).toContain('Server subpath: subpath.react-server')
-        expect(result).toContain('Client subpath: subpath.default')
-      }
-    )
-  })
-
-  it('should be able to opt-out 3rd party packages being bundled in server components', async () => {
-    await fetchViaHTTP(next.url, '/react-server/optout').then(
-      async (response) => {
-        const result = await resolveStreamResponse(response)
-        expect(result).toContain('Server: index.default')
-        expect(result).toContain('Server subpath: subpath.default')
-        expect(result).toContain('Client: index.default')
-        expect(result).toContain('Client subpath: subpath.default')
-      }
-    )
-  })
-
   if (!isNextDev) {
     it('should generate edge SSR manifests for Node.js', async () => {
       const distServerDir = path.join(distDir, 'server')

test/e2e/app-dir/rsc-basic/app/shared/page.js

@@ -1,7 +1,7 @@
 import ClientFromDirect from '../../components/client'
 import ClientFromShared from '../../components/shared'
 import SharedFromClient from '../../components/shared-client'
-import Random from '../../components/random-module-instance'
+// import Random from '../../components/random-module-instance'
 import Bar from '../../components/bar'
 
 export default function Page() {
@@ -12,7 +12,7 @@ export default function Page() {
   // It's expected to have hydration mismatch here.
   return (
     <div id="main" suppressHydrationWarning>
-      <Random />
+      {/* <Random /> */}
       <br />
       <ClientFromDirect />
       <br />

test/e2e/app-dir/rsc-external.test.ts

@@ -0,0 +1,113 @@
+import path from 'path'
+import { renderViaHTTP, fetchViaHTTP } from 'next-test-utils'
+import { createNext, FileRef } from 'e2e-utils'
+import { NextInstance } from 'test/lib/next-modes/base'
+
+async function resolveStreamResponse(response: any, onData?: any) {
+  let result = ''
+  onData = onData || (() => {})
+  await new Promise((resolve) => {
+    response.body.on('data', (chunk) => {
+      result += chunk.toString()
+      onData(chunk.toString(), result)
+    })
+
+    response.body.on('end', resolve)
+  })
+  return result
+}
+
+describe('app dir - rsc external dependency', () => {
+  let next: NextInstance
+
+  if ((global as any).isNextDeploy) {
+    it('should skip for deploy mode for now', () => {})
+    return
+  }
+
+  beforeAll(async () => {
+    next = await createNext({
+      files: new FileRef(path.join(__dirname, './rsc-external')),
+      dependencies: {
+        react: 'experimental',
+        'react-dom': 'experimental',
+      },
+      packageJson: {
+        scripts: {
+          setup: `cp -r ./node_modules_bak/* ./node_modules`,
+          build: 'yarn setup && next build',
+          dev: 'yarn setup && next dev',
+          start: 'next start',
+        },
+      },
+      installCommand: 'yarn',
+      startCommand: (global as any).isNextDev ? 'yarn dev' : 'yarn start',
+      buildCommand: 'yarn build',
+    })
+  })
+  afterAll(() => next.destroy())
+
+  const { isNextDeploy } = global as any
+  const isReact17 = process.env.NEXT_TEST_REACT_VERSION === '^17'
+  if (isNextDeploy || isReact17) {
+    it('should skip tests for next-deploy and react 17', () => {})
+    return
+  }
+
+  it('should be able to opt-out 3rd party packages being bundled in server components', async () => {
+    await fetchViaHTTP(next.url, '/react-server/optout').then(
+      async (response) => {
+        const result = await resolveStreamResponse(response)
+        expect(result).toContain('Server: index.default')
+        expect(result).toContain('Server subpath: subpath.default')
+        expect(result).toContain('Client: index.default')
+        expect(result).toContain('Client subpath: subpath.default')
+      }
+    )
+  })
+
+  it('should handle external async module libraries correctly', async () => {
+    const clientHtml = await renderViaHTTP(next.url, '/external-imports/client')
+    const serverHtml = await renderViaHTTP(next.url, '/external-imports/server')
+    const sharedHtml = await renderViaHTTP(next.url, '/shared-esm-dep')
+
+    expect(clientHtml).toContain('module type:esm-export')
+    expect(clientHtml).toContain('export named:named')
+    expect(clientHtml).toContain('export value:123')
+    expect(clientHtml).toContain('export array:4,5,6')
+    expect(clientHtml).toContain('export object:{x:1}')
+
+    // support esm module imports on server side, and indirect imports from shared components
+    expect(serverHtml).toContain('random-module-instance')
+    expect(sharedHtml).toContain(
+      'node_modules instance from client module random-module-instance'
+    )
+  })
+
+  it('should resolve the subset react in server components based on the react-server condition', async () => {
+    await fetchViaHTTP(next.url, '/react-server').then(async (response) => {
+      const result = await resolveStreamResponse(response)
+      expect(result).toContain('Server: <!-- -->subset')
+      expect(result).toContain('Client: <!-- -->full')
+    })
+  })
+
+  it('should resolve 3rd party package exports based on the react-server condition', async () => {
+    await fetchViaHTTP(next.url, '/react-server/3rd-party-package').then(
+      async (response) => {
+        const result = await resolveStreamResponse(response)
+
+        // Package should be resolved based on the react-server condition,
+        // as well as package's internal & external dependencies.
+        expect(result).toContain(
+          'Server: index.react-server:react.subset:dep.server'
+        )
+        expect(result).toContain('Client: index.default:react.full:dep.default')
+
+        // Subpath exports should be resolved based on the condition too.
+        expect(result).toContain('Server subpath: subpath.react-server')
+        expect(result).toContain('Client subpath: subpath.default')
+      }
+    )
+  })
+})

test/e2e/app-dir/rsc-external/app/layout.js

@@ -0,0 +1,8 @@
+export default function AppLayout({ children }) {
+  return (
+    <html>
+      <head></head>
+      <body>{children}</body>
+    </html>
+  )
+}

test/e2e/app-dir/rsc-external/app/shared-esm-dep/page.js

@@ -0,0 +1,5 @@
+import Random from '../../components/random-module-instance'
+
+export default function Page() {
+  return <Random />
+}

test/e2e/app-dir/rsc-external/components/random-module-instance.js

@@ -0,0 +1,7 @@
+'client'
+
+import { name } from 'random-module-instance'
+
+export default function () {
+  return `node_modules instance from client module ${name}`
+}

test/e2e/app-dir/rsc-external/next.config.js

@@ -0,0 +1,7 @@
+module.exports = {
+  reactStrictMode: true,
+  experimental: {
+    appDir: true,
+    serverComponentsExternalPackages: ['conditional-exports-optout'],
+  },
+}