From ed2a32a987033de472e4ddf81175c6c64776bf57 Mon Sep 17 00:00:00 2001 From: Benjamin Woodruff Date: Tue, 26 Aug 2025 15:31:15 -0700 Subject: [PATCH] Turbopack: Remove undocumented legacy syntax for built-in conditions (e.g. foreign, browser) --- crates/next-core/src/next_config.rs | 144 ++++++------------ .../src/next_shared/webpack_rules/babel.rs | 8 +- .../src/next_shared/webpack_rules/mod.rs | 2 +- packages/next/src/build/swc/index.ts | 72 ++++----- packages/next/src/server/config-schema.ts | 14 +- packages/next/src/server/config-shared.ts | 9 +- .../webpack-loader-conditions/next.config.js | 42 ++--- 7 files changed, 107 insertions(+), 184 deletions(-) diff --git a/crates/next-core/src/next_config.rs b/crates/next-core/src/next_config.rs index 87b45cbe96f2a..ed9221b9b2bdc 100644 --- a/crates/next-core/src/next_config.rs +++ b/crates/next-core/src/next_config.rs @@ -1,5 +1,3 @@ -use std::{collections::BTreeSet, str::FromStr}; - use anyhow::{Context, Result, bail}; use either::Either; use rustc_hash::FxHashSet; @@ -667,21 +665,11 @@ impl TryFrom for ConditionItem { } } -#[derive( - Clone, Debug, PartialEq, Eq, Serialize, Deserialize, TraceRawVcs, NonLocalValue, OperationValue, -)] -#[serde(rename_all = "camelCase", untagged)] -pub enum RuleConfigItem { - Options(RuleConfigItemOptions), - LegacyConditional(FxIndexMap), - LegacyBoolean(bool), -} - #[derive( Clone, Debug, PartialEq, Eq, Serialize, Deserialize, TraceRawVcs, NonLocalValue, OperationValue, )] #[serde(rename_all = "camelCase")] -pub struct RuleConfigItemOptions { +pub struct RuleConfigItem { pub loaders: Vec, #[serde(default, alias = "as")] pub rename_as: Option, @@ -1401,11 +1389,7 @@ impl NextConfig { } #[turbo_tasks::function] - pub async fn webpack_rules( - &self, - active_conditions: BTreeSet, - project_path: FileSystemPath, - ) -> Result> { + pub async fn webpack_rules(&self, project_path: FileSystemPath) -> Result> { let Some(turbo_rules) = self.turbopack.as_ref().and_then(|t| t.rules.as_ref()) else { return Ok(Vc::cell(Vec::new())); }; @@ -1429,43 +1413,6 @@ impl NextConfig { .collect(), ) } - enum FindRuleResult<'a> { - Found(&'a RuleConfigItemOptions), - NotFound, - Break, - } - // This logic is needed for the `LegacyConditional`/`LegacyBoolean` configuration - // syntax. This is technically public syntax, but was never documented and it is - // unlikely that anyone is depending on it (outside of some Next.js internals). - fn find_rule<'a>( - rule: &'a RuleConfigItem, - active_conditions: &BTreeSet, - ) -> FindRuleResult<'a> { - match rule { - RuleConfigItem::Options(rule) => FindRuleResult::Found(rule), - RuleConfigItem::LegacyConditional(map) => { - for (condition, rule) in map.iter() { - let condition = WebpackLoaderBuiltinCondition::from_str(condition); - if let Ok(condition) = condition - && (condition == WebpackLoaderBuiltinCondition::Default - || active_conditions.contains(&condition)) - { - match find_rule(rule, active_conditions) { - FindRuleResult::Found(rule) => { - return FindRuleResult::Found(rule); - } - FindRuleResult::Break => { - return FindRuleResult::Break; - } - FindRuleResult::NotFound => {} - } - } - } - FindRuleResult::NotFound - } - RuleConfigItem::LegacyBoolean(_) => FindRuleResult::Break, - } - } let config_file_path = || project_path.join(&self.config_file_name); for item in &rule_collection.0 { match item { @@ -1479,55 +1426,52 @@ impl NextConfig { }, )); } - RuleConfigCollectionItem::Full(rule_config_item) => { - if let FindRuleResult::Found(RuleConfigItemOptions { - loaders, - rename_as, - condition, - }) = find_rule(rule_config_item, &active_conditions) + RuleConfigCollectionItem::Full(RuleConfigItem { + loaders, + rename_as, + condition, + }) => { + // If the extension contains a wildcard, and the rename_as does not, + // emit an issue to prevent users from encountering duplicate module + // names. + if glob.contains("*") + && let Some(rename_as) = rename_as.as_ref() + && !rename_as.contains("*") { - // If the extension contains a wildcard, and the rename_as does not, - // emit an issue to prevent users from encountering duplicate module - // names. - if glob.contains("*") - && let Some(rename_as) = rename_as.as_ref() - && !rename_as.contains("*") - { - InvalidLoaderRuleRenameAsIssue { - glob: glob.clone(), + InvalidLoaderRuleRenameAsIssue { + glob: glob.clone(), + config_file_path: config_file_path()?, + rename_as: rename_as.clone(), + } + .resolved_cell() + .emit(); + } + + // convert from Next.js-specific condition type to internal Turbopack + // condition type + let condition = if let Some(condition) = condition { + if let Ok(cond) = ConditionItem::try_from(condition.clone()) { + Some(cond) + } else { + InvalidLoaderRuleConditionIssue { + condition: condition.clone(), config_file_path: config_file_path()?, - rename_as: rename_as.clone(), } .resolved_cell() .emit(); - } - - // convert from Next.js-specific condition type to internal Turbopack - // condition type - let condition = if let Some(condition) = condition { - if let Ok(cond) = ConditionItem::try_from(condition.clone()) { - Some(cond) - } else { - InvalidLoaderRuleConditionIssue { - condition: condition.clone(), - config_file_path: config_file_path()?, - } - .resolved_cell() - .emit(); - None - } - } else { None - }; - rules.push(( - glob.clone(), - LoaderRuleItem { - loaders: transform_loaders(&mut loaders.iter()), - rename_as: rename_as.clone(), - condition, - }, - )); - } + } + } else { + None + }; + rules.push(( + glob.clone(), + LoaderRuleItem { + loaders: transform_loaders(&mut loaders.iter()), + rename_as: rename_as.clone(), + condition, + }, + )); } } } @@ -2001,11 +1945,11 @@ mod tests { } }); - let rule_config: RuleConfigItemOptions = serde_json::from_value(json_value).unwrap(); + let rule_config: RuleConfigItem = serde_json::from_value(json_value).unwrap(); assert_eq!( rule_config, - RuleConfigItemOptions { + RuleConfigItem { loaders: vec![], rename_as: Some(rcstr!("*.js")), condition: Some(ConfigConditionItem::All( diff --git a/crates/next-core/src/next_shared/webpack_rules/babel.rs b/crates/next-core/src/next_shared/webpack_rules/babel.rs index bbf499fe8456d..17f2ae259f8a1 100644 --- a/crates/next-core/src/next_shared/webpack_rules/babel.rs +++ b/crates/next-core/src/next_shared/webpack_rules/babel.rs @@ -5,7 +5,7 @@ use regex::Regex; use turbo_rcstr::{RcStr, rcstr}; use turbo_tasks::{ResolvedVc, Vc}; use turbo_tasks_fs::{self, FileSystemEntryType, FileSystemPath}; -use turbopack::module_options::LoaderRuleItem; +use turbopack::module_options::{ConditionItem, LoaderRuleItem}; use turbopack_core::{ issue::{Issue, IssueExt, IssueSeverity, IssueStage, OptionStyledString, StyledString}, reference_type::{CommonJsReferenceSubType, ReferenceType}, @@ -13,6 +13,8 @@ use turbopack_core::{ }; use turbopack_node::transforms::webpack::WebpackLoaderItem; +use crate::next_shared::webpack_rules::WebpackLoaderBuiltinCondition; + // https://babeljs.io/docs/config-files // TODO: Also support a `babel` key in a package.json file const BABEL_CONFIG_FILES: &[&str] = &[ @@ -89,7 +91,9 @@ pub async fn get_babel_loader_rules( options: Default::default(), }]), rename_as: Some(rcstr!("*")), - condition: None, + condition: Some(ConditionItem::Not(Box::new(ConditionItem::Builtin( + RcStr::from(WebpackLoaderBuiltinCondition::Foreign.as_str()), + )))), }, )]) } diff --git a/crates/next-core/src/next_shared/webpack_rules/mod.rs b/crates/next-core/src/next_shared/webpack_rules/mod.rs index 979078db445c6..594f56f49db83 100644 --- a/crates/next-core/src/next_shared/webpack_rules/mod.rs +++ b/crates/next-core/src/next_shared/webpack_rules/mod.rs @@ -149,7 +149,7 @@ pub async fn webpack_loader_options( builtin_conditions: BTreeSet, ) -> Result> { let mut rules = next_config - .webpack_rules(builtin_conditions.clone(), project_path.clone()) + .webpack_rules(project_path.clone()) .owned() .await?; diff --git a/packages/next/src/build/swc/index.ts b/packages/next/src/build/swc/index.ts index 0caf25f602479..6860dd35bf176 100644 --- a/packages/next/src/build/swc/index.ts +++ b/packages/next/src/build/swc/index.ts @@ -16,7 +16,6 @@ import type { TurbopackRuleCondition, TurbopackRuleConfigCollection, TurbopackRuleConfigItem, - TurbopackRuleConfigItemOptions, } from '../../server/config-shared' import { isDeepStrictEqual } from 'util' import { type DefineEnvOptions, getDefineEnv } from '../define-env' @@ -836,29 +835,32 @@ function bindingToApi( for (const key of ruleKeys) { nextConfig.turbopack.conditions[`#reactCompiler/${key}`] = { - path: key, - content: - options.compilationMode === 'annotation' - ? /['"]use memo['"]/ - : !options.compilationMode || - options.compilationMode === 'infer' - ? // Matches declaration or useXXX or (self closing jsx) - /['"]use memo['"]|\Wuse[A-Z]|<\/|\/>/ - : undefined, + all: [ + 'browser', + { not: 'foreign' }, + { + path: key, + content: + options.compilationMode === 'annotation' + ? /['"]use memo['"]/ + : !options.compilationMode || + options.compilationMode === 'infer' + ? // Matches declaration or useXXX or (self closing jsx) + /['"]use memo['"]|\Wuse[A-Z]|<\/|\/>/ + : undefined, + }, + ], } nextConfig.turbopack.rules[`#reactCompiler/${key}`] = { - browser: { - foreign: false, - loaders: [ - getReactCompilerLoader( - reactCompilerOptions, - projectPath, - nextConfig.dev, - /* isServer */ false, - /* reactCompilerExclude */ undefined - ), - ], - }, + loaders: [ + getReactCompilerLoader( + reactCompilerOptions, + projectPath, + nextConfig.dev, + /* isServer */ false, + /* reactCompilerExclude */ undefined + ), + ], } } } @@ -1039,26 +1041,14 @@ function bindingToApi( glob: string ): any { if (!rule) return rule + for (const item of rule.loaders) { + checkLoaderItem(item, glob) + } let serializedRule: any = rule - if ('loaders' in rule) { - const narrowedRule = rule as TurbopackRuleConfigItemOptions - for (const item of narrowedRule.loaders) { - checkLoaderItem(item, glob) - } - if (narrowedRule.condition != null) { - serializedRule = { - ...rule, - condition: serializeRuleCondition(narrowedRule.condition), - } - } - } else { - serializedRule = {} - for (const [key, value] of Object.entries(rule)) { - if (typeof value === 'object' && value) { - serializedRule[key] = serializeConfigItem(value, glob) - } else { - serializedRule[key] = value - } + if (rule.condition != null) { + serializedRule = { + ...rule, + condition: serializeRuleCondition(rule.condition), } } return serializedRule diff --git a/packages/next/src/server/config-schema.ts b/packages/next/src/server/config-schema.ts index 9352c6e6566e6..45cc6bd61e18f 100644 --- a/packages/next/src/server/config-schema.ts +++ b/packages/next/src/server/config-schema.ts @@ -11,7 +11,6 @@ import type { DeprecatedExperimentalTurboOptions, TurbopackOptions, TurbopackRuleConfigItem, - TurbopackRuleConfigItemOptions, TurbopackRuleConfigCollection, TurbopackRuleCondition, TurbopackLoaderBuiltinCondition, @@ -135,26 +134,17 @@ const zTurbopackCondition: zod.ZodType = z.union([ }), ]) -const zTurbopackRuleConfigItemOptions: zod.ZodType = +const zTurbopackRuleConfigItem: zod.ZodType = z.strictObject({ loaders: z.array(zTurbopackLoaderItem), as: z.string().optional(), condition: zTurbopackCondition.optional(), }) -const zTurbopackRuleConfigItem: zod.ZodType = z.union([ - z.literal(false), - z.record( - zTurbopackLoaderBuiltinCondition, - z.lazy(() => zTurbopackRuleConfigItem) - ), - zTurbopackRuleConfigItemOptions, -]) - const zTurbopackRuleConfigCollection: zod.ZodType = z.union([ zTurbopackRuleConfigItem, - z.array(z.union([zTurbopackLoaderItem, zTurbopackRuleConfigItemOptions])), + z.array(z.union([zTurbopackLoaderItem, zTurbopackRuleConfigItem])), ]) const zTurbopackConfig: zod.ZodType = z.strictObject({ diff --git a/packages/next/src/server/config-shared.ts b/packages/next/src/server/config-shared.ts index c018edb015576..6eead9ff5be31 100644 --- a/packages/next/src/server/config-shared.ts +++ b/packages/next/src/server/config-shared.ts @@ -278,17 +278,12 @@ export type TurbopackRuleCondition = content?: RegExp } -export type TurbopackRuleConfigItemOptions = { +export type TurbopackRuleConfigItem = { loaders: TurbopackLoaderItem[] as?: string condition?: TurbopackRuleCondition } -export type TurbopackRuleConfigItem = - | TurbopackRuleConfigItemOptions - | { [condition in TurbopackLoaderBuiltinCondition]?: TurbopackRuleConfigItem } - | false - /** * This can be an object representing a single configuration, or a list of * loaders and/or rule configuration objects. @@ -300,7 +295,7 @@ export type TurbopackRuleConfigItem = */ export type TurbopackRuleConfigCollection = | TurbopackRuleConfigItem - | (TurbopackLoaderItem | TurbopackRuleConfigItemOptions)[] + | (TurbopackLoaderItem | TurbopackRuleConfigItem)[] export interface TurbopackOptions { /** diff --git a/test/e2e/app-dir/webpack-loader-conditions/next.config.js b/test/e2e/app-dir/webpack-loader-conditions/next.config.js index fc11041f27a7e..ab341e9310147 100644 --- a/test/e2e/app-dir/webpack-loader-conditions/next.config.js +++ b/test/e2e/app-dir/webpack-loader-conditions/next.config.js @@ -4,27 +4,27 @@ const nextConfig = { turbopack: { rules: { - '*.test-file.js': { - browser: { - foreign: { - loaders: [ - { - loader: require.resolve('./test-file-loader.js'), - options: { browser: true, foreign: true }, - }, - ], - }, - default: { - loaders: [ - { - loader: require.resolve('./test-file-loader.js'), - options: { browser: true }, - }, - ], - }, + '*.test-file.js': [ + { + condition: { all: ['browser', 'foreign'] }, + loaders: [ + { + loader: require.resolve('./test-file-loader.js'), + options: { browser: true, foreign: true }, + }, + ], + }, + { + condition: { all: ['browser', { not: 'foreign' }] }, + loaders: [ + { + loader: require.resolve('./test-file-loader.js'), + options: { browser: true }, + }, + ], }, - foreign: false, - default: { + { + condition: { not: { any: ['browser', 'foreign'] } }, loaders: [ { loader: require.resolve('./test-file-loader.js'), @@ -32,7 +32,7 @@ const nextConfig = { }, ], }, - }, + ], }, }, }