Skip to content

Commit

Permalink
Fix barrel optimization to ignore layers (vercel#59254)
Browse files Browse the repository at this point in the history
Fixes vercel#57624. The recent issue was an unexpected side effect caused by
vercel@305bb01,
which only affects specific packages like `@mui/material`.

The problem was that the entry file of `@mui/material` has `"use
client"` at top, which affects the compilation result to output
reference info only (when on the RSC layer), instead of keeping the
original export statements. And the fix here is to ignore all layer info
and React specific transforms here, as barrel optimization isn't related
to all these framework features at all. To keep all directives
unchanged, the SWC transform needs to parse and pass that info to the
Webpack loader.

This PR adds a test to ensure that `@mui/material` is working as
expected (less than 1500 modules compiled). Without this feature it'll
be ~2400 modules.

Closes NEXT-1793, closes NEXT-1762.
  • Loading branch information
shuding authored Dec 5, 2023
1 parent b88e263 commit 6790004
Show file tree
Hide file tree
Showing 10 changed files with 162 additions and 54 deletions.
42 changes: 40 additions & 2 deletions packages/next-swc/crates/core/src/optimize_barrel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,16 @@ impl Fold for OptimizeBarrel {

// We only apply this optimization to barrel files. Here we consider
// a barrel file to be a file that only exports from other modules.

// Besides that, lit expressions are allowed as well ("use client", etc.).
let mut allowed_directives = true;
let mut directives = vec![];

let mut is_barrel = true;
for item in &items {
match item {
ModuleItem::ModuleDecl(decl) => {
allowed_directives = false;
match decl {
ModuleDecl::Import(_) => {}
// export { foo } from './foo';
Expand Down Expand Up @@ -198,17 +203,25 @@ impl Fold for OptimizeBarrel {
}
ModuleItem::Stmt(stmt) => match stmt {
Stmt::Expr(expr) => match &*expr.expr {
Expr::Lit(_) => {
new_items.push(item.clone());
Expr::Lit(l) => {
if let Lit::Str(s) = l {
if allowed_directives && s.value.starts_with("use ") {
directives.push(s.value.to_string());
}
} else {
allowed_directives = false;
}
}
_ => {
allowed_directives = false;
if !self.wildcard {
is_barrel = false;
break;
}
}
},
_ => {
allowed_directives = false;
if !self.wildcard {
is_barrel = false;
break;
Expand Down Expand Up @@ -259,6 +272,31 @@ impl Fold for OptimizeBarrel {
type_only: false,
})));
}

// Push directives.
if !directives.is_empty() {
new_items.push(ModuleItem::ModuleDecl(ModuleDecl::ExportDecl(ExportDecl {
span: DUMMY_SP,
decl: Decl::Var(Box::new(VarDecl {
span: DUMMY_SP,
kind: VarDeclKind::Const,
declare: false,
decls: vec![VarDeclarator {
span: DUMMY_SP,
name: Pat::Ident(BindingIdent {
id: private_ident!("__next_private_directive_list__"),
type_ann: None,
}),
init: Some(Box::new(Expr::Lit(Lit::Str(Str {
span: DUMMY_SP,
value: serde_json::to_string(&directives).unwrap().into(),
raw: None,
})))),
definite: false,
}],
})),
})));
}
}

new_items
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
/* Test */
/* Test directives inside comments
'use server'
*/

// This should be kept
'use client'

import foo, { a, b } from 'foo'
Expand All @@ -7,3 +13,7 @@ export { a as x }
export { y } from '1'
export { b }
export { foo as default, z }

// This should be removed as it's not on top
// prettier-ignore
'use strict'
Original file line number Diff line number Diff line change
@@ -1,2 +1,5 @@
'use client';
/* Test */ /* Test directives inside comments
'use server'
*/ // This should be kept
export const __next_private_export_map__ = '[["x","foo","a"],["y","1","y"],["b","foo","b"],["default","foo","default"],["z","bar","default"]]';
export const __next_private_directive_list__ = '["use client"]';
67 changes: 35 additions & 32 deletions packages/next/src/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -746,7 +746,7 @@ export default async function getBaseWebpackConfig(
config.experimental.externalDir || !!config.transpilePackages

const codeCondition = {
test: /\.(tsx|ts|js|cjs|mjs|jsx)$/,
test: { or: [/\.(tsx|ts|js|cjs|mjs|jsx)$/, /__barrel_optimize__/] },
...(shouldIncludeExternalDirs
? // Allowing importing TS/TSX files from outside of the root dir.
{}
Expand Down Expand Up @@ -1124,37 +1124,6 @@ export default async function getBaseWebpackConfig(
},
module: {
rules: [
{
// This loader rule works like a bridge between user's import and
// the target module behind a package's barrel file. It reads SWC's
// analysis result from the previous loader, and directly returns the
// code that only exports values that are asked by the user.
test: /__barrel_optimize__/,
use: ({ resourceQuery }: { resourceQuery: string }) => {
const names = (
resourceQuery.match(/\?names=([^&]+)/)?.[1] || ''
).split(',')

return [
{
loader: 'next-barrel-loader',
options: {
names,
swcCacheDir: path.join(
dir,
config?.distDir ?? '.next',
'cache',
'swc'
),
},
// This is part of the request value to serve as the module key.
// The barrel loader are no-op re-exported modules keyed by
// export names.
ident: 'next-barrel-loader:' + resourceQuery,
},
]
},
},
// Alias server-only and client-only to proper exports based on bundling layers
{
issuerLayer: {
Expand Down Expand Up @@ -1580,6 +1549,40 @@ export default async function getBaseWebpackConfig(
test: /[\\/]next[\\/]dist[\\/](esm[\\/])?server[\\/]og[\\/]image-response\.js/,
sideEffects: false,
},
{
// This loader rule should be before other rules, as it can output code
// that still contains `"use client"` or `"use server"` statements that
// needs to be re-transformed by the RSC compilers.
// This loader rule works like a bridge between user's import and
// the target module behind a package's barrel file. It reads SWC's
// analysis result from the previous loader, and directly returns the
// code that only exports values that are asked by the user.
test: /__barrel_optimize__/,
use: ({ resourceQuery }: { resourceQuery: string }) => {
const names = (
resourceQuery.match(/\?names=([^&]+)/)?.[1] || ''
).split(',')

return [
{
loader: 'next-barrel-loader',
options: {
names,
swcCacheDir: path.join(
dir,
config?.distDir ?? '.next',
'cache',
'swc'
),
},
// This is part of the request value to serve as the module key.
// The barrel loader are no-op re-exported modules keyed by
// export names.
ident: 'next-barrel-loader:' + resourceQuery,
},
]
},
},
],
},
plugins: [
Expand Down
45 changes: 30 additions & 15 deletions packages/next/src/build/webpack/loaders/next-barrel-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ import type webpack from 'webpack'

import path from 'path'
import { transform } from '../../swc'
import { WEBPACK_LAYERS } from '../../../lib/constants'

// This is a in-memory cache for the mapping of barrel exports. This only applies
// to the packages that we optimize. It will never change (e.g. upgrading packages)
Expand All @@ -97,14 +96,13 @@ import { WEBPACK_LAYERS } from '../../../lib/constants'
const barrelTransformMappingCache = new Map<
string,
{
prefix: string
exportList: [string, string, string][]
wildcardExports: string[]
isClientEntry: boolean
} | null
>()

async function getBarrelMapping(
layer: string | null | undefined,
resourcePath: string,
swcCacheDir: string,
resolve: (context: string, request: string) => Promise<string>,
Expand Down Expand Up @@ -135,9 +133,6 @@ async function getBarrelMapping(
optimizeBarrelExports: {
wildcard: isWildcard,
},
serverComponents: {
isReactServerLayer: layer === WEBPACK_LAYERS.reactServerComponents,
},
jsc: {
parser: {
syntax: isTypeScript ? 'typescript' : 'ecmascript',
Expand All @@ -155,7 +150,11 @@ async function getBarrelMapping(

// Avoid circular `export *` dependencies
const visited = new Set<string>()
async function getMatches(file: string, isWildcard: boolean) {
async function getMatches(
file: string,
isWildcard: boolean,
isClientEntry: boolean
) {
if (visited.has(file)) {
return null
}
Expand All @@ -180,7 +179,15 @@ async function getBarrelMapping(
return null
}

const prefix = matches[1]
const matchedDirectives = output.match(
/^([^]*)export (const|var) __next_private_directive_list__ = '([^']+)'/
)
const directiveList = matchedDirectives
? JSON.parse(matchedDirectives[3])
: []
// "use client" in barrel files has to be transferred to the target file.
isClientEntry = directiveList.includes('use client')

let exportList = JSON.parse(matches[3].slice(1, -1)) as [
string,
string,
Expand Down Expand Up @@ -209,7 +216,11 @@ async function getBarrelMapping(
req.replace('__barrel_optimize__?names=__PLACEHOLDER__!=!', '')
)

const targetMatches = await getMatches(targetPath, true)
const targetMatches = await getMatches(
targetPath,
true,
isClientEntry
)
if (targetMatches) {
// Merge the export list
exportList = exportList.concat(targetMatches.exportList)
Expand All @@ -219,13 +230,13 @@ async function getBarrelMapping(
}

return {
prefix,
exportList,
wildcardExports,
isClientEntry,
}
}

const res = await getMatches(resourcePath, false)
const res = await getMatches(resourcePath, false, false)
barrelTransformMappingCache.set(resourcePath, res)

return res
Expand All @@ -249,7 +260,6 @@ const NextBarrelLoader = async function (
})

const mapping = await getBarrelMapping(
this._module?.layer,
this.resourcePath,
swcCacheDir,
resolve,
Expand All @@ -271,15 +281,14 @@ const NextBarrelLoader = async function (
return
}

// It needs to keep the prefix for comments and directives like "use client".
const prefix = mapping.prefix
const exportList = mapping.exportList
const isClientEntry = mapping.isClientEntry
const exportMap = new Map<string, [string, string]>()
for (const [name, filePath, orig] of exportList) {
exportMap.set(name, [filePath, orig])
}

let output = prefix
let output = ''
let missedNames: string[] = []
for (const name of names) {
// If the name matches
Expand Down Expand Up @@ -313,6 +322,12 @@ const NextBarrelLoader = async function (
}
}

// When it has `"use client"` inherited from its barrel files, we need to
// prefix it to this target file as well.
if (isClientEntry) {
output = `"use client";\n${output}`
}

this.callback(null, output)
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import { RSC_MODULE_TYPES } from '../../../shared/lib/constants'
import {
BARREL_OPTIMIZATION_PREFIX,
RSC_MODULE_TYPES,
} from '../../../shared/lib/constants'
import { getModuleBuildInfo } from './get-module-build-info'
import { regexCSS } from './utils'

Expand Down Expand Up @@ -26,7 +29,11 @@ export default function transformSource(this: any) {
.filter((request) => (isServer ? !regexCSS.test(request) : true))
.map(
(request) =>
`import(/* webpackMode: "eager" */ ${JSON.stringify(request)})`
`import(/* webpackMode: "eager" */ ${JSON.stringify(
request.startsWith(BARREL_OPTIMIZATION_PREFIX)
? request.replace(':', '!=!')
: request
)})`
)
.join(';\n')

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,11 @@ export class FlightClientEntryPlugin {
const modQuery = mod.resourceResolveData?.query || ''
// query is already part of mod.resource
// so it's only necessary to add it for matchResource or mod.resourceResolveData
const modResource = modPath ? modPath + modQuery : mod.resource
const modResource = modPath
? modPath.startsWith(BARREL_OPTIMIZATION_PREFIX)
? mod.resource
: modPath + modQuery
: mod.resource

if (mod.layer !== WEBPACK_LAYERS.serverSideRendering) {
return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ createNextDescribe(
'@heroicons/react': '2.0.18',
'@visx/visx': '3.3.0',
'recursive-barrel': '1.0.0',
'@mui/material': '5.14.19',
'@emotion/styled': '11.11.0',
'@emotion/react': '11.11.1',
},
},
({ next }) => {
Expand Down Expand Up @@ -127,6 +130,25 @@ createNextDescribe(
expect(html).toContain('<linearGradient')
})

it('should support MUI', async () => {
let logs = ''
next.on('stdout', (log) => {
logs += log
})

// Ensure that MUI is working
const html = await next.render('/mui')
expect(html).toContain('test_mui')

const modules = [...logs.matchAll(/\((\d+) modules\)/g)]
expect(modules.length).toBeGreaterThanOrEqual(1)
for (const [, moduleCount] of modules) {
// Ensure that the number of modules is less than 1500 - otherwise we're
// importing the entire library.
expect(parseInt(moduleCount)).toBeLessThan(1500)
}
})

it('should not break "use client" directive in optimized packages', async () => {
const html = await next.render('/client')
expect(html).toContain('this is a client component')
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { Button } from '@mui/material'

export default function Page() {
return <Button>test_mui</Button>
}
Loading

0 comments on commit 6790004

Please sign in to comment.