From b369ccfcf4851b18d0e9876f01b8d8acd9844868 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Thu, 4 Jul 2024 14:04:57 +0200 Subject: [PATCH] Transpile geist by default (#67402) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### What Transpile `geist` package by default. Currently users need to add `geist` into `transpilePackages` to make it work with pages router as it's external packages but require transform. This PR will resolve that issue. cc @JohnPhamous ### Why geist package is using `next/font` which requires to work with SWC transform. But so far only ESM syntax can be captured by SWC since CJS is pretty difficult to cover as the package import is not statically analyzable. We introduced a new list that it could be transpiled and include geist package into bundle. Add it in both `next/jest` side and webpack/turbopack bundling side --------- Co-authored-by: 강동윤 (Donny) --- .../next-core/src/next_server/context.rs | 28 ++++++----- .../next-swc/crates/next-core/src/util.rs | 47 ++++++++++++------- packages/next/src/build/handle-externals.ts | 11 +++-- packages/next/src/build/jest/jest.ts | 6 ++- packages/next/src/build/webpack-config.ts | 8 +++- .../src/lib/default-transpiled-packages.json | 1 + test/e2e/geist-font/geist-font.test.ts | 19 ++++++++ test/e2e/geist-font/pages/_app.js | 9 ++++ test/e2e/geist-font/pages/foo.js | 3 ++ 9 files changed, 98 insertions(+), 34 deletions(-) create mode 100644 packages/next/src/lib/default-transpiled-packages.json create mode 100644 test/e2e/geist-font/geist-font.test.ts create mode 100644 test/e2e/geist-font/pages/_app.js create mode 100644 test/e2e/geist-font/pages/foo.js diff --git a/packages/next-swc/crates/next-core/src/next_server/context.rs b/packages/next-swc/crates/next-core/src/next_server/context.rs index 760c661ab6..fb9404d7b5 100644 --- a/packages/next-swc/crates/next-core/src/next_server/context.rs +++ b/packages/next-swc/crates/next-core/src/next_server/context.rs @@ -72,7 +72,10 @@ use crate::{ get_decorators_transform_options, get_jsx_transform_options, get_typescript_transform_options, }, - util::{foreign_code_context_condition, load_next_js_templateon, NextRuntime}, + util::{ + foreign_code_context_condition, get_transpiled_packages, load_next_js_templateon, + NextRuntime, + }, }; #[turbo_tasks::value(serialization = "auto_for_input")] @@ -140,13 +143,6 @@ pub async fn get_server_resolve_options_context( let invalid_styled_jsx_client_only_resolve_plugin = get_invalid_styled_jsx_resolve_plugin(project_path); - let mut transpile_packages = next_config.transpile_packages().await?.clone_value(); - transpile_packages.extend( - (*next_config.optimize_package_imports().await?) - .iter() - .cloned(), - ); - // Always load these predefined packages as external. let mut external_packages: Vec = load_next_js_templateon( project_path, @@ -154,9 +150,19 @@ pub async fn get_server_resolve_options_context( ) .await?; + let mut transpiled_packages = get_transpiled_packages(next_config, project_path) + .await? + .clone_value(); + + transpiled_packages.extend( + (*next_config.optimize_package_imports().await?) + .iter() + .cloned(), + ); + let server_external_packages = &*next_config.server_external_packages().await?; - let conflicting_packages = transpile_packages + let conflicting_packages = transpiled_packages .iter() .filter(|package| server_external_packages.contains(package)) .collect::>(); @@ -172,7 +178,7 @@ pub async fn get_server_resolve_options_context( // Add the config's own list of external packages. external_packages.extend(server_external_packages.iter().cloned()); - external_packages.retain(|item| !transpile_packages.contains(item)); + external_packages.retain(|item| !transpiled_packages.contains(item)); let ty = ty.into_value(); @@ -203,7 +209,7 @@ pub async fn get_server_resolve_options_context( ExternalCjsModulesResolvePlugin::new( project_path, project_path.root(), - ExternalPredicate::AllExcept(Vc::cell(transpile_packages)).cell(), + ExternalPredicate::AllExcept(Vc::cell(transpiled_packages)).cell(), *next_config.import_externals().await?, ) }; diff --git a/packages/next-swc/crates/next-core/src/util.rs b/packages/next-swc/crates/next-core/src/util.rs index 58bf8b5837..474ff6217d 100644 --- a/packages/next-swc/crates/next-core/src/util.rs +++ b/packages/next-swc/crates/next-core/src/util.rs @@ -89,11 +89,29 @@ pub fn get_asset_path_from_pathname(pathname: &str, ext: &str) -> String { format!("{}{}", get_asset_prefix_from_pathname(pathname), ext) } +#[turbo_tasks::function] +pub async fn get_transpiled_packages( + next_config: Vc, + project_path: Vc, +) -> Result>> { + let mut transpile_packages: Vec = next_config.transpile_packages().await?.clone_value(); + + let default_transpiled_packages: Vec = load_next_js_templateon( + project_path, + "dist/lib/default-transpiled-packages.json".into(), + ) + .await?; + + transpile_packages.extend(default_transpiled_packages.iter().cloned()); + + Ok(Vc::cell(transpile_packages)) +} + pub async fn foreign_code_context_condition( next_config: Vc, project_path: Vc, ) -> Result { - let transpile_packages = next_config.transpile_packages().await?; + let transpiled_packages = get_transpiled_packages(next_config, project_path).await?; // The next template files are allowed to import the user's code via import // mapping, and imports must use the project-level [ResolveOptions] instead @@ -103,23 +121,16 @@ pub async fn foreign_code_context_condition( get_next_package(project_path).join(NEXT_TEMPLATE_PATH.into()), )); - let result = if transpile_packages.is_empty() { - ContextCondition::all(vec![ - ContextCondition::InDirectory("node_modules".to_string()), - not_next_template_dir, - ]) - } else { - ContextCondition::all(vec![ - ContextCondition::InDirectory("node_modules".to_string()), - not_next_template_dir, - ContextCondition::not(ContextCondition::any( - transpile_packages - .iter() - .map(|package| ContextCondition::InDirectory(format!("node_modules/{package}"))) - .collect(), - )), - ]) - }; + let result = ContextCondition::all(vec![ + ContextCondition::InDirectory("node_modules".to_string()), + not_next_template_dir, + ContextCondition::not(ContextCondition::any( + transpiled_packages + .iter() + .map(|package| ContextCondition::InDirectory(format!("node_modules/{package}"))) + .collect(), + )), + ]); Ok(result) } diff --git a/packages/next/src/build/handle-externals.ts b/packages/next/src/build/handle-externals.ts index 5b38dd26e0..7d0db3eb37 100644 --- a/packages/next/src/build/handle-externals.ts +++ b/packages/next/src/build/handle-externals.ts @@ -135,11 +135,13 @@ export function makeExternalHandler({ config, optOutBundlingPackages, optOutBundlingPackageRegex, + transpiledPackages, dir, }: { config: NextConfigComplete optOutBundlingPackages: string[] optOutBundlingPackageRegex: RegExp + transpiledPackages: string[] dir: string }) { let resolvedExternalPackageDirs: Map @@ -322,10 +324,10 @@ export function makeExternalHandler({ // If a package should be transpiled by Next.js, we skip making it external. // It doesn't matter what the extension is, as we'll transpile it anyway. - if (config.transpilePackages && !resolvedExternalPackageDirs) { + if (transpiledPackages && !resolvedExternalPackageDirs) { resolvedExternalPackageDirs = new Map() // We need to resolve all the external package dirs initially. - for (const pkg of config.transpilePackages) { + for (const pkg of transpiledPackages) { const pkgRes = await resolveExternal( dir, config.experimental.esmExternals, @@ -350,6 +352,7 @@ export function makeExternalHandler({ externalType, isOptOutBundling, request, + transpiledPackages, }) if (resolvedBundlingOptOutRes) { return resolvedBundlingOptOutRes @@ -368,6 +371,7 @@ function resolveBundlingOptOutPackages({ externalType, isOptOutBundling, request, + transpiledPackages, }: { resolvedRes: string config: NextConfigComplete @@ -376,6 +380,7 @@ function resolveBundlingOptOutPackages({ externalType: string isOptOutBundling: boolean request: string + transpiledPackages: string[] }) { if (nodeModulesRegex.test(resolvedRes)) { const shouldBundlePages = @@ -385,7 +390,7 @@ function resolveBundlingOptOutPackages({ shouldBundlePages || isResourceInPackages( resolvedRes, - config.transpilePackages, + transpiledPackages, resolvedExternalPackageDirs ) diff --git a/packages/next/src/build/jest/jest.ts b/packages/next/src/build/jest/jest.ts index 3a718374d3..46d4738d83 100644 --- a/packages/next/src/build/jest/jest.ts +++ b/packages/next/src/build/jest/jest.ts @@ -9,6 +9,8 @@ import { loadBindings, lockfilePatchPromise } from '../swc' import type { JestTransformerConfig } from '../swc/jest-transformer' import type { Config } from '@jest/types' +const DEFAULT_TRANSPILED_PACKAGES: string[] = require('../../lib/default-transpiled-packages.json') + async function getConfig(dir: string) { const conf = await loadConfig(PHASE_TEST, dir) return conf @@ -100,7 +102,9 @@ export default function nextJest(options: { dir?: string } = {}) { await lockfilePatchPromise.cur } - const transpiled = (nextConfig?.transpilePackages ?? []).join('|') + const transpiled = (nextConfig?.transpilePackages ?? []) + .concat(DEFAULT_TRANSPILED_PACKAGES) + .join('|') const jestTransformerConfig: JestTransformerConfig = { modularizeImports: nextConfig?.modularizeImports, diff --git a/packages/next/src/build/webpack-config.ts b/packages/next/src/build/webpack-config.ts index 3bf30b5087..3ac013daa7 100644 --- a/packages/next/src/build/webpack-config.ts +++ b/packages/next/src/build/webpack-config.ts @@ -98,6 +98,9 @@ type ClientEntries = { const EXTERNAL_PACKAGES = require('../lib/server-external-packages.json') as string[] +const DEFAULT_TRANSPILED_PACKAGES = + require('../lib/default-transpiled-packages.json') as string[] + export const NEXT_PROJECT_ROOT = path.join(__dirname, '..', '..') export const NEXT_PROJECT_ROOT_DIST = path.join(NEXT_PROJECT_ROOT, 'dist') const NEXT_PROJECT_ROOT_DIST_CLIENT = path.join( @@ -395,7 +398,9 @@ export default async function getBaseWebpackConfig( // since `pages` doesn't always bundle by default we need to // auto-include optimizePackageImports in transpilePackages - const finalTranspilePackages: string[] = config.transpilePackages || [] + const finalTranspilePackages: string[] = ( + config.transpilePackages || [] + ).concat(DEFAULT_TRANSPILED_PACKAGES) for (const pkg of config.experimental.optimizePackageImports || []) { if (!finalTranspilePackages.includes(pkg)) { @@ -855,6 +860,7 @@ export default async function getBaseWebpackConfig( config, optOutBundlingPackages, optOutBundlingPackageRegex, + transpiledPackages: finalTranspilePackages, dir, }) diff --git a/packages/next/src/lib/default-transpiled-packages.json b/packages/next/src/lib/default-transpiled-packages.json new file mode 100644 index 0000000000..aca6e6df9a --- /dev/null +++ b/packages/next/src/lib/default-transpiled-packages.json @@ -0,0 +1 @@ +["geist"] diff --git a/test/e2e/geist-font/geist-font.test.ts b/test/e2e/geist-font/geist-font.test.ts new file mode 100644 index 0000000000..9cff37ad50 --- /dev/null +++ b/test/e2e/geist-font/geist-font.test.ts @@ -0,0 +1,19 @@ +import { nextTestSetup } from 'e2e-utils' +import { assertNoRedbox } from 'next-test-utils' + +describe('geist-font', () => { + const { next } = nextTestSetup({ + files: __dirname, + dependencies: { + geist: 'latest', + }, + }) + + it('should work with geist font in pages router', async () => { + const browser = await next.browser('/foo') + + await assertNoRedbox(browser) + const text = await browser.elementByCss('p').text() + expect(text).toBe('Foo page') + }) +}) diff --git a/test/e2e/geist-font/pages/_app.js b/test/e2e/geist-font/pages/_app.js new file mode 100644 index 0000000000..bd97dc9f73 --- /dev/null +++ b/test/e2e/geist-font/pages/_app.js @@ -0,0 +1,9 @@ +import { GeistSans } from 'geist/font/sans' + +export default function MyApp({ Component, pageProps }) { + return ( +
+ +
+ ) +} diff --git a/test/e2e/geist-font/pages/foo.js b/test/e2e/geist-font/pages/foo.js new file mode 100644 index 0000000000..081a3b4b62 --- /dev/null +++ b/test/e2e/geist-font/pages/foo.js @@ -0,0 +1,3 @@ +export default function Page() { + return

Foo page

+}