Skip to content

Commit 76b08ea

Browse files
mischnicmischnic
committed
Turbopack: fix duplicate externals module
1 parent dc48e55 commit 76b08ea

File tree

3 files changed

+95
-87
lines changed

3 files changed

+95
-87
lines changed

‎turbopack/crates/turbopack-core/src/resolve/mod.rs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ pub enum ModuleResolveResultItem {
7676
/// uri, path, reference, etc.
7777
name: RcStr,
7878
ty: ExternalType,
79-
traced: Option<ResolvedVc<ModuleResolveResult>>,
8079
},
8180
/// A module could not be created (according to the rules, e.g. no module type as assigned)
8281
Unknown(ResolvedVc<Box<dyn Source>>),
@@ -717,11 +716,7 @@ impl ResolveResult {
717716
// Should use map_primary_items instead
718717
bail!("map_module doesn't handle traced externals");
719718
}
720-
ModuleResolveResultItem::External {
721-
name,
722-
ty,
723-
traced: None,
724-
}
719+
ModuleResolveResultItem::External { name, ty }
725720
}
726721
ResolveResultItem::Ignore => ModuleResolveResultItem::Ignore,
727722
ResolveResultItem::Empty => ModuleResolveResultItem::Empty,

‎turbopack/crates/turbopack-ecmascript/src/references/external_module.rs

Lines changed: 65 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,21 @@ use std::{fmt::Display, io::Write};
33
use anyhow::Result;
44
use serde::{Deserialize, Serialize};
55
use turbo_rcstr::{RcStr, rcstr};
6-
use turbo_tasks::{NonLocalValue, ResolvedVc, TaskInput, Vc, trace::TraceRawVcs};
7-
use turbo_tasks_fs::{FileContent, FileSystem, VirtualFileSystem, glob::Glob, rope::RopeBuilder};
6+
use turbo_tasks::{NonLocalValue, ResolvedVc, TaskInput, TryJoinIterExt, Vc, trace::TraceRawVcs};
7+
use turbo_tasks_fs::{
8+
FileContent, FileSystem, FileSystemPath, VirtualFileSystem, glob::Glob, rope::RopeBuilder,
9+
};
810
use turbopack_core::{
911
asset::{Asset, AssetContent},
1012
chunk::{AsyncModuleInfo, ChunkItem, ChunkType, ChunkableModule, ChunkingContext},
13+
context::AssetContext,
1114
ident::{AssetIdent, Layer},
1215
module::Module,
1316
module_graph::ModuleGraph,
14-
reference::{ModuleReference, ModuleReferences},
17+
raw_module::RawModule,
18+
reference::{ModuleReference, ModuleReferences, TracedModuleReference},
19+
reference_type::ReferenceType,
20+
resolve::parse::Request,
1521
};
1622

