From eb9047281343d62d6bb60382ec641d4f84cd5866 Mon Sep 17 00:00:00 2001 From: Luke Sandberg Date: Wed, 26 Nov 2025 11:59:39 -0800 Subject: [PATCH 1/8] Rename side_effects and implement it everywhere --- crates/next-core/src/hmr_entry.rs | 7 ++-- .../css_client_reference_module.rs | 9 ++++-- .../ecmascript_client_reference_module.rs | 9 ++++-- .../src/next_dynamic/dynamic_module.rs | 9 ++++-- .../server_component_module.rs | 9 ++++-- .../server_utility_module.rs | 10 ++++-- crates/next-core/src/raw_ecmascript_module.rs | 10 +++--- turbopack/crates/turbopack-core/src/module.rs | 24 +++----------- .../turbopack-core/src/module_graph/mod.rs | 11 +++++-- .../turbopack-core/src/node_addon_module.rs | 9 ++++-- .../crates/turbopack-core/src/raw_module.rs | 9 ++++-- turbopack/crates/turbopack-css/src/asset.rs | 9 ++++-- .../crates/turbopack-css/src/module_asset.rs | 11 +++++-- .../src/async_chunk/module.rs | 9 ++---- .../src/chunk/placeable.rs | 32 +++++++++++++++---- .../src/inlined_bytes_module.rs | 9 ++---- .../crates/turbopack-ecmascript/src/lib.rs | 24 ++++++++------ .../src/manifest/chunk_asset.rs | 9 ++---- .../turbopack-ecmascript/src/merged_module.rs | 12 +++++-- .../src/references/esm/base.rs | 8 ++--- .../src/references/esm/export.rs | 7 ++-- .../src/references/external_module.rs | 13 ++++---- .../src/references/require_context.rs | 9 ++---- .../side_effect_optimization/facade/module.rs | 13 +++----- .../side_effect_optimization/locals/module.rs | 7 ++-- .../src/tree_shake/asset.rs | 20 ++++++------ .../src/tree_shake/side_effect_module.rs | 6 ++-- .../src/typescript/mod.rs | 9 ++++-- .../turbopack-ecmascript/src/webpack/mod.rs | 8 ++++- .../src/worker_chunk/module.rs | 9 ++---- turbopack/crates/turbopack-json/src/lib.rs | 9 ++---- turbopack/crates/turbopack-static/src/css.rs | 9 ++---- turbopack/crates/turbopack-static/src/ecma.rs | 9 ++---- .../crates/turbopack-wasm/src/module_asset.rs | 12 +++++-- turbopack/crates/turbopack-wasm/src/raw.rs | 9 +++++- turbopack/crates/turbopack/src/lib.rs | 12 +++---- 36 files changed, 229 insertions(+), 171 deletions(-) diff --git a/crates/next-core/src/hmr_entry.rs b/crates/next-core/src/hmr_entry.rs index 8653c564bde51..2549e50c864c4 100644 --- a/crates/next-core/src/hmr_entry.rs +++ b/crates/next-core/src/hmr_entry.rs @@ -11,7 +11,7 @@ use turbopack_core::{ EvaluatableAsset, }, ident::AssetIdent, - module::Module, + module::{Module, ModuleSideEffects}, module_graph::ModuleGraph, output::OutputAssetsReference, reference::{ModuleReference, ModuleReferences}, @@ -76,10 +76,9 @@ impl Module for HmrEntryModule { .await?, )])) } - #[turbo_tasks::function] - fn is_marked_as_side_effect_free(self: Vc, _: Vc) -> Vc { - Vc::cell(false) + fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + ModuleSideEffects::SideEffectful.cell() } } diff --git a/crates/next-core/src/next_client_reference/css_client_reference/css_client_reference_module.rs b/crates/next-core/src/next_client_reference/css_client_reference/css_client_reference_module.rs index 194f6e08a8da3..9c6ecc57911b2 100644 --- a/crates/next-core/src/next_client_reference/css_client_reference/css_client_reference_module.rs +++ b/crates/next-core/src/next_client_reference/css_client_reference/css_client_reference_module.rs @@ -1,12 +1,13 @@ use anyhow::Result; use turbo_rcstr::{RcStr, rcstr}; use turbo_tasks::{ResolvedVc, ValueToString, Vc}; -use turbo_tasks_fs::FileContent; +use turbo_tasks_fs::{FileContent, glob::Glob}; +use turbopack::css::chunk::CssChunkPlaceable; use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ChunkGroupType, ChunkableModuleReference, ChunkingType, ChunkingTypeOption}, ident::AssetIdent, - module::Module, + module::{Module, ModuleSideEffects}, reference::{ModuleReference, ModuleReferences}, resolve::ModuleResolveResult, source::OptionSource, @@ -57,6 +58,10 @@ impl Module for CssClientReferenceModule { .await?, )])) } + #[turbo_tasks::function] + fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + ModuleSideEffects::SideEffectful.cell() + } } #[turbo_tasks::value_impl] diff --git a/crates/next-core/src/next_client_reference/ecmascript_client_reference/ecmascript_client_reference_module.rs b/crates/next-core/src/next_client_reference/ecmascript_client_reference/ecmascript_client_reference_module.rs index 35526af5537d8..8657136a009f0 100644 --- a/crates/next-core/src/next_client_reference/ecmascript_client_reference/ecmascript_client_reference_module.rs +++ b/crates/next-core/src/next_client_reference/ecmascript_client_reference/ecmascript_client_reference_module.rs @@ -4,7 +4,7 @@ use anyhow::{Context, Result, bail}; use indoc::writedoc; use turbo_rcstr::{RcStr, rcstr}; use turbo_tasks::{IntoTraitRef, ResolvedVc, ValueToString, Vc}; -use turbo_tasks_fs::{File, FileContent}; +use turbo_tasks_fs::{File, FileContent, glob::Glob}; use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ @@ -14,7 +14,7 @@ use turbopack_core::{ code_builder::CodeBuilder, context::AssetContext, ident::AssetIdent, - module::Module, + module::{Module, ModuleSideEffects}, module_graph::{ModuleGraph, binding_usage_info::ModuleExportUsageInfo}, output::OutputAssetsReference, reference::{ModuleReference, ModuleReferences}, @@ -241,6 +241,11 @@ impl Module for EcmascriptClientReferenceModule { Ok(Vc::cell(references)) } + #[turbo_tasks::function] + fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + // These just export some specially tagged functions + ModuleSideEffects::SideEffectFree.cell() + } } #[turbo_tasks::value_impl] diff --git a/crates/next-core/src/next_dynamic/dynamic_module.rs b/crates/next-core/src/next_dynamic/dynamic_module.rs index a93cc9ff59ffa..e7ff33faf32b5 100644 --- a/crates/next-core/src/next_dynamic/dynamic_module.rs +++ b/crates/next-core/src/next_dynamic/dynamic_module.rs @@ -4,12 +4,12 @@ use anyhow::Result; use indoc::formatdoc; use turbo_rcstr::{RcStr, rcstr}; use turbo_tasks::{ResolvedVc, Vc}; -use turbo_tasks_fs::FileContent; +use turbo_tasks_fs::{FileContent, glob::Glob}; use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ChunkItem, ChunkType, ChunkableModule, ChunkingContext, ModuleChunkItemIdExt}, ident::AssetIdent, - module::Module, + module::{Module, ModuleSideEffects}, module_graph::ModuleGraph, output::OutputAssetsReference, reference::{ModuleReferences, SingleChunkableModuleReference}, @@ -71,6 +71,11 @@ impl Module for NextDynamicEntryModule { .await?, )])) } + #[turbo_tasks::function] + fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + // This just exports another import + ModuleSideEffects::ModuleEvaluationIsSideEffectFree.cell() + } } #[turbo_tasks::value_impl] diff --git a/crates/next-core/src/next_server_component/server_component_module.rs b/crates/next-core/src/next_server_component/server_component_module.rs index 996bdc7f91a53..130bb540ecb1e 100644 --- a/crates/next-core/src/next_server_component/server_component_module.rs +++ b/crates/next-core/src/next_server_component/server_component_module.rs @@ -4,12 +4,12 @@ use anyhow::Result; use indoc::formatdoc; use turbo_rcstr::rcstr; use turbo_tasks::{ResolvedVc, Vc}; -use turbo_tasks_fs::{FileContent, FileSystemPath}; +use turbo_tasks_fs::{FileContent, FileSystemPath, glob::Glob}; use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ChunkItem, ChunkType, ChunkableModule, ChunkingContext, ModuleChunkItemIdExt}, ident::AssetIdent, - module::Module, + module::{Module, ModuleSideEffects}, module_graph::ModuleGraph, output::OutputAssetsReference, reference::ModuleReferences, @@ -67,6 +67,11 @@ impl Module for NextServerComponentModule { .await?, )])) } + #[turbo_tasks::function] + fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + // This just exports another import + ModuleSideEffects::ModuleEvaluationIsSideEffectFree.cell() + } } #[turbo_tasks::value_impl] diff --git a/crates/next-core/src/next_server_utility/server_utility_module.rs b/crates/next-core/src/next_server_utility/server_utility_module.rs index e50548bd50d13..c5712f8771ffa 100644 --- a/crates/next-core/src/next_server_utility/server_utility_module.rs +++ b/crates/next-core/src/next_server_utility/server_utility_module.rs @@ -4,12 +4,12 @@ use anyhow::Result; use indoc::formatdoc; use turbo_rcstr::rcstr; use turbo_tasks::{ResolvedVc, Vc}; -use turbo_tasks_fs::{FileContent, FileSystemPath}; +use turbo_tasks_fs::{FileContent, FileSystemPath, glob::Glob}; use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ChunkItem, ChunkType, ChunkableModule, ChunkingContext, ModuleChunkItemIdExt}, ident::AssetIdent, - module::Module, + module::{Module, ModuleSideEffects}, module_graph::ModuleGraph, output::OutputAssetsReference, reference::ModuleReferences, @@ -67,6 +67,12 @@ impl Module for NextServerUtilityModule { .await?, )])) } + + #[turbo_tasks::function] + fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + // This just exports another import + ModuleSideEffects::ModuleEvaluationIsSideEffectFree.cell() + } } #[turbo_tasks::value_impl] diff --git a/crates/next-core/src/raw_ecmascript_module.rs b/crates/next-core/src/raw_ecmascript_module.rs index 9cee3d657966c..1d7888f03b29c 100644 --- a/crates/next-core/src/raw_ecmascript_module.rs +++ b/crates/next-core/src/raw_ecmascript_module.rs @@ -18,7 +18,7 @@ use turbopack_core::{ }, context::AssetContext, ident::AssetIdent, - module::Module, + module::{Module, ModuleSideEffects}, module_graph::ModuleGraph, output::OutputAssetsReference, resolve::ModulePart, @@ -99,11 +99,9 @@ impl Module for RawEcmascriptModule { } #[turbo_tasks::function] - fn is_marked_as_side_effect_free( - self: Vc, - _side_effect_free_packages: Vc, - ) -> Vc { - Vc::cell(false) + fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + // Is this correct? + ModuleSideEffects::SideEffectFree.cell() } } diff --git a/turbopack/crates/turbopack-core/src/module.rs b/turbopack/crates/turbopack-core/src/module.rs index 21398362a7b40..3b84653f0f13e 100644 --- a/turbopack/crates/turbopack-core/src/module.rs +++ b/turbopack/crates/turbopack-core/src/module.rs @@ -1,6 +1,5 @@ -use serde::{Deserialize, Serialize}; use turbo_rcstr::RcStr; -use turbo_tasks::{NonLocalValue, ResolvedVc, TaskInput, ValueToString, Vc, trace::TraceRawVcs}; +use turbo_tasks::{ResolvedVc, TaskInput, ValueToString, Vc}; use turbo_tasks_fs::glob::Glob; use crate::{asset::Asset, ident::AssetIdent, reference::ModuleReferences, source::OptionSource}; @@ -12,18 +11,8 @@ pub enum StyleType { GlobalStyle, } -#[derive( - Serialize, - Deserialize, - Hash, - Eq, - PartialEq, - Debug, - NonLocalValue, - TraceRawVcs, - bincode::Encode, - bincode::Decode, -)] +#[derive(Hash, Debug, Copy, Clone)] +#[turbo_tasks::value(shared)] pub enum ModuleSideEffects { /// Analysis determined that the module evaluation is side effect free /// the module may still be side effectful based on its imports. @@ -72,12 +61,7 @@ pub trait Module: Asset { /// Returns true if the module is marked as side effect free in package.json or by other means. #[turbo_tasks::function] - fn is_marked_as_side_effect_free( - self: Vc, - _side_effect_free_packages: Vc, - ) -> Vc { - Vc::cell(false) - } + fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc; } #[turbo_tasks::value_trait] diff --git a/turbopack/crates/turbopack-core/src/module_graph/mod.rs b/turbopack/crates/turbopack-core/src/module_graph/mod.rs index deb7711a1044b..b21f9b8448fb4 100644 --- a/turbopack/crates/turbopack-core/src/module_graph/mod.rs +++ b/turbopack/crates/turbopack-core/src/module_graph/mod.rs @@ -1954,12 +1954,12 @@ pub mod tests { use turbo_rcstr::{RcStr, rcstr}; use turbo_tasks::{ResolvedVc, TryJoinIterExt, ValueToString, Vc}; use turbo_tasks_backend::{BackendOptions, TurboTasksBackend, noop_backing_storage}; - use turbo_tasks_fs::{FileSystem, FileSystemPath, VirtualFileSystem}; + use turbo_tasks_fs::{FileSystem, FileSystemPath, VirtualFileSystem, glob::Glob}; use crate::{ asset::{Asset, AssetContent}, ident::AssetIdent, - module::Module, + module::{Module, ModuleSideEffects}, module_graph::{ GraphEntries, GraphTraversalAction, ModuleGraph, ModuleGraphRef, SingleModuleGraph, VisitedModules, chunk_group_info::ChunkGroupEntry, @@ -2357,6 +2357,13 @@ pub mod tests { Ok(Vc::cell(references)) } + #[turbo_tasks::function] + fn side_effects( + self: Vc, + _side_effect_free_packages: Vc, + ) -> Vc { + ModuleSideEffects::SideEffectful.cell() + } } /// Constructs a graph based on the provided dependency adjacency lists and calls the given test diff --git a/turbopack/crates/turbopack-core/src/node_addon_module.rs b/turbopack/crates/turbopack-core/src/node_addon_module.rs index ba1096f4c5968..7800a068fad98 100644 --- a/turbopack/crates/turbopack-core/src/node_addon_module.rs +++ b/turbopack/crates/turbopack-core/src/node_addon_module.rs @@ -4,13 +4,13 @@ use anyhow::{Result, bail}; use regex::Regex; use turbo_rcstr::rcstr; use turbo_tasks::{FxIndexSet, ResolvedVc, TryJoinIterExt, Vc}; -use turbo_tasks_fs::{FileSystemEntryType, FileSystemPath}; +use turbo_tasks_fs::{FileSystemEntryType, FileSystemPath, glob::Glob}; use crate::{ asset::{Asset, AssetContent}, file_source::FileSource, ident::AssetIdent, - module::Module, + module::{Module, ModuleSideEffects}, raw_module::RawModule, reference::{ModuleReferences, TracedModuleReference}, resolve::pattern::{Pattern, PatternMatch, read_matches}, @@ -100,6 +100,11 @@ impl Module for NodeAddonModule { // Most addon modules don't have references to other modules. Ok(ModuleReferences::empty()) } + + #[turbo_tasks::function] + fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + ModuleSideEffects::SideEffectful.cell() + } } #[turbo_tasks::value_impl] diff --git a/turbopack/crates/turbopack-core/src/raw_module.rs b/turbopack/crates/turbopack-core/src/raw_module.rs index dc6fd947b4e96..6f61c64c25734 100644 --- a/turbopack/crates/turbopack-core/src/raw_module.rs +++ b/turbopack/crates/turbopack-core/src/raw_module.rs @@ -1,13 +1,14 @@ use turbo_tasks::{ResolvedVc, Vc}; +use turbo_tasks_fs::glob::Glob; use crate::{ asset::{Asset, AssetContent}, ident::AssetIdent, - module::Module, + module::{Module, ModuleSideEffects}, source::{OptionSource, Source}, }; -/// A module where source code doesn't need to be parsed but can be usd as is. +/// A module where source code doesn't need to be parsed but can be used as is. /// This module has no references to other modules. #[turbo_tasks::value] pub struct RawModule { @@ -25,6 +26,10 @@ impl Module for RawModule { fn source(&self) -> Vc { Vc::cell(Some(self.source)) } + #[turbo_tasks::function] + fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + ModuleSideEffects::SideEffectful.cell() + } } #[turbo_tasks::value_impl] diff --git a/turbopack/crates/turbopack-css/src/asset.rs b/turbopack/crates/turbopack-css/src/asset.rs index caad1b5c2e361..b097d57c077d2 100644 --- a/turbopack/crates/turbopack-css/src/asset.rs +++ b/turbopack/crates/turbopack-css/src/asset.rs @@ -1,14 +1,14 @@ use anyhow::Result; use turbo_rcstr::rcstr; use turbo_tasks::{IntoTraitRef, ResolvedVc, TryJoinIterExt, ValueToString, Vc}; -use turbo_tasks_fs::{FileContent, FileSystemPath}; +use turbo_tasks_fs::{FileContent, FileSystemPath, glob::Glob}; use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ChunkItem, ChunkType, ChunkableModule, ChunkingContext, MinifyType}, context::AssetContext, environment::Environment, ident::AssetIdent, - module::{Module, StyleModule, StyleType}, + module::{Module, ModuleSideEffects, StyleModule, StyleType}, module_graph::ModuleGraph, output::{OutputAssetsReference, OutputAssetsWithReferenced}, reference::{ModuleReference, ModuleReferences}, @@ -153,6 +153,11 @@ impl Module for CssModuleAsset { ParseCssResult::NotFound => Ok(ModuleReferences::empty()), } } + #[turbo_tasks::function] + fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + // global css is always a side effect + ModuleSideEffects::SideEffectful.cell() + } } #[turbo_tasks::value_impl] diff --git a/turbopack/crates/turbopack-css/src/module_asset.rs b/turbopack/crates/turbopack-css/src/module_asset.rs index f0804dfe18609..9072024882cca 100644 --- a/turbopack/crates/turbopack-css/src/module_asset.rs +++ b/turbopack/crates/turbopack-css/src/module_asset.rs @@ -6,7 +6,7 @@ use lightningcss::css_modules::CssModuleReference; use swc_core::common::{BytePos, FileName, LineCol, SourceMap}; use turbo_rcstr::{RcStr, rcstr}; use turbo_tasks::{FxIndexMap, IntoTraitRef, ResolvedVc, ValueToString, Vc}; -use turbo_tasks_fs::{FileSystemPath, rope::Rope}; +use turbo_tasks_fs::{FileSystemPath, glob::Glob, rope::Rope}; use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ChunkItem, ChunkType, ChunkableModule, ChunkingContext, ModuleChunkItemIdExt}, @@ -16,7 +16,7 @@ use turbopack_core::{ Issue, IssueExt, IssueSeverity, IssueSource, IssueStage, OptionIssueSource, OptionStyledString, StyledString, }, - module::Module, + module::{Module, ModuleSideEffects}, module_graph::ModuleGraph, output::OutputAssetsReference, reference::{ModuleReference, ModuleReferences}, @@ -111,6 +111,13 @@ impl Module for ModuleCssAsset { Ok(Vc::cell(references)) } + + #[turbo_tasks::function] + fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + // modules can still effect global styles using `:root` selectors and other similar features + // We could do better with some static analysis if we want + ModuleSideEffects::SideEffectful.cell() + } } #[turbo_tasks::value_impl] diff --git a/turbopack/crates/turbopack-ecmascript/src/async_chunk/module.rs b/turbopack/crates/turbopack-ecmascript/src/async_chunk/module.rs index d32a0551a236f..72f462096b51a 100644 --- a/turbopack/crates/turbopack-ecmascript/src/async_chunk/module.rs +++ b/turbopack/crates/turbopack-ecmascript/src/async_chunk/module.rs @@ -6,7 +6,7 @@ use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ChunkableModule, ChunkingContext, availability_info::AvailabilityInfo}, ident::AssetIdent, - module::Module, + module::{Module, ModuleSideEffects}, module_graph::ModuleGraph, reference::{ModuleReferences, SingleModuleReference}, }; @@ -68,11 +68,8 @@ impl Module for AsyncLoaderModule { } #[turbo_tasks::function] - fn is_marked_as_side_effect_free( - self: Vc, - _side_effect_free_packages: Vc, - ) -> Vc { - Vc::cell(true) + fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + ModuleSideEffects::SideEffectFree.cell() } } diff --git a/turbopack/crates/turbopack-ecmascript/src/chunk/placeable.rs b/turbopack/crates/turbopack-ecmascript/src/chunk/placeable.rs index d9f810b120af5..8d2a28fe9a4d9 100644 --- a/turbopack/crates/turbopack-ecmascript/src/chunk/placeable.rs +++ b/turbopack/crates/turbopack-ecmascript/src/chunk/placeable.rs @@ -179,13 +179,21 @@ impl Issue for SideEffectsInPackageJsonIssue { } } +#[turbo_tasks::value(shared)] +#[derive(Copy, Clone)] +pub enum SideEffectsDeclaration { + SideEffectFree, + SideEffectful, + None, +} + #[turbo_tasks::function] -pub async fn is_marked_as_side_effect_free( +pub async fn get_side_effect_free_declaration( path: FileSystemPath, side_effect_free_packages: Vc, -) -> Result> { +) -> Result> { if side_effect_free_packages.await?.matches(&path.path) { - return Ok(Vc::cell(true)); + return Ok(SideEffectsDeclaration::SideEffectFree.cell()); } let find_package_json = find_context_file(path.parent(), package_json(), false).await?; @@ -193,17 +201,29 @@ pub async fn is_marked_as_side_effect_free( if let FindContextFileResult::Found(package_json, _) = &*find_package_json { match *side_effects_from_package_json(package_json.clone()).await? { SideEffectsValue::None => {} - SideEffectsValue::Constant(side_effects) => return Ok(Vc::cell(!side_effects)), + SideEffectsValue::Constant(side_effects) => { + return Ok(if side_effects { + SideEffectsDeclaration::SideEffectful + } else { + SideEffectsDeclaration::SideEffectFree + } + .cell()); + } SideEffectsValue::Glob(glob) => { if let Some(rel_path) = package_json.parent().get_relative_path_to(&path) { let rel_path = rel_path.strip_prefix("./").unwrap_or(&rel_path); - return Ok(Vc::cell(!glob.await?.matches(rel_path))); + return Ok(if glob.await?.matches(rel_path) { + SideEffectsDeclaration::SideEffectful + } else { + SideEffectsDeclaration::SideEffectFree + } + .cell()); } } } } - Ok(Vc::cell(false)) + Ok(SideEffectsDeclaration::None.cell()) } #[turbo_tasks::value(shared)] diff --git a/turbopack/crates/turbopack-ecmascript/src/inlined_bytes_module.rs b/turbopack/crates/turbopack-ecmascript/src/inlined_bytes_module.rs index 106f45fc3c56d..feb4b185e927a 100644 --- a/turbopack/crates/turbopack-ecmascript/src/inlined_bytes_module.rs +++ b/turbopack/crates/turbopack-ecmascript/src/inlined_bytes_module.rs @@ -8,7 +8,7 @@ use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ChunkItem, ChunkType, ChunkableModule, ChunkingContext}, ident::AssetIdent, - module::Module, + module::{Module, ModuleSideEffects}, module_graph::ModuleGraph, output::OutputAssetsReference, source::Source, @@ -51,11 +51,8 @@ impl Module for InlinedBytesJsModule { } #[turbo_tasks::function] - fn is_marked_as_side_effect_free( - self: Vc, - _side_effect_free_packages: Vc, - ) -> Vc { - Vc::cell(true) + fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + ModuleSideEffects::SideEffectFree.cell() } } diff --git a/turbopack/crates/turbopack-ecmascript/src/lib.rs b/turbopack/crates/turbopack-ecmascript/src/lib.rs index eec1f35a322af..227a717ac6365 100644 --- a/turbopack/crates/turbopack-ecmascript/src/lib.rs +++ b/turbopack/crates/turbopack-ecmascript/src/lib.rs @@ -118,7 +118,10 @@ pub use turbopack_resolve::ecmascript as resolve; use self::chunk::{EcmascriptChunkItemContent, EcmascriptChunkType, EcmascriptExports}; use crate::{ analyzer::graph::EvalContext, - chunk::{EcmascriptChunkPlaceable, placeable::is_marked_as_side_effect_free}, + chunk::{ + EcmascriptChunkPlaceable, + placeable::{SideEffectsDeclaration, get_side_effect_free_declaration}, + }, code_gen::{CodeGens, ModifiableAst}, merged_module::MergedEcmascriptModule, parse::generate_js_source_map, @@ -766,20 +769,23 @@ impl Module for EcmascriptModuleAsset { } #[turbo_tasks::function] - async fn is_marked_as_side_effect_free( + async fn side_effects( self: Vc, side_effect_free_packages: Vc, - ) -> Result> { + ) -> Result> { // Check package.json first, so that we can skip parsing the module if it's marked that way. - let pkg_side_effect_free = is_marked_as_side_effect_free( + // We need to respect package.json configuration over any static analysis we might do. + Ok((match *get_side_effect_free_declaration( self.ident().path().owned().await?, side_effect_free_packages, - ); - Ok(if *pkg_side_effect_free.await? { - pkg_side_effect_free - } else { - Vc::cell(self.analyze().await?.side_effects == ModuleSideEffects::SideEffectFree) + ) + .await? + { + SideEffectsDeclaration::SideEffectful => ModuleSideEffects::SideEffectful, + SideEffectsDeclaration::SideEffectFree => ModuleSideEffects::SideEffectFree, + SideEffectsDeclaration::None => self.analyze().await?.side_effects, }) + .cell()) } } diff --git a/turbopack/crates/turbopack-ecmascript/src/manifest/chunk_asset.rs b/turbopack/crates/turbopack-ecmascript/src/manifest/chunk_asset.rs index 06f3276366ca2..d9a327d2a809d 100644 --- a/turbopack/crates/turbopack-ecmascript/src/manifest/chunk_asset.rs +++ b/turbopack/crates/turbopack-ecmascript/src/manifest/chunk_asset.rs @@ -8,7 +8,7 @@ use turbopack_core::{ ChunkableModule, ChunkingContext, ChunkingContextExt, availability_info::AvailabilityInfo, }, ident::AssetIdent, - module::Module, + module::{Module, ModuleSideEffects}, module_graph::{ ModuleGraph, chunk_group_info::ChunkGroup, module_batch::ChunkableModuleOrBatch, }, @@ -154,11 +154,8 @@ impl Module for ManifestAsyncModule { } #[turbo_tasks::function] - fn is_marked_as_side_effect_free( - self: Vc, - _side_effect_free_packages: Vc, - ) -> Vc { - Vc::cell(true) + fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + ModuleSideEffects::SideEffectFree.cell() } } diff --git a/turbopack/crates/turbopack-ecmascript/src/merged_module.rs b/turbopack/crates/turbopack-ecmascript/src/merged_module.rs index 31b8729b93421..632f650831594 100644 --- a/turbopack/crates/turbopack-ecmascript/src/merged_module.rs +++ b/turbopack/crates/turbopack-ecmascript/src/merged_module.rs @@ -1,5 +1,6 @@ use anyhow::{Context, Result}; use turbo_tasks::{ResolvedVc, Vc}; +use turbo_tasks_fs::glob::Glob; use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ @@ -7,7 +8,7 @@ use turbopack_core::{ MergeableModuleExposure, MergeableModules, MergeableModulesExposed, }, ident::AssetIdent, - module::Module, + module::{Module, ModuleSideEffects}, module_graph::ModuleGraph, output::OutputAssetsReference, reference::ModuleReferences, @@ -83,14 +84,19 @@ impl Module for MergedEcmascriptModule { } #[turbo_tasks::function] - async fn references(self: Vc) -> Result> { + fn references(self: Vc) -> Result> { panic!("references() should not be called"); } #[turbo_tasks::function] - async fn is_self_async(&self) -> Result> { + fn is_self_async(&self) -> Result> { panic!("is_self_async() should not be called"); } + #[turbo_tasks::function] + fn side_effects(&self, _side_effect_free_packages: Vc) -> Vc { + // If needed this could be computed by merging the effects from all the merged modules + panic!("side_effects() should not be called"); + } } #[turbo_tasks::value_impl] diff --git a/turbopack/crates/turbopack-ecmascript/src/references/esm/base.rs b/turbopack/crates/turbopack-ecmascript/src/references/esm/base.rs index 7d4fdbd1d4af7..b262b86d4bb01 100644 --- a/turbopack/crates/turbopack-ecmascript/src/references/esm/base.rs +++ b/turbopack/crates/turbopack-ecmascript/src/references/esm/base.rs @@ -22,7 +22,7 @@ use turbopack_core::{ Issue, IssueExt, IssueSeverity, IssueSource, IssueStage, OptionIssueSource, OptionStyledString, StyledString, }, - module::Module, + module::{Module, ModuleSideEffects}, module_graph::binding_usage_info::ModuleExportUsageInfo, reference::ModuleReference, reference_type::{EcmaScriptModulesReferenceSubType, ImportWithType}, @@ -428,10 +428,8 @@ impl ModuleReference for EsmAssetReference { let side_effect_free_packages = self.module.asset_context().side_effect_free_packages(); - if *self - .module - .is_marked_as_side_effect_free(side_effect_free_packages) - .await? + if *self.module.side_effects(side_effect_free_packages).await? + == ModuleSideEffects::SideEffectFree { return Ok(ModuleResolveResult { primary: Box::new([( diff --git a/turbopack/crates/turbopack-ecmascript/src/references/esm/export.rs b/turbopack/crates/turbopack-ecmascript/src/references/esm/export.rs index a4aa6fbd57444..acf92732b4304 100644 --- a/turbopack/crates/turbopack-ecmascript/src/references/esm/export.rs +++ b/turbopack/crates/turbopack-ecmascript/src/references/esm/export.rs @@ -21,7 +21,7 @@ use turbopack_core::{ chunk::{ChunkingContext, ModuleChunkItemIdExt}, ident::AssetIdent, issue::{IssueExt, IssueSeverity, StyledString, analyze::AnalyzeIssue}, - module::Module, + module::{Module, ModuleSideEffects}, module_graph::binding_usage_info::ModuleExportUsageInfo, reference::ModuleReference, resolve::ModulePart, @@ -208,9 +208,8 @@ pub async fn follow_reexports( }; if !ignore_side_effects - && !*module - .is_marked_as_side_effect_free(side_effect_free_packages) - .await? + && *module.side_effects(side_effect_free_packages).await? + != ModuleSideEffects::SideEffectFree { // TODO It's unfortunate that we have to use the whole module here. // This is often the Facade module, which includes all reexports. diff --git a/turbopack/crates/turbopack-ecmascript/src/references/external_module.rs b/turbopack/crates/turbopack-ecmascript/src/references/external_module.rs index 144074a5830e9..4bcaa85c180ea 100644 --- a/turbopack/crates/turbopack-ecmascript/src/references/external_module.rs +++ b/turbopack/crates/turbopack-ecmascript/src/references/external_module.rs @@ -14,7 +14,7 @@ use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{AsyncModuleInfo, ChunkItem, ChunkType, ChunkableModule, ChunkingContext}, ident::{AssetIdent, Layer}, - module::Module, + module::{Module, ModuleSideEffects}, module_graph::ModuleGraph, output::{ OutputAsset, OutputAssets, OutputAssetsReference, OutputAssetsReferences, @@ -382,11 +382,8 @@ impl Module for CachedExternalModule { } #[turbo_tasks::function] - fn is_marked_as_side_effect_free( - self: Vc, - _side_effect_free_packages: Vc, - ) -> Vc { - Vc::cell(false) + fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + ModuleSideEffects::SideEffectful.cell() } } @@ -571,6 +568,10 @@ impl Module for ModuleWithoutSelfAsync { self.module.references() } + #[turbo_tasks::function] + fn side_effects(&self, side_effect_free_packages: Vc) -> Vc { + self.module.side_effects(side_effect_free_packages) + } // Don't override and use default is_self_async that always returns false } diff --git a/turbopack/crates/turbopack-ecmascript/src/references/require_context.rs b/turbopack/crates/turbopack-ecmascript/src/references/require_context.rs index 4220effb0e3cf..98cf4b0224c3b 100644 --- a/turbopack/crates/turbopack-ecmascript/src/references/require_context.rs +++ b/turbopack/crates/turbopack-ecmascript/src/references/require_context.rs @@ -29,7 +29,7 @@ use turbopack_core::{ }, ident::AssetIdent, issue::IssueSource, - module::Module, + module::{Module, ModuleSideEffects}, module_graph::ModuleGraph, output::OutputAssetsReference, reference::{ModuleReference, ModuleReferences}, @@ -445,11 +445,8 @@ impl Module for RequireContextAsset { } #[turbo_tasks::function] - fn is_marked_as_side_effect_free( - self: Vc, - _side_effect_free_packages: Vc, - ) -> Vc { - Vc::cell(true) + fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + ModuleSideEffects::SideEffectFree.cell() } } diff --git a/turbopack/crates/turbopack-ecmascript/src/side_effect_optimization/facade/module.rs b/turbopack/crates/turbopack-ecmascript/src/side_effect_optimization/facade/module.rs index 7fa5eb6b9a587..6146ac34e9fcd 100644 --- a/turbopack/crates/turbopack-ecmascript/src/side_effect_optimization/facade/module.rs +++ b/turbopack/crates/turbopack-ecmascript/src/side_effect_optimization/facade/module.rs @@ -10,7 +10,7 @@ use turbopack_core::{ MergeableModules, MergeableModulesExposed, }, ident::AssetIdent, - module::Module, + module::{Module, ModuleSideEffects}, module_graph::ModuleGraph, reference::ModuleReferences, resolve::{ExportUsage, ModulePart}, @@ -182,16 +182,11 @@ impl Module for EcmascriptModuleFacadeModule { } #[turbo_tasks::function] - fn is_marked_as_side_effect_free( - &self, - side_effect_free_packages: Vc, - ) -> Result> { + fn side_effects(&self, side_effect_free_packages: Vc) -> Result> { Ok(match self.part { - ModulePart::Facade => self - .module - .is_marked_as_side_effect_free(side_effect_free_packages), + ModulePart::Facade => self.module.side_effects(side_effect_free_packages), ModulePart::RenamedExport { .. } | ModulePart::RenamedNamespace { .. } => { - Vc::cell(true) + ModuleSideEffects::SideEffectFree.cell() } _ => bail!("Unexpected ModulePart for EcmascriptModuleFacadeModule"), }) diff --git a/turbopack/crates/turbopack-ecmascript/src/side_effect_optimization/locals/module.rs b/turbopack/crates/turbopack-ecmascript/src/side_effect_optimization/locals/module.rs index 6f687d24f9518..1b7a43fa273eb 100644 --- a/turbopack/crates/turbopack-ecmascript/src/side_effect_optimization/locals/module.rs +++ b/turbopack/crates/turbopack-ecmascript/src/side_effect_optimization/locals/module.rs @@ -10,7 +10,7 @@ use turbopack_core::{ MergeableModulesExposed, }, ident::AssetIdent, - module::Module, + module::{Module, ModuleSideEffects}, module_graph::ModuleGraph, reference::ModuleReferences, resolve::ModulePart, @@ -73,9 +73,8 @@ impl Module for EcmascriptModuleLocalsModule { } #[turbo_tasks::function] - fn is_marked_as_side_effect_free(&self, side_effect_free_packages: Vc) -> Vc { - self.module - .is_marked_as_side_effect_free(side_effect_free_packages) + fn side_effects(&self, side_effect_free_packages: Vc) -> Vc { + self.module.side_effects(side_effect_free_packages) } } diff --git a/turbopack/crates/turbopack-ecmascript/src/tree_shake/asset.rs b/turbopack/crates/turbopack-ecmascript/src/tree_shake/asset.rs index b0ad370ff2ea0..6aa862a5a7a26 100644 --- a/turbopack/crates/turbopack-ecmascript/src/tree_shake/asset.rs +++ b/turbopack/crates/turbopack-ecmascript/src/tree_shake/asset.rs @@ -7,7 +7,7 @@ use turbopack_core::{ chunk::{AsyncModuleInfo, ChunkableModule, ChunkingContext, EvaluatableAsset}, context::AssetContext, ident::AssetIdent, - module::Module, + module::{Module, ModuleSideEffects}, module_graph::ModuleGraph, reference::{ModuleReference, ModuleReferences, SingleChunkableModuleReference}, resolve::{ExportUsage, ModulePart, origin::ResolveOrigin}, @@ -264,8 +264,9 @@ async fn follow_reexports_with_side_effects( let mut current_export_name = export_name; let result = loop { let is_side_effect_free = *current_module - .is_marked_as_side_effect_free(side_effect_free_packages) - .await?; + .side_effects(side_effect_free_packages) + .await? + == ModuleSideEffects::SideEffectFree; if !is_side_effect_free { side_effects.push(only_effects(*current_module).to_resolved().await?); @@ -353,15 +354,12 @@ impl Module for EcmascriptModulePartAsset { } #[turbo_tasks::function] - async fn is_marked_as_side_effect_free( - &self, - side_effect_free_packages: Vc, - ) -> Result> { + async fn side_effects(&self, side_effect_free_packages: Vc) -> Vc { match self.part { - ModulePart::Exports | ModulePart::Export(..) => Ok(Vc::cell(true)), - _ => Ok(self - .full_module - .is_marked_as_side_effect_free(side_effect_free_packages)), + ModulePart::Exports | ModulePart::Export(..) => { + ModuleSideEffects::SideEffectFree.cell() + } + _ => self.full_module.side_effects(side_effect_free_packages), } } } diff --git a/turbopack/crates/turbopack-ecmascript/src/tree_shake/side_effect_module.rs b/turbopack/crates/turbopack-ecmascript/src/tree_shake/side_effect_module.rs index 5c6b2d9a9be6f..163acad58eefe 100644 --- a/turbopack/crates/turbopack-ecmascript/src/tree_shake/side_effect_module.rs +++ b/turbopack/crates/turbopack-ecmascript/src/tree_shake/side_effect_module.rs @@ -6,7 +6,7 @@ use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ChunkableModule, ChunkingContext, EvaluatableAsset}, ident::AssetIdent, - module::Module, + module::{Module, ModuleSideEffects}, module_graph::ModuleGraph, reference::{ModuleReferences, SingleChunkableModuleReference}, resolve::{ExportUsage, ModulePart}, @@ -114,8 +114,8 @@ impl Module for SideEffectsModule { } #[turbo_tasks::function] - fn is_marked_as_side_effect_free(self: Vc, _: Vc) -> Vc { - Vc::cell(true) + fn side_effects(self: Vc, _: Vc) -> Vc { + ModuleSideEffects::SideEffectFree.cell() } } diff --git a/turbopack/crates/turbopack-ecmascript/src/typescript/mod.rs b/turbopack/crates/turbopack-ecmascript/src/typescript/mod.rs index 97217f735824d..c41260cf35994 100644 --- a/turbopack/crates/turbopack-ecmascript/src/typescript/mod.rs +++ b/turbopack/crates/turbopack-ecmascript/src/typescript/mod.rs @@ -2,11 +2,11 @@ use anyhow::Result; use serde_json::Value as JsonValue; use turbo_rcstr::{RcStr, rcstr}; use turbo_tasks::{ResolvedVc, TryJoinIterExt, ValueToString, Vc}; -use turbo_tasks_fs::DirectoryContent; +use turbo_tasks_fs::{DirectoryContent, glob::Glob}; use turbopack_core::{ asset::{Asset, AssetContent}, ident::AssetIdent, - module::Module, + module::{Module, ModuleSideEffects}, raw_module::RawModule, reference::{ModuleReference, ModuleReferences}, reference_type::{CommonJsReferenceSubType, ReferenceType}, @@ -173,6 +173,11 @@ impl Module for TsConfigModuleAsset { } Ok(Vc::cell(references)) } + + #[turbo_tasks::function] + fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + ModuleSideEffects::SideEffectful.cell() + } } #[turbo_tasks::value_impl] diff --git a/turbopack/crates/turbopack-ecmascript/src/webpack/mod.rs b/turbopack/crates/turbopack-ecmascript/src/webpack/mod.rs index 63fbeaf96593a..7af6a12d12b8d 100644 --- a/turbopack/crates/turbopack-ecmascript/src/webpack/mod.rs +++ b/turbopack/crates/turbopack-ecmascript/src/webpack/mod.rs @@ -2,11 +2,12 @@ use anyhow::Result; use swc_core::ecma::ast::Lit; use turbo_rcstr::{RcStr, rcstr}; use turbo_tasks::{ResolvedVc, ValueToString, Vc}; +use turbo_tasks_fs::glob::Glob; use turbopack_core::{ asset::{Asset, AssetContent}, file_source::FileSource, ident::AssetIdent, - module::Module, + module::{Module, ModuleSideEffects}, reference::{ModuleReference, ModuleReferences}, reference_type::{CommonJsReferenceSubType, ReferenceType}, resolve::{ @@ -62,6 +63,11 @@ impl Module for WebpackModuleAsset { fn references(&self) -> Vc { module_references(*self.source, *self.runtime, *self.transforms) } + + #[turbo_tasks::function] + fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + ModuleSideEffects::SideEffectful.cell() + } } #[turbo_tasks::value_impl] diff --git a/turbopack/crates/turbopack-ecmascript/src/worker_chunk/module.rs b/turbopack/crates/turbopack-ecmascript/src/worker_chunk/module.rs index 09dfb6110c9d7..6830bd134dc5e 100644 --- a/turbopack/crates/turbopack-ecmascript/src/worker_chunk/module.rs +++ b/turbopack/crates/turbopack-ecmascript/src/worker_chunk/module.rs @@ -9,7 +9,7 @@ use turbopack_core::{ ChunkingTypeOption, }, ident::AssetIdent, - module::Module, + module::{Module, ModuleSideEffects}, module_graph::ModuleGraph, reference::{ModuleReference, ModuleReferences}, resolve::ModuleResolveResult, @@ -59,11 +59,8 @@ impl Module for WorkerLoaderModule { } #[turbo_tasks::function] - fn is_marked_as_side_effect_free( - self: Vc, - _side_effect_free_packages: Vc, - ) -> Vc { - Vc::cell(true) + fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + ModuleSideEffects::SideEffectFree.cell() } } diff --git a/turbopack/crates/turbopack-json/src/lib.rs b/turbopack/crates/turbopack-json/src/lib.rs index e77a9f7bbe3bc..40a3246601ee8 100644 --- a/turbopack/crates/turbopack-json/src/lib.rs +++ b/turbopack/crates/turbopack-json/src/lib.rs @@ -20,7 +20,7 @@ use turbopack_core::{ chunk::{ChunkItem, ChunkType, ChunkableModule, ChunkingContext}, code_builder::CodeBuilder, ident::AssetIdent, - module::Module, + module::{Module, ModuleSideEffects}, module_graph::ModuleGraph, output::OutputAssetsReference, source::Source, @@ -59,11 +59,8 @@ impl Module for JsonModuleAsset { } #[turbo_tasks::function] - fn is_marked_as_side_effect_free( - self: Vc, - _side_effect_free_packages: Vc, - ) -> Vc { - Vc::cell(true) + fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + ModuleSideEffects::SideEffectFree.cell() } } diff --git a/turbopack/crates/turbopack-static/src/css.rs b/turbopack/crates/turbopack-static/src/css.rs index 975318dc6cd28..3647e3f1f3591 100644 --- a/turbopack/crates/turbopack-static/src/css.rs +++ b/turbopack/crates/turbopack-static/src/css.rs @@ -5,7 +5,7 @@ use turbopack_core::{ asset::{Asset, AssetContent}, chunk::ChunkingContext, ident::AssetIdent, - module::Module, + module::{Module, ModuleSideEffects}, output::OutputAsset, source::Source, }; @@ -53,11 +53,8 @@ impl Module for StaticUrlCssModule { } #[turbo_tasks::function] - fn is_marked_as_side_effect_free( - self: Vc, - _side_effect_free_packages: Vc, - ) -> Vc { - Vc::cell(true) + fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + ModuleSideEffects::SideEffectFree.cell() } } diff --git a/turbopack/crates/turbopack-static/src/ecma.rs b/turbopack/crates/turbopack-static/src/ecma.rs index 5898740e7b410..0feafc8e6396a 100644 --- a/turbopack/crates/turbopack-static/src/ecma.rs +++ b/turbopack/crates/turbopack-static/src/ecma.rs @@ -6,7 +6,7 @@ use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ChunkItem, ChunkType, ChunkableModule, ChunkingContext}, ident::AssetIdent, - module::Module, + module::{Module, ModuleSideEffects}, module_graph::ModuleGraph, output::{OutputAsset, OutputAssetsReference, OutputAssetsWithReferenced}, source::Source, @@ -65,11 +65,8 @@ impl Module for StaticUrlJsModule { } #[turbo_tasks::function] - fn is_marked_as_side_effect_free( - self: Vc, - _side_effect_free_packages: Vc, - ) -> Vc { - Vc::cell(true) + fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + ModuleSideEffects::SideEffectFree.cell() } } diff --git a/turbopack/crates/turbopack-wasm/src/module_asset.rs b/turbopack/crates/turbopack-wasm/src/module_asset.rs index 3db28a19882fb..5f8ba8407b701 100644 --- a/turbopack/crates/turbopack-wasm/src/module_asset.rs +++ b/turbopack/crates/turbopack-wasm/src/module_asset.rs @@ -1,7 +1,7 @@ use anyhow::{Context, Result, bail}; use turbo_rcstr::rcstr; use turbo_tasks::{IntoTraitRef, ResolvedVc, Vc, fxindexmap}; -use turbo_tasks_fs::FileSystemPath; +use turbo_tasks_fs::{FileSystemPath, glob::Glob}; use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ @@ -10,7 +10,7 @@ use turbopack_core::{ }, context::AssetContext, ident::AssetIdent, - module::{Module, OptionModule}, + module::{Module, ModuleSideEffects, OptionModule}, module_graph::ModuleGraph, output::{OutputAssetsReference, OutputAssetsWithReferenced}, reference::{ModuleReferences, SingleChunkableModuleReference}, @@ -144,6 +144,14 @@ impl Module for WebAssemblyModuleAsset { fn is_self_async(self: Vc) -> Vc { Vc::cell(true) } + + #[turbo_tasks::function] + fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + // Both versions of this module have a top level await that instantiates a wasm module + // wasm module instantiation can trigger arbitrary side effects from the native start + // function + ModuleSideEffects::SideEffectful.cell() + } } #[turbo_tasks::value_impl] diff --git a/turbopack/crates/turbopack-wasm/src/raw.rs b/turbopack/crates/turbopack-wasm/src/raw.rs index 03661228b60b6..12f1495944b50 100644 --- a/turbopack/crates/turbopack-wasm/src/raw.rs +++ b/turbopack/crates/turbopack-wasm/src/raw.rs @@ -1,12 +1,13 @@ use anyhow::{Result, bail}; use turbo_rcstr::rcstr; use turbo_tasks::{IntoTraitRef, ResolvedVc, Vc}; +use turbo_tasks_fs::glob::Glob; use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ChunkItem, ChunkType, ChunkableModule, ChunkingContext}, context::AssetContext, ident::AssetIdent, - module::Module, + module::{Module, ModuleSideEffects}, module_graph::ModuleGraph, output::{OutputAsset, OutputAssetsReference, OutputAssetsWithReferenced}, source::{OptionSource, Source}, @@ -64,6 +65,12 @@ impl Module for RawWebAssemblyModuleAsset { fn source(&self) -> Vc { Vc::cell(Some(ResolvedVc::upcast(self.source))) } + + #[turbo_tasks::function] + fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + // this just exports a path + ModuleSideEffects::SideEffectFree.cell() + } } #[turbo_tasks::value_impl] diff --git a/turbopack/crates/turbopack/src/lib.rs b/turbopack/crates/turbopack/src/lib.rs index 858e817efda19..924f34e58a3d6 100644 --- a/turbopack/crates/turbopack/src/lib.rs +++ b/turbopack/crates/turbopack/src/lib.rs @@ -28,7 +28,7 @@ use turbopack_core::{ context::{AssetContext, ProcessResult}, ident::Layer, issue::{IssueExt, IssueSource, module::ModuleIssue}, - module::Module, + module::{Module, ModuleSideEffects}, node_addon_module::NodeAddonModule, output::{ExpandedOutputAssets, OutputAsset}, raw_module::RawModule, @@ -182,9 +182,8 @@ async fn apply_module_type( .resolve() .await?; - if *module - .is_marked_as_side_effect_free(side_effect_free_packages) - .await? + if *module.side_effects(side_effect_free_packages).await? + == ModuleSideEffects::SideEffectFree { return Ok(ProcessResult::Ignore.cell()); } @@ -316,9 +315,8 @@ async fn apply_module_type( .resolve() .await?; - if *module - .is_marked_as_side_effect_free(side_effect_free_packages) - .await? + if *module.side_effects(side_effect_free_packages).await? + == ModuleSideEffects::SideEffectFree { return Ok(ProcessResult::Ignore.cell()); } From 8ad623be7a3416f836dd437a96eac40d89c3e356 Mon Sep 17 00:00:00 2001 From: Luke Sandberg Date: Sun, 30 Nov 2025 14:22:39 -0800 Subject: [PATCH 2/8] simplify the side_effects api by moving the package glob into EcmascriptModule fewer globs constructed, fewer turbotasks executed --- crates/next-core/src/hmr_entry.rs | 4 +- crates/next-core/src/next_client/context.rs | 7 +- .../css_client_reference_module.rs | 5 +- .../ecmascript_client_reference_module.rs | 4 +- .../src/next_dynamic/dynamic_module.rs | 4 +- crates/next-core/src/next_server/context.rs | 7 +- .../server_component_module.rs | 4 +- .../server_utility_module.rs | 4 +- crates/next-core/src/raw_ecmascript_module.rs | 4 +- .../crates/turbopack-core/src/context.rs | 5 +- turbopack/crates/turbopack-core/src/module.rs | 3 +- .../turbopack-core/src/module_graph/mod.rs | 8 +- .../module_graph/side_effect_module_info.rs | 87 +++++++++++++++++++ .../turbopack-core/src/node_addon_module.rs | 4 +- .../crates/turbopack-core/src/raw_module.rs | 3 +- turbopack/crates/turbopack-css/src/asset.rs | 4 +- .../crates/turbopack-css/src/module_asset.rs | 4 +- .../benches/references.rs | 9 +- .../src/async_chunk/module.rs | 3 +- .../src/inlined_bytes_module.rs | 4 +- .../crates/turbopack-ecmascript/src/lib.rs | 40 ++++++--- .../src/manifest/chunk_asset.rs | 3 +- .../turbopack-ecmascript/src/merged_module.rs | 3 +- .../src/references/esm/base.rs | 23 ++--- .../src/references/esm/export.rs | 5 +- .../src/references/external_module.rs | 9 +- .../src/references/require_context.rs | 4 +- .../side_effect_optimization/facade/module.rs | 6 +- .../side_effect_optimization/locals/module.rs | 5 +- .../src/tree_shake/asset.rs | 35 ++------ .../src/tree_shake/side_effect_module.rs | 4 +- .../src/typescript/mod.rs | 4 +- .../turbopack-ecmascript/src/webpack/mod.rs | 3 +- .../src/worker_chunk/module.rs | 3 +- turbopack/crates/turbopack-json/src/lib.rs | 4 +- turbopack/crates/turbopack-static/src/css.rs | 4 +- turbopack/crates/turbopack-static/src/ecma.rs | 3 +- .../src/noop_asset_context.rs | 9 +- .../crates/turbopack-wasm/src/module_asset.rs | 4 +- turbopack/crates/turbopack-wasm/src/raw.rs | 3 +- turbopack/crates/turbopack/src/lib.rs | 57 +++--------- .../module_options/module_options_context.rs | 28 +++++- 42 files changed, 244 insertions(+), 192 deletions(-) create mode 100644 turbopack/crates/turbopack-core/src/module_graph/side_effect_module_info.rs diff --git a/crates/next-core/src/hmr_entry.rs b/crates/next-core/src/hmr_entry.rs index 2549e50c864c4..abbed4209daba 100644 --- a/crates/next-core/src/hmr_entry.rs +++ b/crates/next-core/src/hmr_entry.rs @@ -3,7 +3,7 @@ use std::io::Write; use anyhow::Result; use turbo_rcstr::{RcStr, rcstr}; use turbo_tasks::{ResolvedVc, ValueToString, Vc}; -use turbo_tasks_fs::{FileSystem, VirtualFileSystem, glob::Glob, rope::RopeBuilder}; +use turbo_tasks_fs::{FileSystem, VirtualFileSystem, rope::RopeBuilder}; use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ @@ -77,7 +77,7 @@ impl Module for HmrEntryModule { )])) } #[turbo_tasks::function] - fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + fn side_effects(self: Vc) -> Vc { ModuleSideEffects::SideEffectful.cell() } } diff --git a/crates/next-core/src/next_client/context.rs b/crates/next-core/src/next_client/context.rs index 22746215bc9b8..cfc6b9c7416c5 100644 --- a/crates/next-core/src/next_client/context.rs +++ b/crates/next-core/src/next_client/context.rs @@ -9,6 +9,7 @@ use turbo_tasks_fs::FileSystemPath; use turbopack::module_options::{ CssOptionsContext, EcmascriptOptionsContext, JsxTransformOptions, ModuleRule, TypescriptTransformOptions, module_options_context::ModuleOptionsContext, + side_effect_free_packages_glob, }; use turbopack_browser::{ BrowserChunkingContext, ChunkSuffix, ContentHashing, CurrentChunkMethod, @@ -337,7 +338,11 @@ pub async fn get_client_module_options_context( execution_context: Some(execution_context), tree_shaking_mode: tree_shaking_mode_for_user_code, enable_postcss_transform, - side_effect_free_packages: next_config.optimize_package_imports().owned().await?, + side_effect_free_packages: Some( + side_effect_free_packages_glob(next_config.optimize_package_imports()) + .to_resolved() + .await?, + ), keep_last_successful_parse: next_mode.is_development(), analyze_mode: if next_mode.is_development() { AnalyzeMode::CodeGeneration diff --git a/crates/next-core/src/next_client_reference/css_client_reference/css_client_reference_module.rs b/crates/next-core/src/next_client_reference/css_client_reference/css_client_reference_module.rs index 9c6ecc57911b2..fc059495d6881 100644 --- a/crates/next-core/src/next_client_reference/css_client_reference/css_client_reference_module.rs +++ b/crates/next-core/src/next_client_reference/css_client_reference/css_client_reference_module.rs @@ -1,8 +1,7 @@ use anyhow::Result; use turbo_rcstr::{RcStr, rcstr}; use turbo_tasks::{ResolvedVc, ValueToString, Vc}; -use turbo_tasks_fs::{FileContent, glob::Glob}; -use turbopack::css::chunk::CssChunkPlaceable; +use turbo_tasks_fs::FileContent; use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ChunkGroupType, ChunkableModuleReference, ChunkingType, ChunkingTypeOption}, @@ -59,7 +58,7 @@ impl Module for CssClientReferenceModule { )])) } #[turbo_tasks::function] - fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + fn side_effects(self: Vc) -> Vc { ModuleSideEffects::SideEffectful.cell() } } diff --git a/crates/next-core/src/next_client_reference/ecmascript_client_reference/ecmascript_client_reference_module.rs b/crates/next-core/src/next_client_reference/ecmascript_client_reference/ecmascript_client_reference_module.rs index 8657136a009f0..e63def3fd0e9a 100644 --- a/crates/next-core/src/next_client_reference/ecmascript_client_reference/ecmascript_client_reference_module.rs +++ b/crates/next-core/src/next_client_reference/ecmascript_client_reference/ecmascript_client_reference_module.rs @@ -4,7 +4,7 @@ use anyhow::{Context, Result, bail}; use indoc::writedoc; use turbo_rcstr::{RcStr, rcstr}; use turbo_tasks::{IntoTraitRef, ResolvedVc, ValueToString, Vc}; -use turbo_tasks_fs::{File, FileContent, glob::Glob}; +use turbo_tasks_fs::{File, FileContent}; use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ @@ -242,7 +242,7 @@ impl Module for EcmascriptClientReferenceModule { Ok(Vc::cell(references)) } #[turbo_tasks::function] - fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + fn side_effects(self: Vc) -> Vc { // These just export some specially tagged functions ModuleSideEffects::SideEffectFree.cell() } diff --git a/crates/next-core/src/next_dynamic/dynamic_module.rs b/crates/next-core/src/next_dynamic/dynamic_module.rs index e7ff33faf32b5..07a0c3b6b1225 100644 --- a/crates/next-core/src/next_dynamic/dynamic_module.rs +++ b/crates/next-core/src/next_dynamic/dynamic_module.rs @@ -4,7 +4,7 @@ use anyhow::Result; use indoc::formatdoc; use turbo_rcstr::{RcStr, rcstr}; use turbo_tasks::{ResolvedVc, Vc}; -use turbo_tasks_fs::{FileContent, glob::Glob}; +use turbo_tasks_fs::FileContent; use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ChunkItem, ChunkType, ChunkableModule, ChunkingContext, ModuleChunkItemIdExt}, @@ -72,7 +72,7 @@ impl Module for NextDynamicEntryModule { )])) } #[turbo_tasks::function] - fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + fn side_effects(self: Vc) -> Vc { // This just exports another import ModuleSideEffects::ModuleEvaluationIsSideEffectFree.cell() } diff --git a/crates/next-core/src/next_server/context.rs b/crates/next-core/src/next_server/context.rs index e5c677f5c004d..47996eeb99140 100644 --- a/crates/next-core/src/next_server/context.rs +++ b/crates/next-core/src/next_server/context.rs @@ -10,6 +10,7 @@ use turbopack::{ module_options::{ CssOptionsContext, EcmascriptOptionsContext, ExternalsTracingOptions, JsxTransformOptions, ModuleOptionsContext, ModuleRule, TypescriptTransformOptions, + side_effect_free_packages_glob, }, transition::Transition, }; @@ -592,7 +593,11 @@ pub async fn get_server_module_options_context( ..Default::default() }, tree_shaking_mode: tree_shaking_mode_for_user_code, - side_effect_free_packages: next_config.optimize_package_imports().owned().await?, + side_effect_free_packages: Some( + side_effect_free_packages_glob(next_config.optimize_package_imports()) + .to_resolved() + .await?, + ), analyze_mode: if next_mode.is_development() { AnalyzeMode::CodeGeneration } else { diff --git a/crates/next-core/src/next_server_component/server_component_module.rs b/crates/next-core/src/next_server_component/server_component_module.rs index 130bb540ecb1e..e3f65fe7d18a3 100644 --- a/crates/next-core/src/next_server_component/server_component_module.rs +++ b/crates/next-core/src/next_server_component/server_component_module.rs @@ -4,7 +4,7 @@ use anyhow::Result; use indoc::formatdoc; use turbo_rcstr::rcstr; use turbo_tasks::{ResolvedVc, Vc}; -use turbo_tasks_fs::{FileContent, FileSystemPath, glob::Glob}; +use turbo_tasks_fs::{FileContent, FileSystemPath}; use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ChunkItem, ChunkType, ChunkableModule, ChunkingContext, ModuleChunkItemIdExt}, @@ -68,7 +68,7 @@ impl Module for NextServerComponentModule { )])) } #[turbo_tasks::function] - fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + fn side_effects(self: Vc) -> Vc { // This just exports another import ModuleSideEffects::ModuleEvaluationIsSideEffectFree.cell() } diff --git a/crates/next-core/src/next_server_utility/server_utility_module.rs b/crates/next-core/src/next_server_utility/server_utility_module.rs index c5712f8771ffa..dd2b7c41f9592 100644 --- a/crates/next-core/src/next_server_utility/server_utility_module.rs +++ b/crates/next-core/src/next_server_utility/server_utility_module.rs @@ -4,7 +4,7 @@ use anyhow::Result; use indoc::formatdoc; use turbo_rcstr::rcstr; use turbo_tasks::{ResolvedVc, Vc}; -use turbo_tasks_fs::{FileContent, FileSystemPath, glob::Glob}; +use turbo_tasks_fs::{FileContent, FileSystemPath}; use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ChunkItem, ChunkType, ChunkableModule, ChunkingContext, ModuleChunkItemIdExt}, @@ -69,7 +69,7 @@ impl Module for NextServerUtilityModule { } #[turbo_tasks::function] - fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + fn side_effects(self: Vc) -> Vc { // This just exports another import ModuleSideEffects::ModuleEvaluationIsSideEffectFree.cell() } diff --git a/crates/next-core/src/raw_ecmascript_module.rs b/crates/next-core/src/raw_ecmascript_module.rs index 1d7888f03b29c..c58cbe07638e9 100644 --- a/crates/next-core/src/raw_ecmascript_module.rs +++ b/crates/next-core/src/raw_ecmascript_module.rs @@ -7,7 +7,7 @@ use regex::Regex; use tracing::Instrument; use turbo_rcstr::rcstr; use turbo_tasks::{FxIndexMap, FxIndexSet, ResolvedVc, TryJoinIterExt, ValueToString, Vc}; -use turbo_tasks_fs::{FileContent, glob::Glob, rope::Rope}; +use turbo_tasks_fs::{FileContent, rope::Rope}; use turbopack::{ModuleAssetContext, module_options::CustomModuleType}; use turbopack_core::{ asset::{Asset, AssetContent}, @@ -99,7 +99,7 @@ impl Module for RawEcmascriptModule { } #[turbo_tasks::function] - fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + fn side_effects(self: Vc) -> Vc { // Is this correct? ModuleSideEffects::SideEffectFree.cell() } diff --git a/turbopack/crates/turbopack-core/src/context.rs b/turbopack/crates/turbopack-core/src/context.rs index 3c135b8fd3ff1..8f9177907a26d 100644 --- a/turbopack/crates/turbopack-core/src/context.rs +++ b/turbopack/crates/turbopack-core/src/context.rs @@ -1,7 +1,7 @@ use anyhow::{Result, bail}; use turbo_rcstr::RcStr; use turbo_tasks::{ResolvedVc, Vc}; -use turbo_tasks_fs::{FileSystemPath, glob::Glob}; +use turbo_tasks_fs::FileSystemPath; use crate::{ compile_time_info::CompileTimeInfo, @@ -104,7 +104,4 @@ pub trait AssetContext { /// Gets a new AssetContext with the transition applied. #[turbo_tasks::function] fn with_transition(self: Vc, transition: RcStr) -> Vc>; - - #[turbo_tasks::function] - fn side_effect_free_packages(self: Vc) -> Vc; } diff --git a/turbopack/crates/turbopack-core/src/module.rs b/turbopack/crates/turbopack-core/src/module.rs index 3b84653f0f13e..b55a6dd35394d 100644 --- a/turbopack/crates/turbopack-core/src/module.rs +++ b/turbopack/crates/turbopack-core/src/module.rs @@ -1,6 +1,5 @@ use turbo_rcstr::RcStr; use turbo_tasks::{ResolvedVc, TaskInput, ValueToString, Vc}; -use turbo_tasks_fs::glob::Glob; use crate::{asset::Asset, ident::AssetIdent, reference::ModuleReferences, source::OptionSource}; @@ -61,7 +60,7 @@ pub trait Module: Asset { /// Returns true if the module is marked as side effect free in package.json or by other means. #[turbo_tasks::function] - fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc; + fn side_effects(self: Vc) -> Vc; } #[turbo_tasks::value_trait] diff --git a/turbopack/crates/turbopack-core/src/module_graph/mod.rs b/turbopack/crates/turbopack-core/src/module_graph/mod.rs index b21f9b8448fb4..4e098c2f1135c 100644 --- a/turbopack/crates/turbopack-core/src/module_graph/mod.rs +++ b/turbopack/crates/turbopack-core/src/module_graph/mod.rs @@ -48,6 +48,7 @@ pub mod chunk_group_info; pub mod merged_modules; pub mod module_batch; pub(crate) mod module_batches; +mod side_effect_module_info; pub(crate) mod style_groups; mod traced_di_graph; @@ -1954,7 +1955,7 @@ pub mod tests { use turbo_rcstr::{RcStr, rcstr}; use turbo_tasks::{ResolvedVc, TryJoinIterExt, ValueToString, Vc}; use turbo_tasks_backend::{BackendOptions, TurboTasksBackend, noop_backing_storage}; - use turbo_tasks_fs::{FileSystem, FileSystemPath, VirtualFileSystem, glob::Glob}; + use turbo_tasks_fs::{FileSystem, FileSystemPath, VirtualFileSystem}; use crate::{ asset::{Asset, AssetContent}, @@ -2358,10 +2359,7 @@ pub mod tests { Ok(Vc::cell(references)) } #[turbo_tasks::function] - fn side_effects( - self: Vc, - _side_effect_free_packages: Vc, - ) -> Vc { + fn side_effects(self: Vc) -> Vc { ModuleSideEffects::SideEffectful.cell() } } diff --git a/turbopack/crates/turbopack-core/src/module_graph/side_effect_module_info.rs b/turbopack/crates/turbopack-core/src/module_graph/side_effect_module_info.rs new file mode 100644 index 0000000000000..f396e1dfa659b --- /dev/null +++ b/turbopack/crates/turbopack-core/src/module_graph/side_effect_module_info.rs @@ -0,0 +1,87 @@ +use anyhow::Result; +use rustc_hash::FxHashSet; +use turbo_tasks::{ResolvedVc, TryJoinIterExt, Vc}; + +use crate::{ + module::Module, + module_graph::{GraphTraversalAction, ModuleGraph, SingleModuleGraphWithBindingUsage}, +}; + +/// This lists all the modules that are side effect free +/// This means they are either declared side effect free by some configuration or they have been +/// determined to be side effect free via static analysis of the module evaluation and dependencies. +#[turbo_tasks::value(transparent)] +pub struct SideEffectFreeModules(FxHashSet>>); + +#[turbo_tasks::function(operation)] +pub async fn compute_side_effect_free_module_info( + graphs: ResolvedVc, +) -> Result> { + // Layout segment optimization, we can individually compute the side effect free modules for + // each graph. + let mut result: Vc = Vc::cell(Default::default()); + let graphs = graphs.await?; + for graph in graphs.iter_graphs() { + result = compute_side_effect_free_module_info_single(graph, result); + } + Ok(result) +} + +#[turbo_tasks::function] +async fn compute_side_effect_free_module_info_single( + graph: SingleModuleGraphWithBindingUsage, + parent_side_effect_free_modules: Vc, +) -> Result> { + let parent_async_modules = parent_side_effect_free_modules.await?; + let graph = graph.read().await?; + + let self_async_modules = graph + .graphs + .first() + .unwrap() + .iter_nodes() + .map(async |node| Ok((node, *node.is_self_async().await?))) + .try_join() + .await? + .into_iter() + .flat_map(|(k, v)| v.then_some(k)) + .chain(parent_async_modules.iter().copied()) + .collect::>(); + + // To determine which modules are async, we need to propagate the self-async flag to all + // importers, which is done using a postorder traversal of the graph. + // + // This however doesn't cover cycles of async modules, which are handled by determining all + // strongly-connected components, and then marking all the whole SCC as async if one of the + // modules in the SCC is async. + + let mut async_modules = self_async_modules; + graph.traverse_edges_from_entries_dfs( + graph.graphs.first().unwrap().entry_modules(), + &mut (), + |_, _, _| Ok(GraphTraversalAction::Continue), + |parent_info, module, _| { + let Some((parent_module, ref_data)) = parent_info else { + // An entry module + return Ok(()); + }; + + if ref_data.chunking_type.is_inherit_async() && async_modules.contains(&module) { + async_modules.insert(parent_module); + } + Ok(()) + }, + )?; + + graph.traverse_cycles( + |ref_data| ref_data.chunking_type.is_inherit_async(), + |cycle| { + if cycle.iter().any(|node| async_modules.contains(node)) { + async_modules.extend(cycle.iter().map(|n| **n)); + } + Ok(()) + }, + )?; + + Ok(Vc::cell(async_modules)) +} diff --git a/turbopack/crates/turbopack-core/src/node_addon_module.rs b/turbopack/crates/turbopack-core/src/node_addon_module.rs index 7800a068fad98..61051634455e5 100644 --- a/turbopack/crates/turbopack-core/src/node_addon_module.rs +++ b/turbopack/crates/turbopack-core/src/node_addon_module.rs @@ -4,7 +4,7 @@ use anyhow::{Result, bail}; use regex::Regex; use turbo_rcstr::rcstr; use turbo_tasks::{FxIndexSet, ResolvedVc, TryJoinIterExt, Vc}; -use turbo_tasks_fs::{FileSystemEntryType, FileSystemPath, glob::Glob}; +use turbo_tasks_fs::{FileSystemEntryType, FileSystemPath}; use crate::{ asset::{Asset, AssetContent}, @@ -102,7 +102,7 @@ impl Module for NodeAddonModule { } #[turbo_tasks::function] - fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + fn side_effects(self: Vc) -> Vc { ModuleSideEffects::SideEffectful.cell() } } diff --git a/turbopack/crates/turbopack-core/src/raw_module.rs b/turbopack/crates/turbopack-core/src/raw_module.rs index 6f61c64c25734..cc0b430eeed33 100644 --- a/turbopack/crates/turbopack-core/src/raw_module.rs +++ b/turbopack/crates/turbopack-core/src/raw_module.rs @@ -1,5 +1,4 @@ use turbo_tasks::{ResolvedVc, Vc}; -use turbo_tasks_fs::glob::Glob; use crate::{ asset::{Asset, AssetContent}, @@ -27,7 +26,7 @@ impl Module for RawModule { Vc::cell(Some(self.source)) } #[turbo_tasks::function] - fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + fn side_effects(self: Vc) -> Vc { ModuleSideEffects::SideEffectful.cell() } } diff --git a/turbopack/crates/turbopack-css/src/asset.rs b/turbopack/crates/turbopack-css/src/asset.rs index b097d57c077d2..9cd9b5e4b38f7 100644 --- a/turbopack/crates/turbopack-css/src/asset.rs +++ b/turbopack/crates/turbopack-css/src/asset.rs @@ -1,7 +1,7 @@ use anyhow::Result; use turbo_rcstr::rcstr; use turbo_tasks::{IntoTraitRef, ResolvedVc, TryJoinIterExt, ValueToString, Vc}; -use turbo_tasks_fs::{FileContent, FileSystemPath, glob::Glob}; +use turbo_tasks_fs::{FileContent, FileSystemPath}; use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ChunkItem, ChunkType, ChunkableModule, ChunkingContext, MinifyType}, @@ -154,7 +154,7 @@ impl Module for CssModuleAsset { } } #[turbo_tasks::function] - fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + fn side_effects(self: Vc) -> Vc { // global css is always a side effect ModuleSideEffects::SideEffectful.cell() } diff --git a/turbopack/crates/turbopack-css/src/module_asset.rs b/turbopack/crates/turbopack-css/src/module_asset.rs index 9072024882cca..4179333ba55ad 100644 --- a/turbopack/crates/turbopack-css/src/module_asset.rs +++ b/turbopack/crates/turbopack-css/src/module_asset.rs @@ -6,7 +6,7 @@ use lightningcss::css_modules::CssModuleReference; use swc_core::common::{BytePos, FileName, LineCol, SourceMap}; use turbo_rcstr::{RcStr, rcstr}; use turbo_tasks::{FxIndexMap, IntoTraitRef, ResolvedVc, ValueToString, Vc}; -use turbo_tasks_fs::{FileSystemPath, glob::Glob, rope::Rope}; +use turbo_tasks_fs::{FileSystemPath, rope::Rope}; use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ChunkItem, ChunkType, ChunkableModule, ChunkingContext, ModuleChunkItemIdExt}, @@ -113,7 +113,7 @@ impl Module for ModuleCssAsset { } #[turbo_tasks::function] - fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + fn side_effects(self: Vc) -> Vc { // modules can still effect global styles using `:root` selectors and other similar features // We could do better with some static analysis if we want ModuleSideEffects::SideEffectful.cell() diff --git a/turbopack/crates/turbopack-ecmascript/benches/references.rs b/turbopack/crates/turbopack-ecmascript/benches/references.rs index 60160f950d5ac..32cf899317210 100644 --- a/turbopack/crates/turbopack-ecmascript/benches/references.rs +++ b/turbopack/crates/turbopack-ecmascript/benches/references.rs @@ -5,7 +5,10 @@ use criterion::{BatchSize, Bencher, BenchmarkId, Criterion, criterion_group, cri use turbo_rcstr::rcstr; use turbo_tasks::{ResolvedVc, TurboTasks}; use turbo_tasks_backend::{BackendOptions, TurboTasksBackend, noop_backing_storage}; -use turbo_tasks_fs::{DiskFileSystem, FileSystem}; +use turbo_tasks_fs::{ + DiskFileSystem, FileSystem, + glob::{Glob, GlobOptions}, +}; use turbopack_core::{ compile_time_info::CompileTimeInfo, environment::{Environment, ExecutionEnvironment, NodeJsEnvironment}, @@ -77,6 +80,9 @@ async fn setup( layer, } .resolved_cell(); + let side_effect_free_packages = Glob::new("".into(), GlobOptions::default()) + .to_resolved() + .await?; let module = EcmascriptModuleAsset::builder( ResolvedVc::upcast( FileSource::new(fs.root().await?.join(file).unwrap()) @@ -96,6 +102,7 @@ async fn setup( } .resolved_cell(), compile_time_info, + Some(side_effect_free_packages), ) .build() .to_resolved() diff --git a/turbopack/crates/turbopack-ecmascript/src/async_chunk/module.rs b/turbopack/crates/turbopack-ecmascript/src/async_chunk/module.rs index 72f462096b51a..4b35fa32b7da7 100644 --- a/turbopack/crates/turbopack-ecmascript/src/async_chunk/module.rs +++ b/turbopack/crates/turbopack-ecmascript/src/async_chunk/module.rs @@ -1,7 +1,6 @@ use anyhow::Result; use turbo_rcstr::rcstr; use turbo_tasks::{ResolvedVc, Vc}; -use turbo_tasks_fs::glob::Glob; use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ChunkableModule, ChunkingContext, availability_info::AvailabilityInfo}, @@ -68,7 +67,7 @@ impl Module for AsyncLoaderModule { } #[turbo_tasks::function] - fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + fn side_effects(self: Vc) -> Vc { ModuleSideEffects::SideEffectFree.cell() } } diff --git a/turbopack/crates/turbopack-ecmascript/src/inlined_bytes_module.rs b/turbopack/crates/turbopack-ecmascript/src/inlined_bytes_module.rs index feb4b185e927a..394e2d202a2b3 100644 --- a/turbopack/crates/turbopack-ecmascript/src/inlined_bytes_module.rs +++ b/turbopack/crates/turbopack-ecmascript/src/inlined_bytes_module.rs @@ -3,7 +3,7 @@ use std::io::{Read, Write}; use anyhow::{Result, bail}; use turbo_rcstr::rcstr; use turbo_tasks::{ResolvedVc, ValueToString, Vc}; -use turbo_tasks_fs::{FileContent, glob::Glob, rope::RopeBuilder}; +use turbo_tasks_fs::{FileContent, rope::RopeBuilder}; use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ChunkItem, ChunkType, ChunkableModule, ChunkingContext}, @@ -51,7 +51,7 @@ impl Module for InlinedBytesJsModule { } #[turbo_tasks::function] - fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + fn side_effects(self: Vc) -> Vc { ModuleSideEffects::SideEffectFree.cell() } } diff --git a/turbopack/crates/turbopack-ecmascript/src/lib.rs b/turbopack/crates/turbopack-ecmascript/src/lib.rs index 227a717ac6365..0ced73a4d5ab8 100644 --- a/turbopack/crates/turbopack-ecmascript/src/lib.rs +++ b/turbopack/crates/turbopack-ecmascript/src/lib.rs @@ -338,6 +338,7 @@ pub struct EcmascriptModuleAssetBuilder { transforms: ResolvedVc, options: ResolvedVc, compile_time_info: ResolvedVc, + side_effect_free_packages: Option>, inner_assets: Option>, } @@ -361,6 +362,7 @@ impl EcmascriptModuleAssetBuilder { *self.transforms, *self.options, *self.compile_time_info, + self.side_effect_free_packages.map(|g| *g), *inner_assets, ) } else { @@ -371,6 +373,7 @@ impl EcmascriptModuleAssetBuilder { *self.transforms, *self.options, *self.compile_time_info, + self.side_effect_free_packages.map(|g| *g), ) } } @@ -384,6 +387,7 @@ pub struct EcmascriptModuleAsset { pub transforms: ResolvedVc, pub options: ResolvedVc, pub compile_time_info: ResolvedVc, + pub side_effect_free_packages: Option>, pub inner_assets: Option>, #[turbo_tasks(debug_ignore)] last_successful_parse: turbo_tasks::TransientState>, @@ -397,6 +401,7 @@ impl core::fmt::Debug for EcmascriptModuleAsset { .field("transforms", &self.transforms) .field("options", &self.options) .field("compile_time_info", &self.compile_time_info) + .field("side_effect_free_packages", &self.side_effect_free_packages) .field("inner_assets", &self.inner_assets) .finish() } @@ -465,6 +470,7 @@ impl EcmascriptModuleAsset { transforms: ResolvedVc, options: ResolvedVc, compile_time_info: ResolvedVc, + side_effect_free_packages: Option>, ) -> EcmascriptModuleAssetBuilder { EcmascriptModuleAssetBuilder { source, @@ -473,6 +479,7 @@ impl EcmascriptModuleAsset { transforms, options, compile_time_info, + side_effect_free_packages, inner_assets: None, } } @@ -634,13 +641,14 @@ async fn determine_module_type_for_directory( #[turbo_tasks::value_impl] impl EcmascriptModuleAsset { #[turbo_tasks::function] - pub fn new( + fn new( source: ResolvedVc>, asset_context: ResolvedVc>, ty: EcmascriptModuleAssetType, transforms: ResolvedVc, options: ResolvedVc, compile_time_info: ResolvedVc, + side_effect_free_packages: Option>, ) -> Vc { Self::cell(EcmascriptModuleAsset { source, @@ -648,21 +656,22 @@ impl EcmascriptModuleAsset { ty, transforms, options, - compile_time_info, + side_effect_free_packages, inner_assets: None, last_successful_parse: Default::default(), }) } #[turbo_tasks::function] - pub async fn new_with_inner_assets( + async fn new_with_inner_assets( source: ResolvedVc>, asset_context: ResolvedVc>, ty: EcmascriptModuleAssetType, transforms: ResolvedVc, options: ResolvedVc, compile_time_info: ResolvedVc, + side_effect_free_packages: Option>, inner_assets: ResolvedVc, ) -> Result> { if inner_assets.await?.is_empty() { @@ -673,6 +682,7 @@ impl EcmascriptModuleAsset { *transforms, *options, *compile_time_info, + side_effect_free_packages.map(|g| *g), )) } else { Ok(Self::cell(EcmascriptModuleAsset { @@ -682,6 +692,7 @@ impl EcmascriptModuleAsset { transforms, options, compile_time_info, + side_effect_free_packages, inner_assets: Some(inner_assets), last_successful_parse: Default::default(), })) @@ -769,18 +780,21 @@ impl Module for EcmascriptModuleAsset { } #[turbo_tasks::function] - async fn side_effects( - self: Vc, - side_effect_free_packages: Vc, - ) -> Result> { + async fn side_effects(self: Vc) -> Result> { + let this = self.await?; // Check package.json first, so that we can skip parsing the module if it's marked that way. // We need to respect package.json configuration over any static analysis we might do. - Ok((match *get_side_effect_free_declaration( - self.ident().path().owned().await?, - side_effect_free_packages, - ) - .await? - { + let side_effects_declaration = match &this.side_effect_free_packages { + Some(side_effect_free_packages) => { + *get_side_effect_free_declaration( + self.ident().path().owned().await?, + **side_effect_free_packages, + ) + .await? + } + None => SideEffectsDeclaration::None, + }; + Ok((match side_effects_declaration { SideEffectsDeclaration::SideEffectful => ModuleSideEffects::SideEffectful, SideEffectsDeclaration::SideEffectFree => ModuleSideEffects::SideEffectFree, SideEffectsDeclaration::None => self.analyze().await?.side_effects, diff --git a/turbopack/crates/turbopack-ecmascript/src/manifest/chunk_asset.rs b/turbopack/crates/turbopack-ecmascript/src/manifest/chunk_asset.rs index d9a327d2a809d..826ec5c644c2c 100644 --- a/turbopack/crates/turbopack-ecmascript/src/manifest/chunk_asset.rs +++ b/turbopack/crates/turbopack-ecmascript/src/manifest/chunk_asset.rs @@ -1,7 +1,6 @@ use anyhow::Result; use turbo_rcstr::{RcStr, rcstr}; use turbo_tasks::{ResolvedVc, TryJoinIterExt, Vc}; -use turbo_tasks_fs::glob::Glob; use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ @@ -154,7 +153,7 @@ impl Module for ManifestAsyncModule { } #[turbo_tasks::function] - fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + fn side_effects(self: Vc) -> Vc { ModuleSideEffects::SideEffectFree.cell() } } diff --git a/turbopack/crates/turbopack-ecmascript/src/merged_module.rs b/turbopack/crates/turbopack-ecmascript/src/merged_module.rs index 632f650831594..eef2535a4dea2 100644 --- a/turbopack/crates/turbopack-ecmascript/src/merged_module.rs +++ b/turbopack/crates/turbopack-ecmascript/src/merged_module.rs @@ -1,6 +1,5 @@ use anyhow::{Context, Result}; use turbo_tasks::{ResolvedVc, Vc}; -use turbo_tasks_fs::glob::Glob; use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ @@ -93,7 +92,7 @@ impl Module for MergedEcmascriptModule { panic!("is_self_async() should not be called"); } #[turbo_tasks::function] - fn side_effects(&self, _side_effect_free_packages: Vc) -> Vc { + fn side_effects(&self) -> Vc { // If needed this could be computed by merging the effects from all the merged modules panic!("side_effects() should not be called"); } diff --git a/turbopack/crates/turbopack-ecmascript/src/references/esm/base.rs b/turbopack/crates/turbopack-ecmascript/src/references/esm/base.rs index b262b86d4bb01..f026cd85ba5c0 100644 --- a/turbopack/crates/turbopack-ecmascript/src/references/esm/base.rs +++ b/turbopack/crates/turbopack-ecmascript/src/references/esm/base.rs @@ -17,7 +17,6 @@ use turbopack_core::{ ChunkableModuleReference, ChunkingContext, ChunkingType, ChunkingTypeOption, ModuleChunkItemIdExt, }, - context::AssetContext, issue::{ Issue, IssueExt, IssueSeverity, IssueSource, IssueStage, OptionIssueSource, OptionStyledString, StyledString, @@ -424,22 +423,14 @@ impl ModuleReference for EsmAssetReference { let request = Request::parse(self.request.clone().into()); if let Some(TreeShakingMode::ModuleFragments) = self.tree_shaking_mode { - if let Some(ModulePart::Evaluation) = &self.export_name { - let side_effect_free_packages = - self.module.asset_context().side_effect_free_packages(); - - if *self.module.side_effects(side_effect_free_packages).await? - == ModuleSideEffects::SideEffectFree - { - return Ok(ModuleResolveResult { - primary: Box::new([( - RequestKey::default(), - ModuleResolveResultItem::Ignore, - )]), - affecting_sources: Default::default(), - } - .cell()); + if let Some(ModulePart::Evaluation) = &self.export_name + && *self.module.side_effects().await? == ModuleSideEffects::SideEffectFree + { + return Ok(ModuleResolveResult { + primary: Box::new([(RequestKey::default(), ModuleResolveResultItem::Ignore)]), + affecting_sources: Default::default(), } + .cell()); } if let Request::Module { module, .. } = &*request.await? diff --git a/turbopack/crates/turbopack-ecmascript/src/references/esm/export.rs b/turbopack/crates/turbopack-ecmascript/src/references/esm/export.rs index acf92732b4304..5868741e85a0c 100644 --- a/turbopack/crates/turbopack-ecmascript/src/references/esm/export.rs +++ b/turbopack/crates/turbopack-ecmascript/src/references/esm/export.rs @@ -16,7 +16,6 @@ use turbo_tasks::{ FxIndexMap, NonLocalValue, ResolvedVc, TryFlatJoinIterExt, ValueToString, Vc, trace::TraceRawVcs, }; -use turbo_tasks_fs::glob::Glob; use turbopack_core::{ chunk::{ChunkingContext, ModuleChunkItemIdExt}, ident::AssetIdent, @@ -190,7 +189,6 @@ pub struct FollowExportsResult { pub async fn follow_reexports( module: ResolvedVc>, export_name: RcStr, - side_effect_free_packages: Vc, ignore_side_effect_of_entry: bool, ) -> Result> { let mut ignore_side_effects = ignore_side_effect_of_entry; @@ -208,8 +206,7 @@ pub async fn follow_reexports( }; if !ignore_side_effects - && *module.side_effects(side_effect_free_packages).await? - != ModuleSideEffects::SideEffectFree + && *module.side_effects().await? != ModuleSideEffects::SideEffectFree { // TODO It's unfortunate that we have to use the whole module here. // This is often the Facade module, which includes all reexports. diff --git a/turbopack/crates/turbopack-ecmascript/src/references/external_module.rs b/turbopack/crates/turbopack-ecmascript/src/references/external_module.rs index 4bcaa85c180ea..2f52073fd2bd8 100644 --- a/turbopack/crates/turbopack-ecmascript/src/references/external_module.rs +++ b/turbopack/crates/turbopack-ecmascript/src/references/external_module.rs @@ -6,8 +6,7 @@ use serde::{Deserialize, Serialize}; use turbo_rcstr::{RcStr, rcstr}; use turbo_tasks::{NonLocalValue, ResolvedVc, TaskInput, TryJoinIterExt, Vc, trace::TraceRawVcs}; use turbo_tasks_fs::{ - FileContent, FileSystem, FileSystemPath, LinkType, VirtualFileSystem, glob::Glob, - rope::RopeBuilder, + FileContent, FileSystem, FileSystemPath, LinkType, VirtualFileSystem, rope::RopeBuilder, }; use turbo_tasks_hash::{encode_hex, hash_xxh3_hash64}; use turbopack_core::{ @@ -382,7 +381,7 @@ impl Module for CachedExternalModule { } #[turbo_tasks::function] - fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + fn side_effects(self: Vc) -> Vc { ModuleSideEffects::SideEffectful.cell() } } @@ -569,8 +568,8 @@ impl Module for ModuleWithoutSelfAsync { } #[turbo_tasks::function] - fn side_effects(&self, side_effect_free_packages: Vc) -> Vc { - self.module.side_effects(side_effect_free_packages) + fn side_effects(&self) -> Vc { + self.module.side_effects() } // Don't override and use default is_self_async that always returns false } diff --git a/turbopack/crates/turbopack-ecmascript/src/references/require_context.rs b/turbopack/crates/turbopack-ecmascript/src/references/require_context.rs index 98cf4b0224c3b..260128e83921e 100644 --- a/turbopack/crates/turbopack-ecmascript/src/references/require_context.rs +++ b/turbopack/crates/turbopack-ecmascript/src/references/require_context.rs @@ -20,7 +20,7 @@ use turbo_tasks::{ FxIndexMap, NonLocalValue, ResolvedVc, ValueToString, Vc, debug::ValueDebugFormat, trace::TraceRawVcs, }; -use turbo_tasks_fs::{DirectoryContent, DirectoryEntry, FileSystemPath, glob::Glob}; +use turbo_tasks_fs::{DirectoryContent, DirectoryEntry, FileSystemPath}; use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ @@ -445,7 +445,7 @@ impl Module for RequireContextAsset { } #[turbo_tasks::function] - fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + fn side_effects(self: Vc) -> Vc { ModuleSideEffects::SideEffectFree.cell() } } diff --git a/turbopack/crates/turbopack-ecmascript/src/side_effect_optimization/facade/module.rs b/turbopack/crates/turbopack-ecmascript/src/side_effect_optimization/facade/module.rs index 6146ac34e9fcd..5cd05226e16d0 100644 --- a/turbopack/crates/turbopack-ecmascript/src/side_effect_optimization/facade/module.rs +++ b/turbopack/crates/turbopack-ecmascript/src/side_effect_optimization/facade/module.rs @@ -2,7 +2,7 @@ use std::collections::BTreeMap; use anyhow::{Result, bail}; use turbo_tasks::{ResolvedVc, Vc}; -use turbo_tasks_fs::{File, FileContent, glob::Glob}; +use turbo_tasks_fs::{File, FileContent}; use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ @@ -182,9 +182,9 @@ impl Module for EcmascriptModuleFacadeModule { } #[turbo_tasks::function] - fn side_effects(&self, side_effect_free_packages: Vc) -> Result> { + fn side_effects(&self) -> Result> { Ok(match self.part { - ModulePart::Facade => self.module.side_effects(side_effect_free_packages), + ModulePart::Facade => self.module.side_effects(), ModulePart::RenamedExport { .. } | ModulePart::RenamedNamespace { .. } => { ModuleSideEffects::SideEffectFree.cell() } diff --git a/turbopack/crates/turbopack-ecmascript/src/side_effect_optimization/locals/module.rs b/turbopack/crates/turbopack-ecmascript/src/side_effect_optimization/locals/module.rs index 1b7a43fa273eb..d9c0f005429c0 100644 --- a/turbopack/crates/turbopack-ecmascript/src/side_effect_optimization/locals/module.rs +++ b/turbopack/crates/turbopack-ecmascript/src/side_effect_optimization/locals/module.rs @@ -2,7 +2,6 @@ use std::collections::BTreeMap; use anyhow::{Result, bail}; use turbo_tasks::{ResolvedVc, Vc}; -use turbo_tasks_fs::glob::Glob; use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ @@ -73,8 +72,8 @@ impl Module for EcmascriptModuleLocalsModule { } #[turbo_tasks::function] - fn side_effects(&self, side_effect_free_packages: Vc) -> Vc { - self.module.side_effects(side_effect_free_packages) + fn side_effects(&self) -> Vc { + self.module.side_effects() } } diff --git a/turbopack/crates/turbopack-ecmascript/src/tree_shake/asset.rs b/turbopack/crates/turbopack-ecmascript/src/tree_shake/asset.rs index 6aa862a5a7a26..2c1a21f2b1bde 100644 --- a/turbopack/crates/turbopack-ecmascript/src/tree_shake/asset.rs +++ b/turbopack/crates/turbopack-ecmascript/src/tree_shake/asset.rs @@ -1,16 +1,14 @@ use anyhow::Result; use turbo_rcstr::{RcStr, rcstr}; use turbo_tasks::{ResolvedVc, Vc}; -use turbo_tasks_fs::glob::Glob; use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{AsyncModuleInfo, ChunkableModule, ChunkingContext, EvaluatableAsset}, - context::AssetContext, ident::AssetIdent, module::{Module, ModuleSideEffects}, module_graph::ModuleGraph, reference::{ModuleReference, ModuleReferences, SingleChunkableModuleReference}, - resolve::{ExportUsage, ModulePart, origin::ResolveOrigin}, + resolve::{ExportUsage, ModulePart}, }; use super::{ @@ -175,17 +173,11 @@ impl EcmascriptModulePartAsset { ), )); } - let side_effect_free_packages = module.asset_context().side_effect_free_packages(); let source_module = Vc::upcast(module); let FollowExportsWithSideEffectsResult { side_effects, result, - } = &*follow_reexports_with_side_effects( - source_module, - export.clone(), - side_effect_free_packages, - ) - .await?; + } = &*follow_reexports_with_side_effects(source_module, export.clone()).await?; let FollowExportsResult { module: final_module, export_name: new_export, @@ -256,31 +248,20 @@ struct FollowExportsWithSideEffectsResult { async fn follow_reexports_with_side_effects( module: ResolvedVc>, export_name: RcStr, - side_effect_free_packages: Vc, ) -> Result> { let mut side_effects = vec![]; let mut current_module = module; let mut current_export_name = export_name; let result = loop { - let is_side_effect_free = *current_module - .side_effects(side_effect_free_packages) - .await? - == ModuleSideEffects::SideEffectFree; - - if !is_side_effect_free { + if *current_module.side_effects().await? != ModuleSideEffects::SideEffectFree { side_effects.push(only_effects(*current_module).to_resolved().await?); } // We ignore the side effect of the entry module here, because we need to proceed. - let result = follow_reexports( - *current_module, - current_export_name.clone(), - side_effect_free_packages, - true, - ) - .to_resolved() - .await?; + let result = follow_reexports(*current_module, current_export_name.clone(), true) + .to_resolved() + .await?; let FollowExportsResult { module, @@ -354,12 +335,12 @@ impl Module for EcmascriptModulePartAsset { } #[turbo_tasks::function] - async fn side_effects(&self, side_effect_free_packages: Vc) -> Vc { + async fn side_effects(&self) -> Vc { match self.part { ModulePart::Exports | ModulePart::Export(..) => { ModuleSideEffects::SideEffectFree.cell() } - _ => self.full_module.side_effects(side_effect_free_packages), + _ => self.full_module.side_effects(), } } } diff --git a/turbopack/crates/turbopack-ecmascript/src/tree_shake/side_effect_module.rs b/turbopack/crates/turbopack-ecmascript/src/tree_shake/side_effect_module.rs index 163acad58eefe..8b511fd8a658f 100644 --- a/turbopack/crates/turbopack-ecmascript/src/tree_shake/side_effect_module.rs +++ b/turbopack/crates/turbopack-ecmascript/src/tree_shake/side_effect_module.rs @@ -1,7 +1,7 @@ use anyhow::Result; use turbo_rcstr::{RcStr, rcstr}; use turbo_tasks::{ResolvedVc, TryJoinIterExt, Vc}; -use turbo_tasks_fs::{FileContent, glob::Glob}; +use turbo_tasks_fs::FileContent; use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ChunkableModule, ChunkingContext, EvaluatableAsset}, @@ -114,7 +114,7 @@ impl Module for SideEffectsModule { } #[turbo_tasks::function] - fn side_effects(self: Vc, _: Vc) -> Vc { + fn side_effects(self: Vc) -> Vc { ModuleSideEffects::SideEffectFree.cell() } } diff --git a/turbopack/crates/turbopack-ecmascript/src/typescript/mod.rs b/turbopack/crates/turbopack-ecmascript/src/typescript/mod.rs index c41260cf35994..392b63ef44778 100644 --- a/turbopack/crates/turbopack-ecmascript/src/typescript/mod.rs +++ b/turbopack/crates/turbopack-ecmascript/src/typescript/mod.rs @@ -2,7 +2,7 @@ use anyhow::Result; use serde_json::Value as JsonValue; use turbo_rcstr::{RcStr, rcstr}; use turbo_tasks::{ResolvedVc, TryJoinIterExt, ValueToString, Vc}; -use turbo_tasks_fs::{DirectoryContent, glob::Glob}; +use turbo_tasks_fs::DirectoryContent; use turbopack_core::{ asset::{Asset, AssetContent}, ident::AssetIdent, @@ -175,7 +175,7 @@ impl Module for TsConfigModuleAsset { } #[turbo_tasks::function] - fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + fn side_effects(self: Vc) -> Vc { ModuleSideEffects::SideEffectful.cell() } } diff --git a/turbopack/crates/turbopack-ecmascript/src/webpack/mod.rs b/turbopack/crates/turbopack-ecmascript/src/webpack/mod.rs index 7af6a12d12b8d..11c559a57d337 100644 --- a/turbopack/crates/turbopack-ecmascript/src/webpack/mod.rs +++ b/turbopack/crates/turbopack-ecmascript/src/webpack/mod.rs @@ -2,7 +2,6 @@ use anyhow::Result; use swc_core::ecma::ast::Lit; use turbo_rcstr::{RcStr, rcstr}; use turbo_tasks::{ResolvedVc, ValueToString, Vc}; -use turbo_tasks_fs::glob::Glob; use turbopack_core::{ asset::{Asset, AssetContent}, file_source::FileSource, @@ -65,7 +64,7 @@ impl Module for WebpackModuleAsset { } #[turbo_tasks::function] - fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + fn side_effects(self: Vc) -> Vc { ModuleSideEffects::SideEffectful.cell() } } diff --git a/turbopack/crates/turbopack-ecmascript/src/worker_chunk/module.rs b/turbopack/crates/turbopack-ecmascript/src/worker_chunk/module.rs index 6830bd134dc5e..825a29bf2551f 100644 --- a/turbopack/crates/turbopack-ecmascript/src/worker_chunk/module.rs +++ b/turbopack/crates/turbopack-ecmascript/src/worker_chunk/module.rs @@ -1,7 +1,6 @@ use anyhow::Result; use turbo_rcstr::{RcStr, rcstr}; use turbo_tasks::{ResolvedVc, ValueToString, Vc}; -use turbo_tasks_fs::glob::Glob; use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ @@ -59,7 +58,7 @@ impl Module for WorkerLoaderModule { } #[turbo_tasks::function] - fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + fn side_effects(self: Vc) -> Vc { ModuleSideEffects::SideEffectFree.cell() } } diff --git a/turbopack/crates/turbopack-json/src/lib.rs b/turbopack/crates/turbopack-json/src/lib.rs index 40a3246601ee8..86643337737e1 100644 --- a/turbopack/crates/turbopack-json/src/lib.rs +++ b/turbopack/crates/turbopack-json/src/lib.rs @@ -14,7 +14,7 @@ use std::fmt::Write; use anyhow::{Error, Result, bail}; use turbo_rcstr::rcstr; use turbo_tasks::{ResolvedVc, ValueToString, Vc}; -use turbo_tasks_fs::{FileContent, FileJsonContent, glob::Glob}; +use turbo_tasks_fs::{FileContent, FileJsonContent}; use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ChunkItem, ChunkType, ChunkableModule, ChunkingContext}, @@ -59,7 +59,7 @@ impl Module for JsonModuleAsset { } #[turbo_tasks::function] - fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + fn side_effects(self: Vc) -> Vc { ModuleSideEffects::SideEffectFree.cell() } } diff --git a/turbopack/crates/turbopack-static/src/css.rs b/turbopack/crates/turbopack-static/src/css.rs index 3647e3f1f3591..66a6c0ac0fc47 100644 --- a/turbopack/crates/turbopack-static/src/css.rs +++ b/turbopack/crates/turbopack-static/src/css.rs @@ -1,6 +1,5 @@ use turbo_rcstr::{RcStr, rcstr}; use turbo_tasks::{ResolvedVc, Vc}; -use turbo_tasks_fs::glob::Glob; use turbopack_core::{ asset::{Asset, AssetContent}, chunk::ChunkingContext, @@ -53,7 +52,8 @@ impl Module for StaticUrlCssModule { } #[turbo_tasks::function] - fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + + fn side_effects(self: Vc) -> Vc { ModuleSideEffects::SideEffectFree.cell() } } diff --git a/turbopack/crates/turbopack-static/src/ecma.rs b/turbopack/crates/turbopack-static/src/ecma.rs index 0feafc8e6396a..d867459a6887a 100644 --- a/turbopack/crates/turbopack-static/src/ecma.rs +++ b/turbopack/crates/turbopack-static/src/ecma.rs @@ -1,7 +1,6 @@ use anyhow::Result; use turbo_rcstr::{RcStr, rcstr}; use turbo_tasks::{ResolvedVc, Vc}; -use turbo_tasks_fs::glob::Glob; use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ChunkItem, ChunkType, ChunkableModule, ChunkingContext}, @@ -65,7 +64,7 @@ impl Module for StaticUrlJsModule { } #[turbo_tasks::function] - fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + fn side_effects(self: Vc) -> Vc { ModuleSideEffects::SideEffectFree.cell() } } diff --git a/turbopack/crates/turbopack-test-utils/src/noop_asset_context.rs b/turbopack/crates/turbopack-test-utils/src/noop_asset_context.rs index 1602c0df8c213..89bc7a43901e2 100644 --- a/turbopack/crates/turbopack-test-utils/src/noop_asset_context.rs +++ b/turbopack/crates/turbopack-test-utils/src/noop_asset_context.rs @@ -1,7 +1,7 @@ use anyhow::Result; -use turbo_rcstr::{RcStr, rcstr}; +use turbo_rcstr::RcStr; use turbo_tasks::{ResolvedVc, Vc}; -use turbo_tasks_fs::{FileSystemPath, glob::Glob}; +use turbo_tasks_fs::FileSystemPath; use turbopack_core::{ compile_time_info::CompileTimeInfo, context::{AssetContext, ProcessResult}, @@ -73,9 +73,4 @@ impl AssetContext for NoopAssetContext { ) -> Result>> { Ok(Vc::upcast(self)) } - - #[turbo_tasks::function] - async fn side_effect_free_packages(&self) -> Result> { - Ok(Glob::new(rcstr!(""), Default::default())) - } } diff --git a/turbopack/crates/turbopack-wasm/src/module_asset.rs b/turbopack/crates/turbopack-wasm/src/module_asset.rs index 5f8ba8407b701..0affe4b5f55ec 100644 --- a/turbopack/crates/turbopack-wasm/src/module_asset.rs +++ b/turbopack/crates/turbopack-wasm/src/module_asset.rs @@ -1,7 +1,7 @@ use anyhow::{Context, Result, bail}; use turbo_rcstr::rcstr; use turbo_tasks::{IntoTraitRef, ResolvedVc, Vc, fxindexmap}; -use turbo_tasks_fs::{FileSystemPath, glob::Glob}; +use turbo_tasks_fs::FileSystemPath; use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ @@ -146,7 +146,7 @@ impl Module for WebAssemblyModuleAsset { } #[turbo_tasks::function] - fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + fn side_effects(self: Vc) -> Vc { // Both versions of this module have a top level await that instantiates a wasm module // wasm module instantiation can trigger arbitrary side effects from the native start // function diff --git a/turbopack/crates/turbopack-wasm/src/raw.rs b/turbopack/crates/turbopack-wasm/src/raw.rs index 12f1495944b50..1d2fa1ab9bad7 100644 --- a/turbopack/crates/turbopack-wasm/src/raw.rs +++ b/turbopack/crates/turbopack-wasm/src/raw.rs @@ -1,7 +1,6 @@ use anyhow::{Result, bail}; use turbo_rcstr::rcstr; use turbo_tasks::{IntoTraitRef, ResolvedVc, Vc}; -use turbo_tasks_fs::glob::Glob; use turbopack_core::{ asset::{Asset, AssetContent}, chunk::{ChunkItem, ChunkType, ChunkableModule, ChunkingContext}, @@ -67,7 +66,7 @@ impl Module for RawWebAssemblyModuleAsset { } #[turbo_tasks::function] - fn side_effects(self: Vc, _side_effect_free_packages: Vc) -> Vc { + fn side_effects(self: Vc) -> Vc { // this just exports a path ModuleSideEffects::SideEffectFree.cell() } diff --git a/turbopack/crates/turbopack/src/lib.rs b/turbopack/crates/turbopack/src/lib.rs index 924f34e58a3d6..1cfe18fc6f6f8 100644 --- a/turbopack/crates/turbopack/src/lib.rs +++ b/turbopack/crates/turbopack/src/lib.rs @@ -17,10 +17,8 @@ use module_options::{ModuleOptions, ModuleOptionsContext, ModuleRuleEffect, Modu use tracing::{Instrument, field::Empty}; use turbo_rcstr::{RcStr, rcstr}; use turbo_tasks::{ResolvedVc, TryJoinIterExt, ValueToString, Vc}; -use turbo_tasks_fs::{ - FileSystemPath, - glob::{Glob, GlobOptions}, -}; +use turbo_tasks_fs::FileSystemPath; +pub use turbopack_core::condition; use turbopack_core::{ asset::Asset, chunk::SourceMapsType, @@ -127,6 +125,10 @@ async fn apply_module_type( } .to_resolved() .await?; + let side_effect_free_packages = module_asset_context + .module_options_context() + .await? + .side_effect_free_packages; let mut builder = EcmascriptModuleAsset::builder( source, ResolvedVc::upcast(context_for_module), @@ -140,6 +142,7 @@ async fn apply_module_type( .compile_time_info() .to_resolved() .await?, + side_effect_free_packages, ); match module_type { ModuleType::Ecmascript { .. } => { @@ -177,14 +180,7 @@ async fn apply_module_type( if tree_shaking_mode.is_some() && is_evaluation { // If we are tree shaking, skip the evaluation part if the module is marked as // side effect free. - let side_effect_free_packages = module_asset_context - .side_effect_free_packages() - .resolve() - .await?; - - if *module.side_effects(side_effect_free_packages).await? - == ModuleSideEffects::SideEffectFree - { + if *module.side_effects().await? == ModuleSideEffects::SideEffectFree { return Ok(ProcessResult::Ignore.cell()); } } @@ -207,11 +203,6 @@ async fn apply_module_type( } } ModulePart::Export(_) => { - let side_effect_free_packages = module_asset_context - .side_effect_free_packages() - .resolve() - .await?; - if *module.get_exports().split_locals_and_reexports().await? { apply_reexport_tree_shaking( Vc::upcast( @@ -223,16 +214,11 @@ async fn apply_module_type( .await?, ), part, - side_effect_free_packages, ) .await? } else { - apply_reexport_tree_shaking( - Vc::upcast(*module), - part, - side_effect_free_packages, - ) - .await? + apply_reexport_tree_shaking(Vc::upcast(*module), part) + .await? } } _ => bail!( @@ -310,14 +296,8 @@ async fn apply_module_type( if tree_shaking_mode.is_some() && is_evaluation { // If we are tree shaking, skip the evaluation part if the module is marked as // side effect free. - let side_effect_free_packages = module_asset_context - .side_effect_free_packages() - .resolve() - .await?; - if *module.side_effects(side_effect_free_packages).await? - == ModuleSideEffects::SideEffectFree - { + if *module.side_effects().await? == ModuleSideEffects::SideEffectFree { return Ok(ProcessResult::Ignore.cell()); } } @@ -328,14 +308,13 @@ async fn apply_module_type( async fn apply_reexport_tree_shaking( module: Vc>, part: ModulePart, - side_effect_free_packages: Vc, ) -> Result>> { if let ModulePart::Export(export) = &part { let FollowExportsResult { module: final_module, export_name: new_export, .. - } = &*follow_reexports(module, export.clone(), side_effect_free_packages, true).await?; + } = &*follow_reexports(module, export.clone(), true).await?; let module = if let Some(new_export) = new_export { if *new_export == *export { Vc::upcast(**final_module) @@ -1018,18 +997,6 @@ impl AssetContext for ModuleAssetContext { }, ) } - - #[turbo_tasks::function] - async fn side_effect_free_packages(&self) -> Result> { - let pkgs = &self.module_options_context.await?.side_effect_free_packages; - - let mut globs = String::new(); - globs.push_str("**/node_modules/{"); - globs.push_str(&pkgs.join(",")); - globs.push_str("}/**"); - - Ok(Glob::new(globs.into(), GlobOptions::default())) - } } #[turbo_tasks::function] diff --git a/turbopack/crates/turbopack/src/module_options/module_options_context.rs b/turbopack/crates/turbopack/src/module_options/module_options_context.rs index 4e0385628793c..f61bfe21a13cb 100644 --- a/turbopack/crates/turbopack/src/module_options/module_options_context.rs +++ b/turbopack/crates/turbopack/src/module_options/module_options_context.rs @@ -1,11 +1,15 @@ use std::fmt::Debug; +use anyhow::Result; use bincode::{Decode, Encode}; use serde::{Deserialize, Serialize}; use turbo_esregex::EsRegex; -use turbo_rcstr::RcStr; +use turbo_rcstr::{RcStr, rcstr}; use turbo_tasks::{NonLocalValue, ResolvedVc, ValueDefault, Vc, trace::TraceRawVcs}; -use turbo_tasks_fs::FileSystemPath; +use turbo_tasks_fs::{ + FileSystemPath, + glob::{Glob, GlobOptions}, +}; use turbopack_core::{ chunk::SourceMapsType, compile_time_info::CompileTimeInfo, condition::ContextCondition, environment::Environment, resolve::options::ImportMapping, @@ -182,7 +186,6 @@ pub struct ExternalsTracingOptions { #[turbo_tasks::value(shared)] #[derive(Clone, Default)] -#[serde(default)] pub struct ModuleOptionsContext { pub ecmascript: EcmascriptOptionsContext, pub css: CssOptionsContext, @@ -196,7 +199,7 @@ pub struct ModuleOptionsContext { pub environment: Option>, pub execution_context: Option>, - pub side_effect_free_packages: Vec, + pub side_effect_free_packages: Option>, pub tree_shaking_mode: Option, /// Generate (non-emitted) output assets for static assets and externals, to facilitate @@ -284,3 +287,20 @@ impl ValueDefault for ModuleOptionsContext { Self::cell(Default::default()) } } + +#[turbo_tasks::function] +pub async fn side_effect_free_packages_glob( + side_effect_free_packages: ResolvedVc>, +) -> Result> { + let side_effect_free_packages = &*side_effect_free_packages.await?; + if side_effect_free_packages.is_empty() { + return Ok(Glob::new(rcstr!(""), GlobOptions::default())); + } + + let mut globs = String::new(); + globs.push_str("**/node_modules/{"); + globs.push_str(&side_effect_free_packages.join(",")); + globs.push_str("}/**"); + + Ok(Glob::new(globs.into(), GlobOptions::default())) +} From 35d58e21ee3b63773c1a395079edcf0a8005ba2a Mon Sep 17 00:00:00 2001 From: Luke Sandberg Date: Sun, 30 Nov 2025 14:23:42 -0800 Subject: [PATCH 3/8] fix a benchmark --- .../crates/turbopack-ecmascript/benches/references.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/turbopack/crates/turbopack-ecmascript/benches/references.rs b/turbopack/crates/turbopack-ecmascript/benches/references.rs index 32cf899317210..b65c7b5c8ca7c 100644 --- a/turbopack/crates/turbopack-ecmascript/benches/references.rs +++ b/turbopack/crates/turbopack-ecmascript/benches/references.rs @@ -5,10 +5,7 @@ use criterion::{BatchSize, Bencher, BenchmarkId, Criterion, criterion_group, cri use turbo_rcstr::rcstr; use turbo_tasks::{ResolvedVc, TurboTasks}; use turbo_tasks_backend::{BackendOptions, TurboTasksBackend, noop_backing_storage}; -use turbo_tasks_fs::{ - DiskFileSystem, FileSystem, - glob::{Glob, GlobOptions}, -}; +use turbo_tasks_fs::{DiskFileSystem, FileSystem}; use turbopack_core::{ compile_time_info::CompileTimeInfo, environment::{Environment, ExecutionEnvironment, NodeJsEnvironment}, @@ -80,9 +77,7 @@ async fn setup( layer, } .resolved_cell(); - let side_effect_free_packages = Glob::new("".into(), GlobOptions::default()) - .to_resolved() - .await?; + let module = EcmascriptModuleAsset::builder( ResolvedVc::upcast( FileSource::new(fs.root().await?.join(file).unwrap()) @@ -102,7 +97,7 @@ async fn setup( } .resolved_cell(), compile_time_info, - Some(side_effect_free_packages), + None, ) .build() .to_resolved() From 2431930aeede7ae567971e7c010d80780097b281 Mon Sep 17 00:00:00 2001 From: Luke Sandberg Date: Sun, 30 Nov 2025 19:10:17 -0800 Subject: [PATCH 4/8] integrate into binding_usage_info --- .../src/module_graph/binding_usage_info.rs | 38 ++++-- .../module_graph/side_effect_module_info.rs | 119 +++++++++++------- 2 files changed, 104 insertions(+), 53 deletions(-) diff --git a/turbopack/crates/turbopack-core/src/module_graph/binding_usage_info.rs b/turbopack/crates/turbopack-core/src/module_graph/binding_usage_info.rs index 4d745611e3db2..f2246787d7a56 100644 --- a/turbopack/crates/turbopack-core/src/module_graph/binding_usage_info.rs +++ b/turbopack/crates/turbopack-core/src/module_graph/binding_usage_info.rs @@ -9,7 +9,10 @@ use turbo_tasks::{ResolvedVc, Vc}; use crate::{ module::Module, - module_graph::{GraphEdgeIndex, GraphTraversalAction, ModuleGraph}, + module_graph::{ + GraphEdgeIndex, GraphTraversalAction, ModuleGraph, + side_effect_module_info::compute_side_effect_free_module_info, + }, reference::ModuleReference, resolve::{ExportUsage, ImportUsage}, }; @@ -121,6 +124,7 @@ pub async fn compute_binding_usage_info( without_unused_references" ); } + let side_effect_free_modules = compute_side_effect_free_module_info(*graph).await?; let graph = graph.read_graphs().await?; @@ -147,6 +151,7 @@ pub async fn compute_binding_usage_info( .iter() .all(|e| !source_used_exports.is_export_used(e)) { + // all exports are unused #[cfg(debug_assertions)] debug_unused_references_name.insert(( parent, @@ -170,15 +175,28 @@ pub async fn compute_binding_usage_info( } } ImportUsage::SideEffects => { - #[cfg(debug_assertions)] - debug_unused_references_name.remove(&( - parent, - ref_data.binding_usage.export.clone(), - target, - )); - unused_references_edges.remove(&edge); - unused_references.remove(&ref_data.reference); - // Continue, has to always be included + if side_effect_free_modules.contains(&target) { + #[cfg(debug_assertions)] + debug_unused_references_name.insert(( + parent, + ref_data.binding_usage.export.clone(), + target, + )); + unused_references_edges.insert(edge); + unused_references.insert(ref_data.reference); + + return Ok(GraphTraversalAction::Skip); + } else { + #[cfg(debug_assertions)] + debug_unused_references_name.remove(&( + parent, + ref_data.binding_usage.export.clone(), + target, + )); + unused_references_edges.remove(&edge); + unused_references.remove(&ref_data.reference); + // Continue, has to always be included + } } } } diff --git a/turbopack/crates/turbopack-core/src/module_graph/side_effect_module_info.rs b/turbopack/crates/turbopack-core/src/module_graph/side_effect_module_info.rs index f396e1dfa659b..d23a7c255eeae 100644 --- a/turbopack/crates/turbopack-core/src/module_graph/side_effect_module_info.rs +++ b/turbopack/crates/turbopack-core/src/module_graph/side_effect_module_info.rs @@ -1,9 +1,9 @@ use anyhow::Result; -use rustc_hash::FxHashSet; +use rustc_hash::{FxHashMap, FxHashSet}; use turbo_tasks::{ResolvedVc, TryJoinIterExt, Vc}; use crate::{ - module::Module, + module::{Module, ModuleSideEffects}, module_graph::{GraphTraversalAction, ModuleGraph, SingleModuleGraphWithBindingUsage}, }; @@ -13,7 +13,7 @@ use crate::{ #[turbo_tasks::value(transparent)] pub struct SideEffectFreeModules(FxHashSet>>); -#[turbo_tasks::function(operation)] +#[turbo_tasks::function] pub async fn compute_side_effect_free_module_info( graphs: ResolvedVc, ) -> Result> { @@ -32,56 +32,89 @@ async fn compute_side_effect_free_module_info_single( graph: SingleModuleGraphWithBindingUsage, parent_side_effect_free_modules: Vc, ) -> Result> { - let parent_async_modules = parent_side_effect_free_modules.await?; + let parent_side_effect_free_modules = parent_side_effect_free_modules.await?; let graph = graph.read().await?; - let self_async_modules = graph - .graphs - .first() - .unwrap() - .iter_nodes() - .map(async |node| Ok((node, *node.is_self_async().await?))) + let module_side_effects = graph + .enumerate_nodes() + .map(async |(_, node)| { + Ok(match node { + super::SingleModuleGraphNode::Module(module) => { + // This turbo task always has a cache hit since it is called when building the + // module graph. we could consider moving this information + // into to the module graph, but then changes would invalidate the whole graph. + (*module, *module.side_effects().await?) + } + super::SingleModuleGraphNode::VisitedModule { idx: _, module } => ( + *module, + if parent_side_effect_free_modules.contains(module) { + ModuleSideEffects::DeclaredSideEffectFree + } else { + ModuleSideEffects::SideEffectful + }, + ), + }) + }) .try_join() .await? .into_iter() - .flat_map(|(k, v)| v.then_some(k)) - .chain(parent_async_modules.iter().copied()) - .collect::>(); + .collect::>(); - // To determine which modules are async, we need to propagate the self-async flag to all - // importers, which is done using a postorder traversal of the graph. - // - // This however doesn't cover cycles of async modules, which are handled by determining all - // strongly-connected components, and then marking all the whole SCC as async if one of the - // modules in the SCC is async. + // Modules are categorized as side-effectful, locally side effect free and side effect free. + // So we are really just interested in determining what modules that are locally side effect + // free. logically we want to start at all such modules are determine if their transitive + // dependencies are side effect free. - let mut async_modules = self_async_modules; - graph.traverse_edges_from_entries_dfs( - graph.graphs.first().unwrap().entry_modules(), - &mut (), - |_, _, _| Ok(GraphTraversalAction::Continue), - |parent_info, module, _| { - let Some((parent_module, ref_data)) = parent_info else { - // An entry module - return Ok(()); - }; - - if ref_data.chunking_type.is_inherit_async() && async_modules.contains(&module) { - async_modules.insert(parent_module); + let mut locally_side_effect_free_modules_that_have_side_effects = FxHashSet::default(); + graph.traverse_edges_from_entries_dfs_reversed( + // Start from all the side effectful nodes + module_side_effects.iter().filter_map(|(m, e)| { + if *e == ModuleSideEffects::SideEffectful { + Some(*m) + } else { + None } - Ok(()) + }), + &mut (), + // child is a previously visited module that we know is side effectful + |child, _parent, _s| { + Ok(if let Some((child_module, _edge)) = child { + match module_side_effects.get(&child_module).unwrap() { + ModuleSideEffects::SideEffectful + | ModuleSideEffects::DeclaredSideEffectFree => { + // We have either already seen this or don't want to follow it + GraphTraversalAction::Exclude + } + ModuleSideEffects::ModuleEvaluationIsSideEffectFree => { + // this module is side effect free locally but must depend on something + // effectful so it to is effectful + locally_side_effect_free_modules_that_have_side_effects + .insert(child_module); + GraphTraversalAction::Continue + } + } + } else { + // entry point, keep going + GraphTraversalAction::Continue + }) }, + |_, _, _| Ok(()), )?; - - graph.traverse_cycles( - |ref_data| ref_data.chunking_type.is_inherit_async(), - |cycle| { - if cycle.iter().any(|node| async_modules.contains(node)) { - async_modules.extend(cycle.iter().map(|n| **n)); + let side_effect_free_modules = module_side_effects + .into_iter() + .filter_map(|(m, e)| match e { + ModuleSideEffects::SideEffectful => None, + ModuleSideEffects::DeclaredSideEffectFree => Some(m), + ModuleSideEffects::ModuleEvaluationIsSideEffectFree => { + if locally_side_effect_free_modules_that_have_side_effects.contains(&m) { + None + } else { + Some(m) + } } - Ok(()) - }, - )?; + }) + .chain(parent_side_effect_free_modules.iter().copied()) + .collect::>(); - Ok(Vc::cell(async_modules)) + Ok(Vc::cell(side_effect_free_modules)) } From 3ea35f90ec282579a184f0cf9ea021fa19cbbd83 Mon Sep 17 00:00:00 2001 From: Luke Sandberg Date: Mon, 1 Dec 2025 13:36:15 -0800 Subject: [PATCH 5/8] fix a name --- .../src/module_graph/side_effect_module_info.rs | 7 +++---- turbopack/crates/turbopack/src/lib.rs | 1 - 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/turbopack/crates/turbopack-core/src/module_graph/side_effect_module_info.rs b/turbopack/crates/turbopack-core/src/module_graph/side_effect_module_info.rs index d23a7c255eeae..55b11c270c4e5 100644 --- a/turbopack/crates/turbopack-core/src/module_graph/side_effect_module_info.rs +++ b/turbopack/crates/turbopack-core/src/module_graph/side_effect_module_info.rs @@ -48,7 +48,7 @@ async fn compute_side_effect_free_module_info_single( super::SingleModuleGraphNode::VisitedModule { idx: _, module } => ( *module, if parent_side_effect_free_modules.contains(module) { - ModuleSideEffects::DeclaredSideEffectFree + ModuleSideEffects::SideEffectFree } else { ModuleSideEffects::SideEffectful }, @@ -80,8 +80,7 @@ async fn compute_side_effect_free_module_info_single( |child, _parent, _s| { Ok(if let Some((child_module, _edge)) = child { match module_side_effects.get(&child_module).unwrap() { - ModuleSideEffects::SideEffectful - | ModuleSideEffects::DeclaredSideEffectFree => { + ModuleSideEffects::SideEffectful | ModuleSideEffects::SideEffectFree => { // We have either already seen this or don't want to follow it GraphTraversalAction::Exclude } @@ -104,7 +103,7 @@ async fn compute_side_effect_free_module_info_single( .into_iter() .filter_map(|(m, e)| match e { ModuleSideEffects::SideEffectful => None, - ModuleSideEffects::DeclaredSideEffectFree => Some(m), + ModuleSideEffects::SideEffectFree => Some(m), ModuleSideEffects::ModuleEvaluationIsSideEffectFree => { if locally_side_effect_free_modules_that_have_side_effects.contains(&m) { None diff --git a/turbopack/crates/turbopack/src/lib.rs b/turbopack/crates/turbopack/src/lib.rs index 1cfe18fc6f6f8..20891acf86e41 100644 --- a/turbopack/crates/turbopack/src/lib.rs +++ b/turbopack/crates/turbopack/src/lib.rs @@ -296,7 +296,6 @@ async fn apply_module_type( if tree_shaking_mode.is_some() && is_evaluation { // If we are tree shaking, skip the evaluation part if the module is marked as // side effect free. - if *module.side_effects().await? == ModuleSideEffects::SideEffectFree { return Ok(ProcessResult::Ignore.cell()); } From cb5615ff799d2d3f6aebe50797708231c437fe62 Mon Sep 17 00:00:00 2001 From: Luke Sandberg Date: Wed, 3 Dec 2025 07:01:39 -0800 Subject: [PATCH 6/8] Update raw_ecmascript_module.rs --- crates/next-core/src/raw_ecmascript_module.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/next-core/src/raw_ecmascript_module.rs b/crates/next-core/src/raw_ecmascript_module.rs index c58cbe07638e9..3d67d9fea8540 100644 --- a/crates/next-core/src/raw_ecmascript_module.rs +++ b/crates/next-core/src/raw_ecmascript_module.rs @@ -100,8 +100,7 @@ impl Module for RawEcmascriptModule { #[turbo_tasks::function] fn side_effects(self: Vc) -> Vc { - // Is this correct? - ModuleSideEffects::SideEffectFree.cell() + ModuleSideEffects::SideEffectful.cell() } } From ace34ee915307d82b2b9c3b6b0c8b38a9578c71e Mon Sep 17 00:00:00 2001 From: Luke Sandberg Date: Sun, 7 Dec 2025 09:29:04 -0800 Subject: [PATCH 7/8] review comments --- .../ecmascript_client_reference_module.rs | 5 +++-- .../module_graph/side_effect_module_info.rs | 16 +++++++-------- .../src/references/external_module.rs | 20 +++++++++---------- 3 files changed, 21 insertions(+), 20 deletions(-) diff --git a/crates/next-core/src/next_client_reference/ecmascript_client_reference/ecmascript_client_reference_module.rs b/crates/next-core/src/next_client_reference/ecmascript_client_reference/ecmascript_client_reference_module.rs index e63def3fd0e9a..8afd9b5af6729 100644 --- a/crates/next-core/src/next_client_reference/ecmascript_client_reference/ecmascript_client_reference_module.rs +++ b/crates/next-core/src/next_client_reference/ecmascript_client_reference/ecmascript_client_reference_module.rs @@ -243,8 +243,9 @@ impl Module for EcmascriptClientReferenceModule { } #[turbo_tasks::function] fn side_effects(self: Vc) -> Vc { - // These just export some specially tagged functions - ModuleSideEffects::SideEffectFree.cell() + // These just re-export some specially tagged functions, however we do assume that client + // references are executed client side so we need to preserve these in the graph. + ModuleSideEffects::SideEffectful.cell() } } diff --git a/turbopack/crates/turbopack-core/src/module_graph/side_effect_module_info.rs b/turbopack/crates/turbopack-core/src/module_graph/side_effect_module_info.rs index 55b11c270c4e5..52ef8cb061ef6 100644 --- a/turbopack/crates/turbopack-core/src/module_graph/side_effect_module_info.rs +++ b/turbopack/crates/turbopack-core/src/module_graph/side_effect_module_info.rs @@ -77,23 +77,23 @@ async fn compute_side_effect_free_module_info_single( }), &mut (), // child is a previously visited module that we know is side effectful - |child, _parent, _s| { - Ok(if let Some((child_module, _edge)) = child { - match module_side_effects.get(&child_module).unwrap() { + // parent is a module that depends on it. + |child, parent, _s| { + Ok(if child.is_some() { + match module_side_effects.get(&parent).unwrap() { ModuleSideEffects::SideEffectful | ModuleSideEffects::SideEffectFree => { // We have either already seen this or don't want to follow it GraphTraversalAction::Exclude } ModuleSideEffects::ModuleEvaluationIsSideEffectFree => { - // this module is side effect free locally but must depend on something - // effectful so it to is effectful - locally_side_effect_free_modules_that_have_side_effects - .insert(child_module); + // this module is side effect free locally but depends on `child` which is + // effectful so it too is effectful + locally_side_effect_free_modules_that_have_side_effects.insert(parent); GraphTraversalAction::Continue } } } else { - // entry point, keep going + // entry point, we already determined it was effectful, keep going GraphTraversalAction::Continue }) }, diff --git a/turbopack/crates/turbopack-ecmascript/src/references/external_module.rs b/turbopack/crates/turbopack-ecmascript/src/references/external_module.rs index 2f52073fd2bd8..d3e300476d3f9 100644 --- a/turbopack/crates/turbopack-ecmascript/src/references/external_module.rs +++ b/turbopack/crates/turbopack-ecmascript/src/references/external_module.rs @@ -359,7 +359,7 @@ impl Module for CachedExternalModule { // == false`. Optimize this case by using `ModuleWithoutSelfAsync` to // short circuit that computation and thus defer parsing traced modules // to emitting to not block all of chunking on this. - .map(|m| Vc::upcast(ModuleWithoutSelfAsync::new(*m))), + .map(|m| Vc::upcast(SideEffectfulModuleWithoutSelfAsync::new(*m))), ) .map(|s| { Vc::upcast::>(TracedModuleReference::new(s)) @@ -527,23 +527,23 @@ impl EcmascriptChunkItem for CachedExternalModuleChunkItem { } } -/// A wrapper "passthrough" module type that always returns `false` for `is_self_async`. Be careful -/// when using it, as it may hide async dependencies. +/// A wrapper "passthrough" module type that always returns `false` for `is_self_async` and +/// `SideEffects` for `side_effects`.Be careful when using it, as it may hide async dependencies. #[turbo_tasks::value] -pub struct ModuleWithoutSelfAsync { +struct SideEffectfulModuleWithoutSelfAsync { module: ResolvedVc>, } #[turbo_tasks::value_impl] -impl ModuleWithoutSelfAsync { +impl SideEffectfulModuleWithoutSelfAsync { #[turbo_tasks::function] - pub fn new(module: ResolvedVc>) -> Vc { - Self::cell(ModuleWithoutSelfAsync { module }) + fn new(module: ResolvedVc>) -> Vc { + Self::cell(SideEffectfulModuleWithoutSelfAsync { module }) } } #[turbo_tasks::value_impl] -impl Asset for ModuleWithoutSelfAsync { +impl Asset for SideEffectfulModuleWithoutSelfAsync { #[turbo_tasks::function] fn content(&self) -> Vc { self.module.content() @@ -551,7 +551,7 @@ impl Asset for ModuleWithoutSelfAsync { } #[turbo_tasks::value_impl] -impl Module for ModuleWithoutSelfAsync { +impl Module for SideEffectfulModuleWithoutSelfAsync { #[turbo_tasks::function] fn ident(&self) -> Vc { self.module.ident() @@ -569,7 +569,7 @@ impl Module for ModuleWithoutSelfAsync { #[turbo_tasks::function] fn side_effects(&self) -> Vc { - self.module.side_effects() + ModuleSideEffects::SideEffectful.cell() } // Don't override and use default is_self_async that always returns false } From 92cb1419a02a6975c85a8b3c3b91f3cca47dd366 Mon Sep 17 00:00:00 2001 From: Luke Sandberg Date: Sun, 7 Dec 2025 09:36:20 -0800 Subject: [PATCH 8/8] fix a bug in the SideEffectsModule --- .../turbopack-ecmascript/src/tree_shake/side_effect_module.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/turbopack/crates/turbopack-ecmascript/src/tree_shake/side_effect_module.rs b/turbopack/crates/turbopack-ecmascript/src/tree_shake/side_effect_module.rs index 8b511fd8a658f..f3ba0323e7472 100644 --- a/turbopack/crates/turbopack-ecmascript/src/tree_shake/side_effect_module.rs +++ b/turbopack/crates/turbopack-ecmascript/src/tree_shake/side_effect_module.rs @@ -115,7 +115,7 @@ impl Module for SideEffectsModule { #[turbo_tasks::function] fn side_effects(self: Vc) -> Vc { - ModuleSideEffects::SideEffectFree.cell() + ModuleSideEffects::SideEffectful.cell() } }