Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix mixed module swc compilation for app router #58967

Merged
merged 20 commits into from
Dec 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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