From 64c2f1d0ddf500508b65ad3f494aaed64393d143 Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Tue, 5 Dec 2023 01:14:17 +0900 Subject: [PATCH 1/6] fix #57624 --- .../webpack/loaders/next-barrel-loader.ts | 5 ---- .../barrel-optimization.test.ts | 27 +++++++++++++++++++ .../fixture/app/mui/page.js | 5 ++++ 3 files changed, 32 insertions(+), 5 deletions(-) create mode 100644 test/development/basic/barrel-optimization/fixture/app/mui/page.js diff --git a/packages/next/src/build/webpack/loaders/next-barrel-loader.ts b/packages/next/src/build/webpack/loaders/next-barrel-loader.ts index 18bc3162ce5b5..29eef83f4b210 100644 --- a/packages/next/src/build/webpack/loaders/next-barrel-loader.ts +++ b/packages/next/src/build/webpack/loaders/next-barrel-loader.ts @@ -104,7 +104,6 @@ const barrelTransformMappingCache = new Map< >() async function getBarrelMapping( - layer: string | null | undefined, resourcePath: string, swcCacheDir: string, resolve: (context: string, request: string) => Promise, @@ -135,9 +134,6 @@ async function getBarrelMapping( optimizeBarrelExports: { wildcard: isWildcard, }, - serverComponents: { - isReactServerLayer: layer === WEBPACK_LAYERS.reactServerComponents, - }, jsc: { parser: { syntax: isTypeScript ? 'typescript' : 'ecmascript', @@ -249,7 +245,6 @@ const NextBarrelLoader = async function ( }) const mapping = await getBarrelMapping( - this._module?.layer, this.resourcePath, swcCacheDir, resolve, diff --git a/test/development/basic/barrel-optimization/barrel-optimization.test.ts b/test/development/basic/barrel-optimization/barrel-optimization.test.ts index 3613cded5e8b2..5b29875cc1c2b 100644 --- a/test/development/basic/barrel-optimization/barrel-optimization.test.ts +++ b/test/development/basic/barrel-optimization/barrel-optimization.test.ts @@ -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 }) => { @@ -127,6 +130,30 @@ createNextDescribe( expect(html).toContain(' { + 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( + /Compiled (\/[\w-]+)*\s*in \d+(\.\d+)?(s|ms) \((\d+) modules\)/g + ), + ] + + expect(modules.length).toBeGreaterThanOrEqual(1) + for (const [, , , , moduleCount] of modules) { + // Ensure that the number of modules is less than 1000 - otherwise we're + // importing the entire library. + expect(parseInt(moduleCount)).toBeLessThan(1000) + } + }) + 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') diff --git a/test/development/basic/barrel-optimization/fixture/app/mui/page.js b/test/development/basic/barrel-optimization/fixture/app/mui/page.js new file mode 100644 index 0000000000000..0a1913f8709b4 --- /dev/null +++ b/test/development/basic/barrel-optimization/fixture/app/mui/page.js @@ -0,0 +1,5 @@ +import { Button } from '@mui/material' + +export default function Page() { + return +} From ae61cc0cb0075ec6d4660a45218e6e1e8189b0a4 Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Tue, 5 Dec 2023 01:52:57 +0900 Subject: [PATCH 2/6] fix tests --- .../src/build/webpack/loaders/next-barrel-loader.ts | 1 - .../barrel-optimization/barrel-optimization.test.ts | 11 +++-------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/packages/next/src/build/webpack/loaders/next-barrel-loader.ts b/packages/next/src/build/webpack/loaders/next-barrel-loader.ts index 29eef83f4b210..49defba08c1bf 100644 --- a/packages/next/src/build/webpack/loaders/next-barrel-loader.ts +++ b/packages/next/src/build/webpack/loaders/next-barrel-loader.ts @@ -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) diff --git a/test/development/basic/barrel-optimization/barrel-optimization.test.ts b/test/development/basic/barrel-optimization/barrel-optimization.test.ts index 5b29875cc1c2b..f9a39ad951215 100644 --- a/test/development/basic/barrel-optimization/barrel-optimization.test.ts +++ b/test/development/basic/barrel-optimization/barrel-optimization.test.ts @@ -130,7 +130,7 @@ createNextDescribe( expect(html).toContain(' { + it.only('should support MUI', async () => { let logs = '' next.on('stdout', (log) => { logs += log @@ -140,14 +140,9 @@ createNextDescribe( const html = await next.render('/mui') expect(html).toContain('test_mui') - const modules = [ - ...logs.matchAll( - /Compiled (\/[\w-]+)*\s*in \d+(\.\d+)?(s|ms) \((\d+) modules\)/g - ), - ] - + const modules = [...logs.matchAll(/\((\d+) modules\)/g)] expect(modules.length).toBeGreaterThanOrEqual(1) - for (const [, , , , moduleCount] of modules) { + for (const [, moduleCount] of modules) { // Ensure that the number of modules is less than 1000 - otherwise we're // importing the entire library. expect(parseInt(moduleCount)).toBeLessThan(1000) From 05fffbf821a6899544d2139ba83dd69c787429b1 Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Tue, 5 Dec 2023 01:53:32 +0900 Subject: [PATCH 3/6] remove .only --- .../basic/barrel-optimization/barrel-optimization.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/development/basic/barrel-optimization/barrel-optimization.test.ts b/test/development/basic/barrel-optimization/barrel-optimization.test.ts index f9a39ad951215..c933cf5b4125e 100644 --- a/test/development/basic/barrel-optimization/barrel-optimization.test.ts +++ b/test/development/basic/barrel-optimization/barrel-optimization.test.ts @@ -130,7 +130,7 @@ createNextDescribe( expect(html).toContain(' { + it('should support MUI', async () => { let logs = '' next.on('stdout', (log) => { logs += log From c83cb7cf913384145437aebb95d63ddab0550f87 Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Wed, 6 Dec 2023 00:01:06 +0900 Subject: [PATCH 4/6] fix client entry cases --- .../crates/core/src/optimize_barrel.rs | 44 +++++++++++- .../fixture/optimize-barrel/normal/4/input.js | 9 +++ .../optimize-barrel/normal/4/output.js | 5 +- packages/next/src/build/webpack-config.ts | 67 ++++++++++--------- .../webpack/loaders/next-barrel-loader.ts | 39 ++++++++--- .../next-flight-client-entry-loader.ts | 11 ++- .../plugins/flight-client-entry-plugin.ts | 6 +- 7 files changed, 134 insertions(+), 47 deletions(-) diff --git a/packages/next-swc/crates/core/src/optimize_barrel.rs b/packages/next-swc/crates/core/src/optimize_barrel.rs index 6f0b26a03c734..737c4ac9b56c4 100644 --- a/packages/next-swc/crates/core/src/optimize_barrel.rs +++ b/packages/next-swc/crates/core/src/optimize_barrel.rs @@ -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'; @@ -198,10 +203,19 @@ 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 { + if 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; @@ -209,6 +223,7 @@ impl Fold for OptimizeBarrel { } }, _ => { + allowed_directives = false; if !self.wildcard { is_barrel = false; break; @@ -259,6 +274,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 diff --git a/packages/next-swc/crates/core/tests/fixture/optimize-barrel/normal/4/input.js b/packages/next-swc/crates/core/tests/fixture/optimize-barrel/normal/4/input.js index bd460bc9a27bd..8455d6a041072 100644 --- a/packages/next-swc/crates/core/tests/fixture/optimize-barrel/normal/4/input.js +++ b/packages/next-swc/crates/core/tests/fixture/optimize-barrel/normal/4/input.js @@ -1,3 +1,9 @@ +/* Test */ +/* Test directives inside comments +'use server' +*/ + +// This should be kept 'use client' import foo, { a, b } from 'foo' @@ -7,3 +13,6 @@ 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 +'use strict' diff --git a/packages/next-swc/crates/core/tests/fixture/optimize-barrel/normal/4/output.js b/packages/next-swc/crates/core/tests/fixture/optimize-barrel/normal/4/output.js index abe88ed8a7f81..4f0a733801c73 100644 --- a/packages/next-swc/crates/core/tests/fixture/optimize-barrel/normal/4/output.js +++ b/packages/next-swc/crates/core/tests/fixture/optimize-barrel/normal/4/output.js @@ -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"]'; \ No newline at end of file diff --git a/packages/next/src/build/webpack-config.ts b/packages/next/src/build/webpack-config.ts index cec79d499fcc9..60dde7bbc8ab5 100644 --- a/packages/next/src/build/webpack-config.ts +++ b/packages/next/src/build/webpack-config.ts @@ -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. {} @@ -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: { @@ -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: [ diff --git a/packages/next/src/build/webpack/loaders/next-barrel-loader.ts b/packages/next/src/build/webpack/loaders/next-barrel-loader.ts index 49defba08c1bf..2a3e7408d9079 100644 --- a/packages/next/src/build/webpack/loaders/next-barrel-loader.ts +++ b/packages/next/src/build/webpack/loaders/next-barrel-loader.ts @@ -96,9 +96,9 @@ import { transform } from '../../swc' const barrelTransformMappingCache = new Map< string, { - prefix: string exportList: [string, string, string][] wildcardExports: string[] + isClientEntry: boolean } | null >() @@ -150,7 +150,11 @@ async function getBarrelMapping( // Avoid circular `export *` dependencies const visited = new Set() - async function getMatches(file: string, isWildcard: boolean) { + async function getMatches( + file: string, + isWildcard: boolean, + isClientEntry: boolean + ) { if (visited.has(file)) { return null } @@ -175,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, @@ -204,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) @@ -214,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 @@ -265,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() 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 @@ -307,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) } diff --git a/packages/next/src/build/webpack/loaders/next-flight-client-entry-loader.ts b/packages/next/src/build/webpack/loaders/next-flight-client-entry-loader.ts index 0be3b6fa2b75c..13690a20a01da 100644 --- a/packages/next/src/build/webpack/loaders/next-flight-client-entry-loader.ts +++ b/packages/next/src/build/webpack/loaders/next-flight-client-entry-loader.ts @@ -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' @@ -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') diff --git a/packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts b/packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts index b9014ad967395..7e486baf0dabb 100644 --- a/packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts +++ b/packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts @@ -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 From ad669aaaf6ac919b2a8e0cb6b9559547a1cd8d15 Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Wed, 6 Dec 2023 00:43:35 +0900 Subject: [PATCH 5/6] fix lint errors --- packages/next-swc/crates/core/src/optimize_barrel.rs | 6 ++---- .../core/tests/fixture/optimize-barrel/normal/4/input.js | 1 + packages/next-swc/crates/core/tests/full.rs | 1 + .../basic/barrel-optimization/barrel-optimization.test.ts | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/next-swc/crates/core/src/optimize_barrel.rs b/packages/next-swc/crates/core/src/optimize_barrel.rs index 737c4ac9b56c4..7224c9d7359b0 100644 --- a/packages/next-swc/crates/core/src/optimize_barrel.rs +++ b/packages/next-swc/crates/core/src/optimize_barrel.rs @@ -205,10 +205,8 @@ impl Fold for OptimizeBarrel { Stmt::Expr(expr) => match &*expr.expr { Expr::Lit(l) => { if let Lit::Str(s) = l { - if allowed_directives { - if s.value.starts_with("use ") { - directives.push(s.value.to_string()); - } + if allowed_directives && s.value.starts_with("use ") { + directives.push(s.value.to_string()); } } else { allowed_directives = false; diff --git a/packages/next-swc/crates/core/tests/fixture/optimize-barrel/normal/4/input.js b/packages/next-swc/crates/core/tests/fixture/optimize-barrel/normal/4/input.js index 8455d6a041072..dc6d0c70c8f55 100644 --- a/packages/next-swc/crates/core/tests/fixture/optimize-barrel/normal/4/input.js +++ b/packages/next-swc/crates/core/tests/fixture/optimize-barrel/normal/4/input.js @@ -15,4 +15,5 @@ export { b } export { foo as default, z } // This should be removed as it's not on top +// prettier-ignore 'use strict' diff --git a/packages/next-swc/crates/core/tests/full.rs b/packages/next-swc/crates/core/tests/full.rs index 7c6c90d5af2be..4821a24eeb4c0 100644 --- a/packages/next-swc/crates/core/tests/full.rs +++ b/packages/next-swc/crates/core/tests/full.rs @@ -81,6 +81,7 @@ fn test(input: &Path, minify: bool) { auto_modularize_imports: None, optimize_barrel_exports: None, optimize_server_react: None, + prefer_esm: true, }; let unresolved_mark = Mark::new(); diff --git a/test/development/basic/barrel-optimization/barrel-optimization.test.ts b/test/development/basic/barrel-optimization/barrel-optimization.test.ts index c933cf5b4125e..6fb86c5861381 100644 --- a/test/development/basic/barrel-optimization/barrel-optimization.test.ts +++ b/test/development/basic/barrel-optimization/barrel-optimization.test.ts @@ -143,9 +143,9 @@ createNextDescribe( 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 1000 - otherwise we're + // Ensure that the number of modules is less than 1500 - otherwise we're // importing the entire library. - expect(parseInt(moduleCount)).toBeLessThan(1000) + expect(parseInt(moduleCount)).toBeLessThan(1500) } }) From c53e21e2eda9cbdd1068bce0ac38302b58236c56 Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Wed, 6 Dec 2023 00:45:50 +0900 Subject: [PATCH 6/6] exclude turbopack test --- test/turbopack-tests-manifest.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/turbopack-tests-manifest.json b/test/turbopack-tests-manifest.json index 4638dba61e735..5bc1830d28c67 100644 --- a/test/turbopack-tests-manifest.json +++ b/test/turbopack-tests-manifest.json @@ -1459,7 +1459,8 @@ "optimizePackageImports app - should render the icons correctly without creating all the modules", "optimizePackageImports pages - should optimize recursive wildcard export mapping", "optimizePackageImports pages - should render the icons correctly without creating all the modules", - "optimizePackageImports should support visx" + "optimizePackageImports should support visx", + "optimizePackageImports should support MUI" ], "pending": [], "flakey": [],