From 8d019eac4083e9c6d388b5f83595a9112b2b44c3 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Thu, 15 Jun 2023 11:24:30 +0200 Subject: [PATCH 1/7] Revert "Revert "Prefer module fields for RSC server layer" (#51316)" This reverts commit 409668107d3adb29c55860527855a2fb4834a770. --- packages/next/src/build/webpack-config.ts | 4 +++ .../app-dir/app-external/app-external.test.ts | 35 +++++++++---------- .../react-server/3rd-party-package/page.js | 3 ++ .../server-module-field/index.cjs | 1 + .../server-module-field/index.esm.js | 1 + .../server-module-field/package.json | 4 +++ 6 files changed, 30 insertions(+), 18 deletions(-) create mode 100644 test/e2e/app-dir/app-external/node_modules_bak/server-module-field/index.cjs create mode 100644 test/e2e/app-dir/app-external/node_modules_bak/server-module-field/index.esm.js create mode 100644 test/e2e/app-dir/app-external/node_modules_bak/server-module-field/package.json diff --git a/packages/next/src/build/webpack-config.ts b/packages/next/src/build/webpack-config.ts index 2681305d6fb49..1d065adf81e5b 100644 --- a/packages/next/src/build/webpack-config.ts +++ b/packages/next/src/build/webpack-config.ts @@ -1597,6 +1597,10 @@ export default async function getBaseWebpackConfig( ], }, resolve: { + mainFields: isEdgeServer + ? mainFieldsPerCompiler[COMPILER_NAMES.edgeServer] + : // Prefer module fields over main fields for isomorphic packages on server layer + ['module', 'main'], conditionNames: reactServerCondition, // If missing the alias override here, the default alias will be used which aliases // react to the direct file path, not the package name. In that case the condition 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 38592f5ba1b47..4fd527d5a9044 100644 --- a/test/e2e/app-dir/app-external/app-external.test.ts +++ b/test/e2e/app-dir/app-external/app-external.test.ts @@ -91,24 +91,23 @@ createNextDescribe( }) it('should resolve 3rd party package exports based on the react-server condition', async () => { - await next - .fetch('/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') - }) + const $ = await next.render$('/react-server/3rd-party-package') + + const result = $('body').text() + + // 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') + + // Prefer `module` field for isomorphic packages. + expect($('#main-field').text()).toContain('server-module-field:module') }) it('should correctly collect global css imports and mark them as side effects', async () => { diff --git a/test/e2e/app-dir/app-external/app/react-server/3rd-party-package/page.js b/test/e2e/app-dir/app-external/app/react-server/3rd-party-package/page.js index 33141e12f7685..92e9f01672faa 100644 --- a/test/e2e/app-dir/app-external/app/react-server/3rd-party-package/page.js +++ b/test/e2e/app-dir/app-external/app/react-server/3rd-party-package/page.js @@ -1,5 +1,6 @@ import v from 'conditional-exports' import v1 from 'conditional-exports/subpath' +import { name as serverFieldName } from 'server-module-field' import Client from './client' @@ -11,6 +12,8 @@ export default function Page() { {`Server subpath: ${v1}`}
+
+
{`Server module field: ${serverFieldName}`}
) } diff --git a/test/e2e/app-dir/app-external/node_modules_bak/server-module-field/index.cjs b/test/e2e/app-dir/app-external/node_modules_bak/server-module-field/index.cjs new file mode 100644 index 0000000000000..bead07e159aa3 --- /dev/null +++ b/test/e2e/app-dir/app-external/node_modules_bak/server-module-field/index.cjs @@ -0,0 +1 @@ +exports.name = 'server-module-field:main' diff --git a/test/e2e/app-dir/app-external/node_modules_bak/server-module-field/index.esm.js b/test/e2e/app-dir/app-external/node_modules_bak/server-module-field/index.esm.js new file mode 100644 index 0000000000000..02218634f7d07 --- /dev/null +++ b/test/e2e/app-dir/app-external/node_modules_bak/server-module-field/index.esm.js @@ -0,0 +1 @@ +export const name = 'server-module-field:module' diff --git a/test/e2e/app-dir/app-external/node_modules_bak/server-module-field/package.json b/test/e2e/app-dir/app-external/node_modules_bak/server-module-field/package.json new file mode 100644 index 0000000000000..d6cb0ed97cb3d --- /dev/null +++ b/test/e2e/app-dir/app-external/node_modules_bak/server-module-field/package.json @@ -0,0 +1,4 @@ +{ + "main": "./index.cjs", + "module": "./index.esm.js" +} From 8df4ae49a2a65d65011946e4629b57fe82c7462a Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Fri, 6 Oct 2023 17:44:30 +0200 Subject: [PATCH 2/7] refactor main field resolver --- .../src/build/webpack-config-rules/resolve.ts | 37 +++++++++++++++++++ packages/next/src/build/webpack-config.ts | 30 +++++---------- 2 files changed, 47 insertions(+), 20 deletions(-) create mode 100644 packages/next/src/build/webpack-config-rules/resolve.ts diff --git a/packages/next/src/build/webpack-config-rules/resolve.ts b/packages/next/src/build/webpack-config-rules/resolve.ts new file mode 100644 index 0000000000000..be2010d9b5b30 --- /dev/null +++ b/packages/next/src/build/webpack-config-rules/resolve.ts @@ -0,0 +1,37 @@ +import { COMPILER_NAMES, CompilerNameValues } from '../../shared/lib/constants' + +// exports. +export const edgeConditionNames = [ + 'edge-light', + 'worker', + // inherits the default conditions + '...', +] + +const mainFieldsPerCompiler: Record< + CompilerNameValues | 'app-router-server', + string[] +> = { + // For default case, prefer CJS over ESM on server side. e.g. pages dir SSR + [COMPILER_NAMES.server]: ['main', 'module'], + [COMPILER_NAMES.client]: ['browser', 'module', 'main'], + [COMPILER_NAMES.edgeServer]: edgeConditionNames, + // For app router since everything is bundled, prefer ESM over CJS + ['app-router-server']: ['module', 'main'], +} + +export function getMainField( + pageType: 'app' | 'pages', + compilerType: CompilerNameValues +) { + if (compilerType === COMPILER_NAMES.edgeServer) { + return edgeConditionNames + } else if (compilerType === COMPILER_NAMES.client) { + return mainFieldsPerCompiler[COMPILER_NAMES.client] + } + + // Prefer module fields over main fields for isomorphic packages on server layer + return pageType === 'app' + ? mainFieldsPerCompiler['app-router-server'] + : mainFieldsPerCompiler[COMPILER_NAMES.server] +} diff --git a/packages/next/src/build/webpack-config.ts b/packages/next/src/build/webpack-config.ts index 1d065adf81e5b..7daecf18ca28f 100644 --- a/packages/next/src/build/webpack-config.ts +++ b/packages/next/src/build/webpack-config.ts @@ -74,6 +74,10 @@ import { needsExperimentalReact } from '../lib/needs-experimental-react' import { getDefineEnvPlugin } from './webpack/plugins/define-env-plugin' import { SWCLoaderOptions } from './webpack/loaders/next-swc-loader' import { isResourceInPackages, makeExternalHandler } from './handle-externals' +import { + getMainField, + edgeConditionNames, +} from './webpack-config-rules/resolve' type ExcludesFalse = (x: T | false) => x is T type ClientEntries = { @@ -104,21 +108,6 @@ const babelIncludeRegexes: RegExp[] = [ const asyncStoragesRegex = /next[\\/]dist[\\/](esm[\\/])?client[\\/]components[\\/](static-generation-async-storage|action-async-storage|request-async-storage)/ -// exports. -const edgeConditionNames = [ - 'edge-light', - 'worker', - // inherits the default conditions - '...', -] - -// packageJson. -const mainFieldsPerCompiler: Record = { - [COMPILER_NAMES.server]: ['main', 'module'], - [COMPILER_NAMES.client]: ['browser', 'module', 'main'], - [COMPILER_NAMES.edgeServer]: edgeConditionNames, -} - // Support for NODE_PATH const nodePathList = (process.env.NODE_PATH || '') .split(process.platform === 'win32' ? ';' : ':') @@ -918,7 +907,8 @@ export default async function getBaseWebpackConfig( }, } : undefined), - mainFields: mainFieldsPerCompiler[compilerType], + // default main fields use pages dir ones, and customize app router ones in loaders. + mainFields: getMainField('pages', compilerType), ...(isEdgeServer && { conditionNames: edgeConditionNames, }), @@ -1597,10 +1587,7 @@ export default async function getBaseWebpackConfig( ], }, resolve: { - mainFields: isEdgeServer - ? mainFieldsPerCompiler[COMPILER_NAMES.edgeServer] - : // Prefer module fields over main fields for isomorphic packages on server layer - ['module', 'main'], + mainFields: getMainField('app', 'server'), conditionNames: reactServerCondition, // If missing the alias override here, the default alias will be used which aliases // react to the direct file path, not the package name. In that case the condition @@ -1745,6 +1732,9 @@ export default async function getBaseWebpackConfig( ], exclude: [codeCondition.exclude], use: swcLoaderForClientLayer, + resolve: { + mainFields: getMainField('app', 'server'), + }, }, ] : []), From 8987c68af733fafcb93e49c9ffc3b17c5d4a92cd Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Fri, 6 Oct 2023 18:04:57 +0200 Subject: [PATCH 3/7] fix compiler type --- packages/next/src/build/handle-externals.ts | 6 +----- packages/next/src/build/webpack-config.ts | 4 ++-- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/packages/next/src/build/handle-externals.ts b/packages/next/src/build/handle-externals.ts index a52f9dea1a021..d45bec5b2a395 100644 --- a/packages/next/src/build/handle-externals.ts +++ b/packages/next/src/build/handle-externals.ts @@ -65,11 +65,7 @@ export async function resolveExternal( let preferEsmOptions = esmExternals && isEsmRequested ? [true, false] : [false] - // Disable esm resolving for app/ and pages/ so for esm package using under pages/ - // won't load react through esm loader - if (hasAppDir) { - preferEsmOptions = [false] - } + for (const preferEsm of preferEsmOptions) { const resolve = getResolve( preferEsm ? esmResolveOptions : nodeResolveOptions diff --git a/packages/next/src/build/webpack-config.ts b/packages/next/src/build/webpack-config.ts index 7daecf18ca28f..654e02d12f204 100644 --- a/packages/next/src/build/webpack-config.ts +++ b/packages/next/src/build/webpack-config.ts @@ -1587,7 +1587,7 @@ export default async function getBaseWebpackConfig( ], }, resolve: { - mainFields: getMainField('app', 'server'), + mainFields: getMainField('app', compilerType), conditionNames: reactServerCondition, // If missing the alias override here, the default alias will be used which aliases // react to the direct file path, not the package name. In that case the condition @@ -1733,7 +1733,7 @@ export default async function getBaseWebpackConfig( exclude: [codeCondition.exclude], use: swcLoaderForClientLayer, resolve: { - mainFields: getMainField('app', 'server'), + mainFields: getMainField('app', compilerType), }, }, ] From 85d0547fd4e13f6185981105a840988af52ea7c3 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Fri, 6 Oct 2023 18:13:46 +0200 Subject: [PATCH 4/7] remove flag --- packages/next/src/build/handle-externals.ts | 3 --- .../src/build/webpack/plugins/next-trace-entrypoints-plugin.ts | 1 - 2 files changed, 4 deletions(-) diff --git a/packages/next/src/build/handle-externals.ts b/packages/next/src/build/handle-externals.ts index d45bec5b2a395..be57b82f07027 100644 --- a/packages/next/src/build/handle-externals.ts +++ b/packages/next/src/build/handle-externals.ts @@ -43,7 +43,6 @@ export async function resolveExternal( context: string, request: string, isEsmRequested: boolean, - hasAppDir: boolean, getResolve: ( options: any ) => ( @@ -288,7 +287,6 @@ export function makeExternalHandler({ context, request, isEsmRequested, - hasAppDir, getResolve, isLocal ? resolveNextExternal : undefined ) @@ -348,7 +346,6 @@ export function makeExternalHandler({ config.experimental.esmExternals, context, pkg + '/package.json', - hasAppDir, isEsmRequested, getResolve, isLocal ? resolveNextExternal : undefined diff --git a/packages/next/src/build/webpack/plugins/next-trace-entrypoints-plugin.ts b/packages/next/src/build/webpack/plugins/next-trace-entrypoints-plugin.ts index 7ce133dbb774d..ac399803acd0f 100644 --- a/packages/next/src/build/webpack/plugins/next-trace-entrypoints-plugin.ts +++ b/packages/next/src/build/webpack/plugins/next-trace-entrypoints-plugin.ts @@ -745,7 +745,6 @@ export class TraceEntryPointsPlugin implements webpack.WebpackPluginInstance { context, request, isEsmRequested, - !!this.appDirEnabled, (options) => (_: string, resRequest: string) => { return getResolve(options)(parent, resRequest, job) }, From 4caf04b400c7f29061931c5cb847260987b42853 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Fri, 6 Oct 2023 18:23:07 +0200 Subject: [PATCH 5/7] fix lint --- packages/next/src/build/handle-externals.ts | 2 -- packages/next/src/build/webpack-config-rules/resolve.ts | 2 +- packages/next/src/build/webpack-config.ts | 1 - 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/next/src/build/handle-externals.ts b/packages/next/src/build/handle-externals.ts index be57b82f07027..e825631c62824 100644 --- a/packages/next/src/build/handle-externals.ts +++ b/packages/next/src/build/handle-externals.ts @@ -129,12 +129,10 @@ export function makeExternalHandler({ config, optOutBundlingPackageRegex, dir, - hasAppDir, }: { config: NextConfigComplete optOutBundlingPackageRegex: RegExp dir: string - hasAppDir: boolean }) { let resolvedExternalPackageDirs: Map const looseEsmExternals = config.experimental?.esmExternals === 'loose' diff --git a/packages/next/src/build/webpack-config-rules/resolve.ts b/packages/next/src/build/webpack-config-rules/resolve.ts index be2010d9b5b30..d57543d4d868d 100644 --- a/packages/next/src/build/webpack-config-rules/resolve.ts +++ b/packages/next/src/build/webpack-config-rules/resolve.ts @@ -17,7 +17,7 @@ const mainFieldsPerCompiler: Record< [COMPILER_NAMES.client]: ['browser', 'module', 'main'], [COMPILER_NAMES.edgeServer]: edgeConditionNames, // For app router since everything is bundled, prefer ESM over CJS - ['app-router-server']: ['module', 'main'], + 'app-router-server': ['module', 'main'], } export function getMainField( diff --git a/packages/next/src/build/webpack-config.ts b/packages/next/src/build/webpack-config.ts index 654e02d12f204..fcf99d5c6969b 100644 --- a/packages/next/src/build/webpack-config.ts +++ b/packages/next/src/build/webpack-config.ts @@ -1016,7 +1016,6 @@ export default async function getBaseWebpackConfig( config, optOutBundlingPackageRegex, dir, - hasAppDir, }) const shouldIncludeExternalDirs = From 07f1d646f702460b4854ba8b831f874c6ef9c5c1 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Fri, 6 Oct 2023 19:07:02 +0200 Subject: [PATCH 6/7] add test for external opt out --- test/e2e/app-dir/app-external/app-external.test.ts | 3 ++- .../e2e/app-dir/app-external/app/react-server/optout/page.js | 4 ++++ .../node_modules_bak/conditional-exports-optout/package.json | 3 +++ .../node_modules_bak/conditional-exports-optout/react.js | 5 +++++ .../node_modules_bak/conditional-exports/package.json | 3 +++ .../node_modules_bak/conditional-exports/react.js | 5 +++++ 6 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 test/e2e/app-dir/app-external/node_modules_bak/conditional-exports-optout/react.js create mode 100644 test/e2e/app-dir/app-external/node_modules_bak/conditional-exports/react.js 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 4fd527d5a9044..b4e66cb205966 100644 --- a/test/e2e/app-dir/app-external/app-external.test.ts +++ b/test/e2e/app-dir/app-external/app-external.test.ts @@ -39,7 +39,7 @@ createNextDescribe( buildCommand: 'yarn build', skipDeployment: true, }, - ({ next, isNextDev }) => { + ({ next }) => { it('should be able to opt-out 3rd party packages being bundled in server components', async () => { await next.fetch('/react-server/optout').then(async (response) => { const result = await resolveStreamResponse(response) @@ -47,6 +47,7 @@ createNextDescribe( expect(result).toContain('Server subpath: subpath.default') expect(result).toContain('Client: index.default') expect(result).toContain('Client subpath: subpath.default') + expect(result).toContain('opt-out-react-version: 18.2.0') }) }) diff --git a/test/e2e/app-dir/app-external/app/react-server/optout/page.js b/test/e2e/app-dir/app-external/app/react-server/optout/page.js index fc7bd55ab86c3..45ba1ccff0358 100644 --- a/test/e2e/app-dir/app-external/app/react-server/optout/page.js +++ b/test/e2e/app-dir/app-external/app/react-server/optout/page.js @@ -1,5 +1,6 @@ import v from 'conditional-exports-optout' import v1 from 'conditional-exports-optout/subpath' +import { getReactVersion } from 'conditional-exports-optout/react' import Client from './client' @@ -11,6 +12,9 @@ export default function Page() { {`Server subpath: ${v1}`}
+

