Support esm externals in app router (#65041)

### What

Support `esmExternals` working in app router

### Why

`esmExternals` was disabled for app router that most of the packages are
picking up the CJS bundles for externals. This PR enables to resolve the
ESM bundle for external packages.

We have two issues discovered while enabling the flag, any esm external
packages will fail in client page SSR and server action. We fixed them
by changing the below bundling logics:

* When a client page having a async dependency, we can await the page
during in rendering
* When a server action having a async dependency, we changed the server
action entry creation with webpack along with the server client entry
creation together, then webpack can handle the modules async propagation
properly.


Fixes #60756 
Closes NEXT-2435
Closes NEXT-2472
Closes NEXT-3225

---------

Co-authored-by: Tobias Koppers <tobias.koppers@googlemail.com>

Author: huozhi

packages/next/src/build/handle-externals.ts

@@ -1,5 +1,6 @@
-import { WEBPACK_LAYERS } from '../lib/constants'
 import type { WebpackLayerName } from '../lib/constants'
+import type { NextConfigComplete } from '../server/config-shared'
+import type { ResolveOptions } from 'webpack'
 import { defaultOverrides } from '../server/require-hook'
 import { BARREL_OPTIMIZATION_PREFIX } from '../shared/lib/constants'
 import path from '../shared/lib/isomorphic/path'
@@ -10,7 +11,6 @@ import {
   NODE_RESOLVE_OPTIONS,
 } from './webpack-config'
 import { isWebpackAppLayer, isWebpackServerOnlyLayer } from './utils'
-import type { NextConfigComplete } from '../server/config-shared'
 import { normalizePathSep } from '../shared/lib/page-path/normalize-path-sep'
 const reactPackagesRegex = /^(react|react-dom|react-server-dom-webpack)($|\/)/
 
@@ -25,15 +25,6 @@ const externalPattern = new RegExp(
 
 const nodeModulesRegex = /node_modules[/\\].*\.[mc]?js$/
 
-function containsImportInPackages(
-  request: string,
-  packages: string[]
-): boolean {
-  return packages.some(
-    (pkg) => request === pkg || request.startsWith(pkg + '/')
-  )
-}
-
 export function isResourceInPackages(
   resource: string,
   packageNames?: string[],
@@ -57,9 +48,9 @@ export async function resolveExternal(
   context: string,
   request: string,
   isEsmRequested: boolean,
-  optOutBundlingPackages: string[],
+  _optOutBundlingPackages: string[],
   getResolve: (
-    options: any
+    options: ResolveOptions
   ) => (
     resolveContext: string,
     resolveRequest: string
@@ -78,19 +69,12 @@ export async function resolveExternal(
   let isEsm: boolean = false
 
   const preferEsmOptions =
-    esmExternals &&
-    isEsmRequested &&
-    // For package that marked as externals that should be not bundled,
-    // we don't resolve them as ESM since it could be resolved as async module,
-    // such as `import(external package)` in the bundle, valued as a `Promise`.
-    !containsImportInPackages(request, optOutBundlingPackages)
-      ? [true, false]
-      : [false]
+    esmExternals && isEsmRequested ? [true, false] : [false]
 
   for (const preferEsm of preferEsmOptions) {
-    const resolve = getResolve(
-      preferEsm ? esmResolveOptions : nodeResolveOptions
-    )
+    const resolveOptions = preferEsm ? esmResolveOptions : nodeResolveOptions
+
+    const resolve = getResolve(resolveOptions)
 
     // Resolve the import with the webpack provided context, this
     // ensures we're resolving the correct version when multiple
@@ -273,23 +257,6 @@ export function makeExternalHandler({
       return resolveNextExternal(request)
     }
 
-    // Early return if the request needs to be bundled, such as in the client layer.
-    // Treat react packages and next internals as external for SSR layer,
-    // also map react to builtin ones with require-hook.
-    // Otherwise keep continue the process to resolve the externals.
-    if (layer === WEBPACK_LAYERS.serverSideRendering) {
-      const isRelative = request.startsWith('.')
-      const fullRequest = isRelative
-        ? normalizePathSep(path.join(context, request))
-        : request
-
-      // Check if it's opt out bundling package first
-      if (containsImportInPackages(fullRequest, optOutBundlingPackages)) {
-        return fullRequest
-      }
-      return resolveNextExternal(fullRequest)
-    }
-
     // TODO-APP: Let's avoid this resolve call as much as possible, and eventually get rid of it.
     const resolveResult = await resolveExternal(
       dir,
@@ -320,6 +287,13 @@ export function makeExternalHandler({
       return
     }
 
+    const isOptOutBundling = optOutBundlingPackageRegex.test(res)
+    // Apply bundling rules to all app layers.
+    // Since handleExternals only handle the server layers, we don't need to exclude client here
+    if (!isOptOutBundling && isAppLayer) {
+      return
+    }
+
     // ESM externals can only be imported (and not required).
     // Make an exception in loose mode.
     if (!isEsmRequested && isEsm && !looseEsmExternals && !isLocal) {
@@ -370,13 +344,11 @@ export function makeExternalHandler({
 
     const resolvedBundlingOptOutRes = resolveBundlingOptOutPackages({
       resolvedRes: res,
-      optOutBundlingPackageRegex,
       config,
       resolvedExternalPackageDirs,
-      isEsm,
       isAppLayer,
-      layer,
       externalType,
+      isOptOutBundling,
       request,
     })
     if (resolvedBundlingOptOutRes) {
@@ -390,41 +362,32 @@ export function makeExternalHandler({
 
 function resolveBundlingOptOutPackages({
   resolvedRes,
-  optOutBundlingPackageRegex,
   config,
   resolvedExternalPackageDirs,
-  isEsm,
   isAppLayer,
-  layer,
   externalType,
+  isOptOutBundling,
   request,
 }: {
   resolvedRes: string
-  optOutBundlingPackageRegex: RegExp
   config: NextConfigComplete
   resolvedExternalPackageDirs: Map<string, string>
-  isEsm: boolean
   isAppLayer: boolean
-  layer: WebpackLayerName | null
   externalType: string
+  isOptOutBundling: boolean
   request: string
 }) {
-  const shouldBeBundled =
-    isResourceInPackages(
-      resolvedRes,
-      config.transpilePackages,
-      resolvedExternalPackageDirs
-    ) ||
-    (isEsm && isAppLayer) ||
-    (!isAppLayer && config.bundlePagesRouterDependencies)
-
   if (nodeModulesRegex.test(resolvedRes)) {
-    const isOptOutBundling = optOutBundlingPackageRegex.test(resolvedRes)
-    if (isWebpackServerOnlyLayer(layer)) {
-      if (isOptOutBundling) {
-        return `${externalType} ${request}` // Externalize if opted out
-      }
-    } else if (!shouldBeBundled || isOptOutBundling) {
+    const shouldBundlePages =
+      !isAppLayer && config.bundlePagesRouterDependencies && !isOptOutBundling
+    const shouldBeBundled =
+      shouldBundlePages ||
+      isResourceInPackages(
+        resolvedRes,
+        config.transpilePackages,
+        resolvedExternalPackageDirs
+      )
+    if (!shouldBeBundled) {
       return `${externalType} ${request}` // Externalize if not bundled or opted out
     }
   }

packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts

@@ -430,75 +430,6 @@ export class FlightClientEntryPlugin {
       )
     }
 
-    compilation.hooks.finishModules.tapPromise(PLUGIN_NAME, async () => {
-      const addedClientActionEntryList: Promise<any>[] = []
-      const actionMapsPerClientEntry: Record<string, Map<string, string[]>> = {}
-
-      // We need to create extra action entries that are created from the
-      // client layer.
-      // Start from each entry's created SSR dependency from our previous step.
-      for (const [name, ssrEntryDependencies] of Object.entries(
-        createdSSRDependenciesForEntry
-      )) {
-        // Collect from all entries, e.g. layout.js, page.js, loading.js, ...
-        // add aggregate them.
-        const actionEntryImports = this.collectClientActionsFromDependencies({
-          compilation,
-          dependencies: ssrEntryDependencies,
-        })
-
-        if (actionEntryImports.size > 0) {
-          if (!actionMapsPerClientEntry[name]) {
-            actionMapsPerClientEntry[name] = new Map()
-          }
-          actionMapsPerClientEntry[name] = new Map([
-            ...actionMapsPerClientEntry[name],
-            ...actionEntryImports,
-          ])
-        }
-      }
-
-      for (const [name, actionEntryImports] of Object.entries(
-        actionMapsPerClientEntry
-      )) {
-        // If an action method is already created in the server layer, we don't
-        // need to create it again in the action layer.
-        // This is to avoid duplicate action instances and make sure the module
-        // state is shared.
-        let remainingClientImportedActions = false
-        const remainingActionEntryImports = new Map<string, string[]>()
-        for (const [dep, actionNames] of actionEntryImports) {
-          const remainingActionNames = []
-          for (const actionName of actionNames) {
-            const id = name + '@' + dep + '@' + actionName
-            if (!createdActions.has(id)) {
-              remainingActionNames.push(actionName)
-            }
-          }
-          if (remainingActionNames.length > 0) {
-            remainingActionEntryImports.set(dep, remainingActionNames)
-            remainingClientImportedActions = true
-          }
-        }
-
-        if (remainingClientImportedActions) {
-          addedClientActionEntryList.push(
-            this.injectActionEntry({
-              compiler,
-              compilation,
-              actions: remainingActionEntryImports,
-              entryName: name,
-              bundlePath: name,
-              fromClient: true,
-            })
-          )
-        }
-      }
-
-      await Promise.all(addedClientActionEntryList)
-      return
-    })
-
     // Invalidate in development to trigger recompilation
     const invalidator = getInvalidator(compiler.outputPath)
     // Check if any of the entry injections need an invalidation
@@ -521,6 +452,72 @@ export class FlightClientEntryPlugin {
 
     // Wait for action entries to be added.
     await Promise.all(addActionEntryList)
+
+    const addedClientActionEntryList: Promise<any>[] = []
+    const actionMapsPerClientEntry: Record<string, Map<string, string[]>> = {}
+
+    // We need to create extra action entries that are created from the
+    // client layer.
+    // Start from each entry's created SSR dependency from our previous step.
+    for (const [name, ssrEntryDependencies] of Object.entries(
+      createdSSRDependenciesForEntry
+    )) {
+      // Collect from all entries, e.g. layout.js, page.js, loading.js, ...
+      // add aggregate them.
+      const actionEntryImports = this.collectClientActionsFromDependencies({
+        compilation,
+        dependencies: ssrEntryDependencies,
+      })
+
+      if (actionEntryImports.size > 0) {
+        if (!actionMapsPerClientEntry[name]) {
+          actionMapsPerClientEntry[name] = new Map()
+        }
+        actionMapsPerClientEntry[name] = new Map([
+          ...actionMapsPerClientEntry[name],
+          ...actionEntryImports,
+        ])
+      }
+    }
+
+    for (const [name, actionEntryImports] of Object.entries(
+      actionMapsPerClientEntry
+    )) {
+      // If an action method is already created in the server layer, we don't
+      // need to create it again in the action layer.
+      // This is to avoid duplicate action instances and make sure the module
+      // state is shared.
+      let remainingClientImportedActions = false
+      const remainingActionEntryImports = new Map<string, string[]>()
+      for (const [dep, actionNames] of actionEntryImports) {
+        const remainingActionNames = []
+        for (const actionName of actionNames) {
+          const id = name + '@' + dep + '@' + actionName
+          if (!createdActions.has(id)) {
+            remainingActionNames.push(actionName)
+          }
+        }
+        if (remainingActionNames.length > 0) {
+          remainingActionEntryImports.set(dep, remainingActionNames)
+          remainingClientImportedActions = true
+        }
+      }
+
+      if (remainingClientImportedActions) {
+        addedClientActionEntryList.push(
+          this.injectActionEntry({
+            compiler,
+            compilation,
+            actions: remainingActionEntryImports,
+            entryName: name,
+            bundlePath: name,
+            fromClient: true,
+          })
+        )
+      }
+    }
+
+    await Promise.all(addedClientActionEntryList)
   }
 
   collectClientActionsFromDependencies({

packages/next/src/server/app-render/create-component-tree.tsx

@@ -265,7 +265,7 @@ async function createComponentTreeInternal({
   }
 
   const LayoutOrPage: React.ComponentType<any> | undefined = layoutOrPageMod
-    ? interopDefault(layoutOrPageMod)
+    ? await interopDefault(layoutOrPageMod)
     : undefined
 
   /**

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

@@ -16,7 +16,7 @@ async function resolveStreamResponse(response: any, onData?: any) {
 }
 
 describe('app dir - external dependency', () => {
-  const { next, skipped } = nextTestSetup({
+  const { next, skipped, isTurbopack } = nextTestSetup({
     files: __dirname,
     dependencies: {
       swr: 'latest',
@@ -279,14 +279,17 @@ describe('app dir - external dependency', () => {
   })
 
   describe('server actions', () => {
-    it('should not prefer to resolve esm over cjs for bundling optout packages', async () => {
+    it('should prefer to resolve esm over cjs for bundling optout packages', async () => {
       const browser = await next.browser('/optout/action')
       expect(await browser.elementByCss('#dual-pkg-outout p').text()).toBe('')
 
       browser.elementByCss('#dual-pkg-outout button').click()
       await check(async () => {
         const text = await browser.elementByCss('#dual-pkg-outout p').text()
-        expect(text).toBe('dual-pkg-optout:cjs')
+        // TODO: enable esm externals for app router in turbopack for actions
+        expect(text).toBe(
+          isTurbopack ? 'dual-pkg-optout:cjs' : 'dual-pkg-optout:mjs'
+        )
         return 'success'
       }, /success/)
     })