Support esm externals in app router

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,34 @@ 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.experimental.bundlePagesExternals)
-
   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.experimental.bundlePagesExternals &&
+      !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/)
     })

test/e2e/app-dir/app-external/app/optout/action/actions.js

@@ -1,7 +1,9 @@
 'use server'
 
 import { value as dualPkgOptoutValue } from 'dual-pkg-optout'
+import * as mod from 'dual-pkg-optout'
 
 export async function getDualOptoutValue() {
+  console.log('getDualOptoutValue', dualPkgOptoutValue, 'mod', mod)
   return dualPkgOptoutValue
 }

test/e2e/app-dir/app-external/app/optout/action/page.js

@@ -10,7 +10,9 @@ export default function Page() {
       <p>{optoutDisplayValue}</p>
       <button
         onClick={async () => {
-          setOptoutDisplayValue(await getDualOptoutValue())
+          const value = await getDualOptoutValue()
+          console.log('value', value, 'from', getDualOptoutValue)
+          setOptoutDisplayValue(value)
         }}
       >
         dual-pkg-optout