Skip to content

Commit

Permalink
Prefer module over main on main fields for app router server compiler (
Browse files Browse the repository at this point in the history
…#56532)

This change is to pick the esm assets for RSC server layer and server rendering side, this is for production and development perf purpose, also to let them apply with the ESM related optimization such as tree-shaking, barrel files optimizations, etc.

We found a few packages that can't be optimized easily in bundling because we're using "main" field so the packages are not able to be tree-shaked, ending up with large bundle in the dist. This will change a lot for the bundling improvements as some packages are only having "main" and "module" field. So switching from CJS to ESM means better bundling, more optimization, and faster code.

#56501 was a precondition for this, as previously the bundling strategy was applied to some library but triggered the invalid hooks erroring.

### Other Monior Change
Previously we'll prefer to resolve CJS as there're 2 versions of react, using CJS assets will help let them pass by require-hook to use canary react for app router bundling assets. But now we changed the approach to bundling nextjs runtime and react packages. Now we dropped the condition that prefered to resolve CJS exports for externals, since if you're putting them in `serverComponentsExternalPackages`, they're not using the built-in react, so could potentially having trouble if any dependency is using react but excluded in bundles. So far we didn't see any report to this.

Closes NEXT-1286
  • Loading branch information
huozhi authored Oct 12, 2023
1 parent 62ab0e3 commit 6814ff1
Show file tree
Hide file tree
Showing 14 changed files with 99 additions and 47 deletions.
11 changes: 1 addition & 10 deletions packages/next/src/build/handle-externals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ export async function resolveExternal(
context: string,
request: string,
isEsmRequested: boolean,
hasAppDir: boolean,
getResolve: (
options: any
) => (
Expand All @@ -66,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
Expand Down Expand Up @@ -135,12 +130,10 @@ export function makeExternalHandler({
config,
optOutBundlingPackageRegex,
dir,
hasAppDir,
}: {
config: NextConfigComplete
optOutBundlingPackageRegex: RegExp
dir: string
hasAppDir: boolean
}) {
let resolvedExternalPackageDirs: Map<string, string>
const looseEsmExternals = config.experimental?.esmExternals === 'loose'
Expand Down Expand Up @@ -293,7 +286,6 @@ export function makeExternalHandler({
context,
request,
isEsmRequested,
hasAppDir,
getResolve,
isLocal ? resolveNextExternal : undefined
)
Expand Down Expand Up @@ -353,7 +345,6 @@ export function makeExternalHandler({
config.experimental.esmExternals,
context,
pkg + '/package.json',
hasAppDir,
isEsmRequested,
getResolve,
isLocal ? resolveNextExternal : undefined
Expand Down
40 changes: 40 additions & 0 deletions packages/next/src/build/webpack-config-rules/resolve.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import {
COMPILER_NAMES,
type CompilerNameValues,
} from '../../shared/lib/constants'

// exports.<conditionName>
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]
}
27 changes: 10 additions & 17 deletions packages/next/src/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ import { needsExperimentalReact } from '../lib/needs-experimental-react'
import { getDefineEnvPlugin } from './webpack/plugins/define-env-plugin'
import type { SWCLoaderOptions } from './webpack/loaders/next-swc-loader'
import { isResourceInPackages, makeExternalHandler } from './handle-externals'
import {
getMainField,
edgeConditionNames,
} from './webpack-config-rules/resolve'

type ExcludesFalse = <T>(x: T | false) => x is T
type ClientEntries = {
Expand Down Expand Up @@ -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.<conditionName>
const edgeConditionNames = [
'edge-light',
'worker',
// inherits the default conditions
'...',
]

// packageJson.<mainField>
const mainFieldsPerCompiler: Record<CompilerNameValues, string[]> = {
[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' ? ';' : ':')
Expand Down Expand Up @@ -931,7 +920,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,
}),
Expand Down Expand Up @@ -1039,7 +1029,6 @@ export default async function getBaseWebpackConfig(
config,
optOutBundlingPackageRegex,
dir,
hasAppDir,
})

const shouldIncludeExternalDirs =
Expand Down Expand Up @@ -1610,6 +1599,7 @@ export default async function getBaseWebpackConfig(
],
},
resolve: {
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
Expand Down Expand Up @@ -1754,6 +1744,9 @@ export default async function getBaseWebpackConfig(
],
exclude: [codeCondition.exclude],
use: swcLoaderForClientLayer,
resolve: {
mainFields: getMainField('app', compilerType),
},
},
]
: []),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -743,7 +743,6 @@ export class TraceEntryPointsPlugin implements webpack.WebpackPluginInstance {
context,
request,
isEsmRequested,
!!this.appDirEnabled,
(options) => (_: string, resRequest: string) => {
return getResolve(options)(parent, resRequest, job)
},
Expand Down
38 changes: 19 additions & 19 deletions test/e2e/app-dir/app-external/app-external.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,15 @@ 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)
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')
expect(result).toContain('opt-out-react-version: 18.2.0')
})
})

Expand Down Expand Up @@ -91,24 +92,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 () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -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'

Expand All @@ -11,6 +12,8 @@ export default function Page() {
{`Server subpath: ${v1}`}
<br />
<Client />
<br />
<div id="main-field">{`Server module field: ${serverFieldName}`}</div>
</div>
)
}
4 changes: 4 additions & 0 deletions test/e2e/app-dir/app-external/app/react-server/optout/page.js
Original file line number Diff line number Diff line change
@@ -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'

Expand All @@ -11,6 +12,9 @@ export default function Page() {
{`Server subpath: ${v1}`}
<br />
<Client />
<p id="optout-react-version">
{`opt-out-react-version: ${getReactVersion()}`}
</p>
</div>
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
"react-server": "./subpath.server.js",
"default": "./subpath.js"
},
"./react": {
"import": "./react.js"
},
"./package.json": "./package.json"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import React from 'react'

export function getReactVersion() {
return React.version
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
"react-server": "./subpath.server.js",
"default": "./subpath.js"
},
"./react": {
"import": "./react.js"
},
"./package.json": "./package.json"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import React from 'react'

export function getReactVersion() {
return React.version
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
exports.name = 'server-module-field:main'
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const name = 'server-module-field:module'
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"main": "./index.cjs",
"module": "./index.esm.js"
}

0 comments on commit 6814ff1

Please sign in to comment.