Skip to content

Turbopack: fix duplicate externals modules #81306

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 9, 2025
+94 −97
Merged

Turbopack: fix duplicate externals modules #81306

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
47f5177
Turbopack: fix duplicate externals module
mischnic Jul 4, 2025
ab645de
fixup
mischnic Jul 7, 2025
d30454b
cleanup
mischnic Jul 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions turbopack/crates/turbopack-core/src/resolve/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ pub enum ModuleResolveResultItem {
/// uri, path, reference, etc.
name: RcStr,
ty: ExternalType,
traced: Option<ResolvedVc<ModuleResolveResult>>,
},
/// A module could not be created (according to the rules, e.g. no module type as assigned)
Unknown(ResolvedVc<Box<dyn Source>>),
Expand Down Expand Up @@ -727,11 +726,7 @@ impl ResolveResult {
// Should use map_primary_items instead
bail!("map_module doesn't handle traced externals");
}
ModuleResolveResultItem::External {
name,
ty,
traced: None,
}
ModuleResolveResultItem::External { name, ty }
}
ResolveResultItem::Ignore => ModuleResolveResultItem::Ignore,
ResolveResultItem::Empty => ModuleResolveResultItem::Empty,
Expand Down
77 changes: 67 additions & 10 deletions turbopack/crates/turbopack-ecmascript/src/references/external_module.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,21 @@ use std::{fmt::Display, io::Write};
use anyhow::Result;
use serde::{Deserialize, Serialize};
use turbo_rcstr::{RcStr, rcstr};
use turbo_tasks::{NonLocalValue, ResolvedVc, TaskInput, Vc, trace::TraceRawVcs};
use turbo_tasks_fs::{FileContent, FileSystem, VirtualFileSystem, glob::Glob, rope::RopeBuilder};
use turbo_tasks::{NonLocalValue, ResolvedVc, TaskInput, TryJoinIterExt, Vc, trace::TraceRawVcs};
use turbo_tasks_fs::{
FileContent, FileSystem, FileSystemPath, VirtualFileSystem, glob::Glob, rope::RopeBuilder,
};
use turbopack_core::{
asset::{Asset, AssetContent},
chunk::{AsyncModuleInfo, ChunkItem, ChunkType, ChunkableModule, ChunkingContext},
context::AssetContext,
ident::{AssetIdent, Layer},
module::Module,
module_graph::ModuleGraph,
reference::{ModuleReference, ModuleReferences},
raw_module::RawModule,
reference::{ModuleReference, ModuleReferences, TracedModuleReference},
reference_type::ReferenceType,
resolve::parse::Request,
};

use crate::{
Expand Down Expand Up @@ -49,6 +55,19 @@ pub enum CachedExternalType {
Script,
}

#[derive(
Clone, Debug, Eq, PartialEq, Serialize, Deserialize, TraceRawVcs, TaskInput, Hash, NonLocalValue,
)]
/// Whether to add a traced reference to the external module using the given context and resolve
/// origin.
pub enum CachedExternalTracingMode {
Untraced,
Traced {
externals_context: ResolvedVc<Box<dyn AssetContext>>,
root_origin: FileSystemPath,
},
}