1723
use crate::{
@@ -49,6 +55,17 @@ pub enum CachedExternalType {
4955
Script,
5056
}
5157

58+
#[derive(
59+
Clone, Debug, Eq, PartialEq, Serialize, Deserialize, TraceRawVcs, TaskInput, Hash, NonLocalValue,
60+
)]
61+
pub enum CachedExternalTracingMode {
62+
Untraced,
63+
Traced {
64+
externals_context: ResolvedVc<Box<dyn AssetContext>>,
65+
root_origin: FileSystemPath,
66+
},
67+
}
68+
5269
impl Display for CachedExternalType {
5370
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
5471
match self {
@@ -63,9 +80,9 @@ impl Display for CachedExternalType {
6380

6481
#[turbo_tasks::value]
6582
pub struct CachedExternalModule {
66-
pub request: RcStr,
67-
pub external_type: CachedExternalType,
68-
pub additional_references: Vec<ResolvedVc<Box<dyn ModuleReference>>>,
83+
request: RcStr,
84+
external_type: CachedExternalType,
85+
tracing_mode: CachedExternalTracingMode,
6986
}
7087

7188
#[turbo_tasks::value_impl]
@@ -74,12 +91,12 @@ impl CachedExternalModule {
7491
pub fn new(
7592
request: RcStr,
7693
external_type: CachedExternalType,
77-
additional_references: Vec<ResolvedVc<Box<dyn ModuleReference>>>,
94+
tracing_mode: CachedExternalTracingMode,
7895
) -> Vc<Self> {
7996
Self::cell(CachedExternalModule {
8097
request,
8198
external_type,
82-
additional_references,
99+
tracing_mode,
83100
})
84101
}
85102

@@ -202,8 +219,46 @@ impl Module for CachedExternalModule {
202219
}
203220

204221
#[turbo_tasks::function]
205-
fn references(&self) -> Result<Vc<ModuleReferences>> {
206-
Ok(Vc::cell(self.additional_references.clone()))
222+
async fn references(&self) -> Result<Vc<ModuleReferences>> {
223+
Ok(match &self.tracing_mode {
224+
CachedExternalTracingMode::Untraced => ModuleReferences::empty(),
225+
CachedExternalTracingMode::Traced {
226+
externals_context,
227+
root_origin,
228+
} => {
229+
let reference_type = match self.external_type {
230+
CachedExternalType::EcmaScriptViaImport => {
231+
ReferenceType::EcmaScriptModules(Default::default())
232+
}
233+
CachedExternalType::CommonJs | CachedExternalType::EcmaScriptViaRequire => {
234+
ReferenceType::CommonJs(Default::default())
235+
}
236+
_ => ReferenceType::Undefined,
237+
};
238+
239+
let external_result = externals_context
240+
.resolve_asset(
241+
root_origin.clone(),
242+
Request::parse_string(self.request.clone()),
243+
externals_context
244+
.resolve_options(root_origin.clone(), reference_type.clone()),
245+
reference_type,
246+
)
247+
.await?;
248+
let references = external_result
249+
.affecting_sources
250+
.iter()
251+
.map(|s| Vc::upcast::<Box<dyn Module>>(RawModule::new(**s)))
252+
.chain(external_result.primary_modules_raw_iter().map(|rvc| *rvc))
253+
.map(|s| {
254+
Vc::upcast::<Box<dyn ModuleReference>>(TracedModuleReference::new(s))
255+
.to_resolved()
256+
})
257+
.try_join()
258+
.await?;
259+
Vc::cell(references)
260+
}
261+
})
207262
}
208263

209264
#[turbo_tasks::function]

‎turbopack/crates/turbopack/src/lib.rs

Lines changed: 29 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ use graph::{AggregatedGraph, AggregatedGraphNodeContent, aggregate};
2626
use module_options::{ModuleOptions, ModuleOptionsContext, ModuleRuleEffect, ModuleType};
2727
use tracing::{Instrument, field::Empty};
2828
use turbo_rcstr::{RcStr, rcstr};
29-
use turbo_tasks::{FxIndexSet, ResolvedVc, ValueToString, Vc};
29+
use turbo_tasks::{ResolvedVc, ValueToString, Vc};
3030
use turbo_tasks_fs::{FileSystemPath, glob::Glob};
3131
pub use turbopack_core::condition;
3232
use turbopack_core::{
@@ -55,7 +55,9 @@ use turbopack_core::{
5555
pub use turbopack_css as css;
5656
pub use turbopack_ecmascript as ecmascript;
5757
use turbopack_ecmascript::{
58-
references::external_module::{CachedExternalModule, CachedExternalType},
58+
references::external_module::{
59+
CachedExternalModule, CachedExternalTracingMode, CachedExternalType,
60+
},
5961
tree_shake::asset::EcmascriptModulePartAsset,
6062
};
6163
use turbopack_json::JsonModuleAsset;
@@ -789,89 +791,45 @@ impl AssetContext for ModuleAssetContext {
789791
}
790792
ResolveResultItem::External { name, ty, traced } => {
791793
let replacement = if replace_externals {
792-
let additional_refs: Vec<Vc<Box<dyn ModuleReference>>> = if let (
793-
ExternalTraced::Traced,
794-
Some(tracing_root),
795-
) = (
796-
traced,
797-
self.module_options_context()
794+
let tracing_mode = if traced == ExternalTraced::Traced
795+
&& let Some(tracing_root) = self
796+
.module_options_context()
798797
.await?
799798
.enable_externals_tracing
800-
.clone(),
801-
) {
802-
let externals_context = externals_tracing_module_context(ty);
803-
let root_origin = tracing_root.join("_")?;
804-
805-
// Normalize reference type, there is no such thing as a
806-
// `ReferenceType::EcmaScriptModules(ImportPart(Evaluation))`
807-
// for externals (and otherwise, this causes duplicate
808-
// CachedExternalModules for both `ImportPart(Evaluation)` and
809-
// `ImportPart(Export("CacheProvider"))`)
810-
let reference_type = match reference_type {
811-
ReferenceType::EcmaScriptModules(_) => {
812-
ReferenceType::EcmaScriptModules(Default::default())
813-
}
814-
ReferenceType::CommonJs(_) => {
815-
ReferenceType::CommonJs(Default::default())
816-
}
817-
ReferenceType::Css(_) => {
818-
ReferenceType::Css(Default::default())
819-
}
820-
ReferenceType::Url(_) => {
821-
ReferenceType::Url(Default::default())
822-
}
823-
_ => ReferenceType::Undefined,
824-
};
825-
826-
let external_result = externals_context
827-
.resolve_asset(
828-
root_origin.clone(),
829-
Request::parse_string(name.clone()),
830-
externals_context.resolve_options(
831-
root_origin,
832-
reference_type.clone(),
833-
),
834-
reference_type,
835-
)
836-
.await?;
799+
.clone()
800+
{
801+
// TODO need to be attatched somewhere
837802

838-
let modules = affecting_sources
803+
// These are all files that are need to resolve the external
804+
// itself, i.e. aliases etc.
805+
let _modules = affecting_sources
839806
.iter()
840-
.chain(external_result.affecting_sources.iter())
841807
.map(|s| Vc::upcast::<Box<dyn Module>>(RawModule::new(**s)))
842-
.chain(
843-
external_result
844-
.primary_modules_raw_iter()
845-
.map(|rvc| *rvc),
846-
)
847-
.collect::<FxIndexSet<_>>();
848-
849-
modules
850-
.into_iter()
851808
.map(|s| {
852809
Vc::upcast::<Box<dyn ModuleReference>>(
853810
TracedModuleReference::new(s),
854811
)
855-
})
856-
.collect()
812+
});
813+
814+
CachedExternalTracingMode::Traced {
815+
externals_context: ResolvedVc::upcast(
816+
externals_tracing_module_context(ty)
817+
.to_resolved()
818+
.await?,
819+
),
820+
root_origin: tracing_root.join("_")?,
821+
}
857822
} else {
858-
vec![]
823+
CachedExternalTracingMode::Untraced
859824
};
860825

861-
replace_external(&name, ty, additional_refs, import_externals)
862-
.await?
826+
replace_external(&name, ty, import_externals, tracing_mode).await?
863827
} else {
864828
None
865829
};
866830

867-
replacement.unwrap_or_else(|| {
868-
ModuleResolveResultItem::External {
869-
name,
870-
ty,
871-
// TODO(micshnic) remove that field entirely ?
872-
traced: None,
873-
}
874-
})
831+
replacement
832+
.unwrap_or_else(|| ModuleResolveResultItem::External { name, ty })
875833
}
876834
ResolveResultItem::Ignore => ModuleResolveResultItem::Ignore,
877835
ResolveResultItem::Empty => ModuleResolveResultItem::Empty,
@@ -1000,8 +958,8 @@ pub async fn emit_asset_into_dir(
1000958
pub async fn replace_external(
1001959
name: &RcStr,
1002960
ty: ExternalType,
1003-
additional_refs: Vec<Vc<Box<dyn ModuleReference>>>,
1004961
import_externals: bool,
962+
tracing_mode: CachedExternalTracingMode,
1005963
) -> Result<Option<ModuleResolveResultItem>> {
1006964
let external_type = match ty {
1007965
ExternalType::CommonJs => CachedExternalType::CommonJs,
@@ -1020,7 +978,7 @@ pub async fn replace_external(
1020978
}
1021979
};
1022980

1023-
let module = CachedExternalModule::new(name.clone(), external_type, additional_refs)
981+
let module = CachedExternalModule::new(name.clone(), external_type, tracing_mode)
1024982
.to_resolved()
1025983
.await?;
1026984

0 commit comments

Comments
 (0)