diff --git a/packages/next/src/build/handle-externals.ts b/packages/next/src/build/handle-externals.ts index 6db179278f33..bb26df1b8a4e 100644 --- a/packages/next/src/build/handle-externals.ts +++ b/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 - 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 } } diff --git a/packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts b/packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts index 932c5356c3e0..cdb25cce5dae 100644 --- a/packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts +++ b/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[] = [] - const actionMapsPerClientEntry: Record> = {} - - // 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() - 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[] = [] + const actionMapsPerClientEntry: Record> = {} + + // 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() + 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({ diff --git a/packages/next/src/server/app-render/create-component-tree.tsx b/packages/next/src/server/app-render/create-component-tree.tsx index e7a076f0baec..acee5b460119 100644 --- a/packages/next/src/server/app-render/create-component-tree.tsx +++ b/packages/next/src/server/app-render/create-component-tree.tsx @@ -265,7 +265,7 @@ async function createComponentTreeInternal({ } const LayoutOrPage: React.ComponentType | undefined = layoutOrPageMod - ? interopDefault(layoutOrPageMod) + ? await interopDefault(layoutOrPageMod) : undefined /** diff --git a/test/e2e/app-dir/app-external/app-external.test.ts b/test/e2e/app-dir/app-external/app-external.test.ts index 733e14baf455..0df01e0f7e0f 100644 --- a/test/e2e/app-dir/app-external/app-external.test.ts +++ b/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/) }) diff --git a/test/e2e/esm-externals/esm-externals.test.ts b/test/e2e/esm-externals/esm-externals.test.ts index 4832bb6b5881..e4ebc9e40cd5 100644 --- a/test/e2e/esm-externals/esm-externals.test.ts +++ b/test/e2e/esm-externals/esm-externals.test.ts @@ -13,13 +13,15 @@ describe('esm-externals', () => { const urls = ['/static', '/ssr', '/ssg'] for (const url of urls) { - // TODO fix webpack behavior + // For invalid esm packages that have "import" pointing to a non-esm-flagged module + // webpack is using the CJS version instead but Turbopack is opting out of + // externalizing and bundling the non-esm-flagged module. const expectedHtml = isTurbopack ? /Hello World\+World\+World\+World\+World\+World/ : url === '/static' ? /Hello World\+World\+Alternative\+World\+World\+World/ : /Hello World\+World\+Alternative\+World\+World\+Alternative/ - // TODO fix webpack behavior + // On client side, webpack always bundlings so it uses the non-esm-flagged module too. const expectedText = isTurbopack || url === '/static' ? /Hello World\+World\+World\+World\+World\+World/ @@ -40,18 +42,21 @@ describe('esm-externals', () => { // App dir { - // TODO App Dir doesn't use esmExternals: true correctly for webpack and Turbopack + // TODO App Dir doesn't use esmExternals: true correctly for Turbopack // so we only verify that the page doesn't crash, but ignore the actual content - // const expectedHtml = /Hello World\+World\+World/ - const expectedHtml = /Hello Wrong\+Wrong\+Alternative/ - // const expectedText = /Hello World\+World\+World/ + // const expectedHtml = isTurbopack ? /Hello World\+World\+World/ : /Hello World\+World\+Alternative/ + const expectedHtml = isTurbopack + ? /Hello Wrong\+Wrong\+Alternative/ + : /Hello World\+World\+Alternative/ const urls = ['/server', '/client'] for (const url of urls) { const expectedText = - url === '/server' + url !== '/server' + ? /Hello World\+World\+World/ + : isTurbopack ? /Hello Wrong\+Wrong\+Alternative/ - : /Hello World\+World\+World/ + : /Hello World\+World\+Alternative/ it(`should return the correct SSR HTML for ${url}`, async () => { const res = await next.fetch(url) const html = await res.text()