impl Display for CachedExternalType {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Expand All @@ -63,9 +82,9 @@ impl Display for CachedExternalType {

#[turbo_tasks::value]
pub struct CachedExternalModule {
pub request: RcStr,
pub external_type: CachedExternalType,
pub additional_references: Vec<ResolvedVc<Box<dyn ModuleReference>>>,
request: RcStr,
external_type: CachedExternalType,
tracing_mode: CachedExternalTracingMode,
}

#[turbo_tasks::value_impl]
Expand All @@ -74,12 +93,12 @@ impl CachedExternalModule {
pub fn new(
request: RcStr,
external_type: CachedExternalType,
additional_references: Vec<ResolvedVc<Box<dyn ModuleReference>>>,
tracing_mode: CachedExternalTracingMode,
) -> Vc<Self> {
Self::cell(CachedExternalModule {
request,
external_type,
additional_references,
tracing_mode,
})
}

Expand Down Expand Up @@ -202,8 +221,46 @@ impl Module for CachedExternalModule {
}

#[turbo_tasks::function]
fn references(&self) -> Result<Vc<ModuleReferences>> {
Ok(Vc::cell(self.additional_references.clone()))
async fn references(&self) -> Result<Vc<ModuleReferences>> {
Ok(match &self.tracing_mode {
CachedExternalTracingMode::Untraced => ModuleReferences::empty(),
CachedExternalTracingMode::Traced {
externals_context,
root_origin,
} => {
let reference_type = match self.external_type {
CachedExternalType::EcmaScriptViaImport => {
ReferenceType::EcmaScriptModules(Default::default())
}
CachedExternalType::CommonJs | CachedExternalType::EcmaScriptViaRequire => {
ReferenceType::CommonJs(Default::default())
}
_ => ReferenceType::Undefined,
};

let external_result = externals_context
.resolve_asset(
root_origin.clone(),
Request::parse_string(self.request.clone()),
externals_context
.resolve_options(root_origin.clone(), reference_type.clone()),
reference_type,
)
.await?;
let references = external_result
.affecting_sources
.iter()
.map(|s| Vc::upcast::<Box<dyn Module>>(RawModule::new(**s)))
.chain(external_result.primary_modules_raw_iter().map(|rvc| *rvc))
.map(|s| {
Vc::upcast::<Box<dyn ModuleReference>>(TracedModuleReference::new(s))
.to_resolved()
})
.try_join()
.await?;
Vc::cell(references)
}
})
}

#[turbo_tasks::function]
Expand Down
107 changes: 26 additions & 81 deletions turbopack/crates/turbopack/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use graph::{AggregatedGraph, AggregatedGraphNodeContent, aggregate};
use module_options::{ModuleOptions, ModuleOptionsContext, ModuleRuleEffect, ModuleType};
use tracing::{Instrument, field::Empty};
use turbo_rcstr::{RcStr, rcstr};
use turbo_tasks::{FxIndexSet, ResolvedVc, ValueToString, Vc};
use turbo_tasks::{ResolvedVc, ValueToString, Vc};
use turbo_tasks_fs::{FileSystemPath, glob::Glob};
pub use turbopack_core::condition;
use turbopack_core::{
Expand All @@ -40,7 +40,6 @@ use turbopack_core::{
module::Module,
output::OutputAsset,
raw_module::RawModule,
reference::{ModuleReference, TracedModuleReference},
reference_type::{
CssReferenceSubType, EcmaScriptModulesReferenceSubType, ImportContext, ImportWithType,
InnerAssets, ReferenceType,
Expand All @@ -55,7 +54,9 @@ use turbopack_core::{
pub use turbopack_css as css;
pub use turbopack_ecmascript as ecmascript;
use turbopack_ecmascript::{
references::external_module::{CachedExternalModule, CachedExternalType},
references::external_module::{
CachedExternalModule, CachedExternalTracingMode, CachedExternalType,
},
tree_shake::asset::EcmascriptModulePartAsset,
};
use turbopack_json::JsonModuleAsset;
Expand Down Expand Up @@ -769,8 +770,6 @@ impl AssetContext for ModuleAssetContext {

let result = result.await?;

let affecting_sources = &result.affecting_sources;

let result = result
.map_primary_items(|item| {
let reference_type = reference_type.clone();
Expand All @@ -789,89 +788,35 @@ impl AssetContext for ModuleAssetContext {
}
ResolveResultItem::External { name, ty, traced } => {
let replacement = if replace_externals {
let additional_refs: Vec<Vc<Box<dyn ModuleReference>>> = if let (
ExternalTraced::Traced,
Some(tracing_root),
) = (
traced,
self.module_options_context()
let tracing_mode = if traced == ExternalTraced::Traced
&& let Some(tracing_root) = &self
.module_options_context()
.await?
.enable_externals_tracing
.clone(),
) {
let externals_context = externals_tracing_module_context(ty);
let root_origin = tracing_root.join("_")?;

// Normalize reference type, there is no such thing as a
// `ReferenceType::EcmaScriptModules(ImportPart(Evaluation))`
// for externals (and otherwise, this causes duplicate
// CachedExternalModules for both `ImportPart(Evaluation)` and
// `ImportPart(Export("CacheProvider"))`)
let reference_type = match reference_type {
ReferenceType::EcmaScriptModules(_) => {
ReferenceType::EcmaScriptModules(Default::default())
}
ReferenceType::CommonJs(_) => {
ReferenceType::CommonJs(Default::default())
}
ReferenceType::Css(_) => {
ReferenceType::Css(Default::default())
}
ReferenceType::Url(_) => {
ReferenceType::Url(Default::default())
}
_ => ReferenceType::Undefined,
};

let external_result = externals_context
.resolve_asset(
root_origin.clone(),
Request::parse_string(name.clone()),
externals_context.resolve_options(
root_origin,
reference_type.clone(),
),
reference_type,
)
.await?;

let modules = affecting_sources
.iter()
.chain(external_result.affecting_sources.iter())
.map(|s| Vc::upcast::<Box<dyn Module>>(RawModule::new(**s)))
.chain(
external_result
.primary_modules_raw_iter()
.map(|rvc| *rvc),
)
.collect::<FxIndexSet<_>>();

modules
.into_iter()
.map(|s| {
Vc::upcast::<Box<dyn ModuleReference>>(
TracedModuleReference::new(s),
)
})
.collect()
{
// result.affecting_sources can be ignored for tracing, as this
// request will later be resolved relative to tracing_root
// anyway.

CachedExternalTracingMode::Traced {
externals_context: ResolvedVc::upcast(
externals_tracing_module_context(ty)
.to_resolved()
.await?,
),
root_origin: tracing_root.join("_")?,
}
} else {
vec![]
CachedExternalTracingMode::Untraced
};

replace_external(&name, ty, additional_refs, import_externals)
.await?
replace_external(&name, ty, import_externals, tracing_mode).await?
} else {
None
};

replacement.unwrap_or_else(|| {
ModuleResolveResultItem::External {
name,
ty,
// TODO(micshnic) remove that field entirely ?
traced: None,
}
})
replacement
.unwrap_or_else(|| ModuleResolveResultItem::External { name, ty })
}
ResolveResultItem::Ignore => ModuleResolveResultItem::Ignore,
ResolveResultItem::Empty => ModuleResolveResultItem::Empty,
Expand Down Expand Up @@ -1000,8 +945,8 @@ pub async fn emit_asset_into_dir(
pub async fn replace_external(
name: &RcStr,
ty: ExternalType,
additional_refs: Vec<Vc<Box<dyn ModuleReference>>>,
import_externals: bool,
tracing_mode: CachedExternalTracingMode,
) -> Result<Option<ModuleResolveResultItem>> {
let external_type = match ty {
ExternalType::CommonJs => CachedExternalType::CommonJs,
Expand All @@ -1020,7 +965,7 @@ pub async fn replace_external(
}
};

let module = CachedExternalModule::new(name.clone(), external_type, additional_refs)
let module = CachedExternalModule::new(name.clone(), external_type, tracing_mode)
.to_resolved()
.await?;

Expand Down
Loading