+ {`opt-out-react-version: ${getReactVersion()}`} +

) } diff --git a/test/e2e/app-dir/app-external/node_modules_bak/conditional-exports-optout/package.json b/test/e2e/app-dir/app-external/node_modules_bak/conditional-exports-optout/package.json index fe1b70d109b2a..3355c20aad28a 100644 --- a/test/e2e/app-dir/app-external/node_modules_bak/conditional-exports-optout/package.json +++ b/test/e2e/app-dir/app-external/node_modules_bak/conditional-exports-optout/package.json @@ -10,6 +10,9 @@ "react-server": "./subpath.server.js", "default": "./subpath.js" }, + "./react": { + "import": "./react.js" + }, "./package.json": "./package.json" } } diff --git a/test/e2e/app-dir/app-external/node_modules_bak/conditional-exports-optout/react.js b/test/e2e/app-dir/app-external/node_modules_bak/conditional-exports-optout/react.js new file mode 100644 index 0000000000000..4f2c2283ed693 --- /dev/null +++ b/test/e2e/app-dir/app-external/node_modules_bak/conditional-exports-optout/react.js @@ -0,0 +1,5 @@ +import React from 'react' + +export function getReactVersion() { + return React.version +} diff --git a/test/e2e/app-dir/app-external/node_modules_bak/conditional-exports/package.json b/test/e2e/app-dir/app-external/node_modules_bak/conditional-exports/package.json index b51ade2e7acfe..06e09e177ae16 100644 --- a/test/e2e/app-dir/app-external/node_modules_bak/conditional-exports/package.json +++ b/test/e2e/app-dir/app-external/node_modules_bak/conditional-exports/package.json @@ -16,6 +16,9 @@ "react-server": "./subpath.server.js", "default": "./subpath.js" }, + "./react": { + "import": "./react.js" + }, "./package.json": "./package.json" } } diff --git a/test/e2e/app-dir/app-external/node_modules_bak/conditional-exports/react.js b/test/e2e/app-dir/app-external/node_modules_bak/conditional-exports/react.js new file mode 100644 index 0000000000000..4f2c2283ed693 --- /dev/null +++ b/test/e2e/app-dir/app-external/node_modules_bak/conditional-exports/react.js @@ -0,0 +1,5 @@ +import React from 'react' + +export function getReactVersion() { + return React.version +} From 1454077854521b1e24ff88c3debec4749ede197b Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Tue, 10 Oct 2023 23:34:10 +0200 Subject: [PATCH 7/7] fix ts --- packages/next/src/build/webpack-config-rules/resolve.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/next/src/build/webpack-config-rules/resolve.ts b/packages/next/src/build/webpack-config-rules/resolve.ts index d57543d4d868d..f50f6c92ee629 100644 --- a/packages/next/src/build/webpack-config-rules/resolve.ts +++ b/packages/next/src/build/webpack-config-rules/resolve.ts @@ -1,4 +1,7 @@ -import { COMPILER_NAMES, CompilerNameValues } from '../../shared/lib/constants' +import { + COMPILER_NAMES, + type CompilerNameValues, +} from '../../shared/lib/constants' // exports. export const edgeConditionNames = [