Skip to content

Commit

Permalink
Fix mixed module swc compilation for app router (#58967)
Browse files Browse the repository at this point in the history
  • Loading branch information
huozhi authored Dec 1, 2023
1 parent cc42e43 commit 87e0b44
Show file tree
Hide file tree
Showing 13 changed files with 131 additions and 29 deletions.
5 changes: 3 additions & 2 deletions packages/next/src/build/handle-externals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@ export function isResourceInPackages(
resource: string,
packageNames?: string[],
packageDirMapping?: Map<string, string>
) {
return packageNames?.some((p: string) =>
): boolean {
if (!packageNames) return false
return packageNames.some((p: string) =>
packageDirMapping && packageDirMapping.has(p)
? resource.startsWith(packageDirMapping.get(p)! + path.sep)
: resource.includes(
Expand Down
21 changes: 20 additions & 1 deletion packages/next/src/build/swc/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,22 @@ function getStyledComponentsOptions(
}
}

/*
Output module type
For app router where server components is enabled, we prefer to bundle es6 modules,
Use output module es6 to make sure:
- the esm module is present
- if the module is mixed syntax, the esm + cjs code are both present
For pages router will remain untouched
*/
function getModuleOptions(
esm: boolean | undefined = false
): { module: { type: 'es6' } } | {} {
return esm ? { module: { type: 'es6' } } : {}
}

function getEmotionOptions(
emotionConfig: undefined | boolean | EmotionConfig,
development: boolean
Expand Down Expand Up @@ -319,6 +335,7 @@ export function getLoaderSWCOptions({
relativeFilePathFromRoot,
serverComponents,
isReactServerLayer,
esm,
}: {
filename: string
development: boolean
Expand All @@ -338,6 +355,7 @@ export function getLoaderSWCOptions({
supportedBrowsers: string[] | undefined
swcCacheDir: string
relativeFilePathFromRoot: string
esm?: boolean
serverComponents?: boolean
isReactServerLayer?: boolean
}) {
Expand Down Expand Up @@ -412,6 +430,7 @@ export function getLoaderSWCOptions({
node: process.versions.node,
},
},
...getModuleOptions(esm),
}
} else {
const options = {
Expand All @@ -423,7 +442,7 @@ export function getLoaderSWCOptions({
type: 'commonjs',
},
}
: {}),
: getModuleOptions(esm)),
disableNextSsg: !isPageFile,
isDevelopment: development,
isServerCompiler: isServer,
Expand Down
14 changes: 7 additions & 7 deletions packages/next/src/build/webpack-config-rules/resolve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,20 @@ export const edgeConditionNames = [
]

const mainFieldsPerCompiler: Record<
CompilerNameValues | 'app-router-server',
CompilerNameValues | 'server-esm',
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'],
// For bundling-all strategy, prefer ESM over CJS
'server-esm': ['module', 'main'],
}

export function getMainField(
pageType: 'app' | 'pages',
compilerType: CompilerNameValues
compilerType: CompilerNameValues,
preferEsm: boolean
) {
if (compilerType === COMPILER_NAMES.edgeServer) {
return edgeConditionNames
Expand All @@ -34,7 +34,7 @@ export function getMainField(
}

// Prefer module fields over main fields for isomorphic packages on server layer
return pageType === 'app'
? mainFieldsPerCompiler['app-router-server']
return preferEsm
? mainFieldsPerCompiler['server-esm']
: mainFieldsPerCompiler[COMPILER_NAMES.server]
}
41 changes: 23 additions & 18 deletions packages/next/src/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -437,17 +437,26 @@ export default async function getBaseWebpackConfig(
}
}

// RSC loaders, prefer ESM, set `esm` to true
const swcServerLayerLoader = getSwcLoader({
serverComponents: true,
isReactServerLayer: true,
esm: true,
})
const swcClientLayerLoader = getSwcLoader({
serverComponents: true,
isReactServerLayer: false,
esm: true,
})
// Default swc loaders for pages doesn't prefer ESM.
const swcDefaultLoader = getSwcLoader({
serverComponents: true,
isReactServerLayer: false,
esm: false,
})

const defaultLoaders = {
babel: useSWCLoader ? swcClientLayerLoader : babelLoader!,
babel: useSWCLoader ? swcDefaultLoader : babelLoader!,
}

const swcLoaderForServerLayer = hasAppDir
Expand Down Expand Up @@ -621,7 +630,7 @@ export default async function getBaseWebpackConfig(
}
: undefined),
// default main fields use pages dir ones, and customize app router ones in loaders.
mainFields: getMainField('pages', compilerType),
mainFields: getMainField(compilerType, false),
...(isEdgeServer && {
conditionNames: edgeConditionNames,
}),
Expand Down Expand Up @@ -736,8 +745,13 @@ export default async function getBaseWebpackConfig(
const shouldIncludeExternalDirs =
config.experimental.externalDir || !!config.transpilePackages

function createLoaderRuleExclude(skipNodeModules: boolean) {
return (excludePath: string) => {
const codeCondition = {
test: /\.(tsx|ts|js|cjs|mjs|jsx)$/,
...(shouldIncludeExternalDirs
? // Allowing importing TS/TSX files from outside of the root dir.
{}
: { include: [dir, ...babelIncludeRegexes] }),
exclude: (excludePath: string) => {
if (babelIncludeRegexes.some((r) => r.test(excludePath))) {
return false
}
Expand All @@ -748,17 +762,8 @@ export default async function getBaseWebpackConfig(
)
if (shouldBeBundled) return false

return skipNodeModules && excludePath.includes('node_modules')
}
}

const codeCondition = {
test: /\.(tsx|ts|js|cjs|mjs|jsx)$/,
...(shouldIncludeExternalDirs
? // Allowing importing TS/TSX files from outside of the root dir.
{}
: { include: [dir, ...babelIncludeRegexes] }),
exclude: createLoaderRuleExclude(true),
return excludePath.includes('node_modules')
},
}

let webpackConfig: webpack.Configuration = {
Expand Down Expand Up @@ -1281,7 +1286,7 @@ export default async function getBaseWebpackConfig(
],
},
resolve: {
mainFields: getMainField('app', compilerType),
mainFields: getMainField(compilerType, true),
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 @@ -1416,15 +1421,15 @@ export default async function getBaseWebpackConfig(
issuerLayer: [WEBPACK_LAYERS.appPagesBrowser],
use: swcLoaderForClientLayer,
resolve: {
mainFields: getMainField('app', compilerType),
mainFields: getMainField(compilerType, true),
},
},
{
test: codeCondition.test,
issuerLayer: [WEBPACK_LAYERS.serverSideRendering],
use: swcLoaderForClientLayer,
resolve: {
mainFields: getMainField('app', compilerType),
mainFields: getMainField(compilerType, true),
},
},
]
Expand Down
3 changes: 3 additions & 0 deletions packages/next/src/build/webpack/loaders/next-swc-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export interface SWCLoaderOptions {
swcCacheDir: string
serverComponents?: boolean
isReactServerLayer?: boolean
esm?: boolean
}

async function loaderTransform(
Expand All @@ -69,6 +70,7 @@ async function loaderTransform(
swcCacheDir,
serverComponents,
isReactServerLayer,
esm,
} = loaderOptions
const isPageFile = filename.startsWith(pagesDir)
const relativeFilePathFromRoot = path.relative(rootDir, filename)
Expand All @@ -92,6 +94,7 @@ async function loaderTransform(
relativeFilePathFromRoot,
serverComponents,
isReactServerLayer,
esm,
})

const programmaticOptions = {
Expand Down
18 changes: 18 additions & 0 deletions test/e2e/app-dir/app-external/app-external.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,24 @@ createNextDescribe(
})
})

describe('mixed syntax external modules', () => {
it('should handle mixed module with next/dynamic', async () => {
const browser = await next.browser('/mixed/dynamic')
expect(await browser.elementByCss('#component').text()).toContain(
'mixed-syntax-esm'
)
})

it('should handle mixed module in server and client components', async () => {
const $ = await next.render$('/mixed/import')
expect(await $('#server').text()).toContain('server:mixed-syntax-esm')
expect(await $('#client').text()).toContain('client:mixed-syntax-esm')
expect(await $('#relative-mixed').text()).toContain(
'relative-mixed-syntax-esm'
)
})
})

it('should export client module references in esm', async () => {
const html = await next.render('/esm-client-ref')
expect(html).toContain('hello')
Expand Down
12 changes: 12 additions & 0 deletions test/e2e/app-dir/app-external/app/mixed/dynamic/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
'use client'

import dynamic from 'next/dynamic'

const Dynamic = dynamic(
() => import('mixed-syntax-esm').then((mod) => mod.Component),
{ ssr: false }
)

export default function Page() {
return <Dynamic />
}
7 changes: 7 additions & 0 deletions test/e2e/app-dir/app-external/app/mixed/import/client.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
'use client'

import { value } from 'mixed-syntax-esm'

export function Client() {
return 'client:' + value
}
5 changes: 5 additions & 0 deletions test/e2e/app-dir/app-external/app/mixed/import/mixed-mod.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
;(module) => {
module.exports = {}
}

export const value = 'relative-mixed-syntax-esm'
15 changes: 15 additions & 0 deletions test/e2e/app-dir/app-external/app/mixed/import/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { value } from 'mixed-syntax-esm'
import { Client } from './client'
import { value as relativeMixedValue } from './mixed-mod.mjs'

export default function Page() {
return (
<>
<p id="server">{'server:' + value}</p>
<p id="client">
<Client />
</p>
<p id="relative-mixed">{relativeMixedValue}</p>
</>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import React from 'react'
;(module) => {
module.exports = {}
}

export const value = 'mixed-syntax-esm'

export function Component() {
return /*#__PURE__*/ React.createElement(
'p',
{ id: 'component' },
'mixed-syntax-esm'
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"exports": "./index.mjs"
}
2 changes: 1 addition & 1 deletion test/e2e/app-dir/third-parties/app/gtm/page.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const Page = () => {
}

return (
<div class="container">
<div className="container">
<GoogleTagManager gtmId="GTM-XYZ" />
<h1>GTM</h1>
<button id="gtm-send" onClick={onClick}>
Expand Down

0 comments on commit 87e0b44

Please sign in to comment.