From 97e3e8789e36dc0b37356d4d3ce71524c4989b86 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Thu, 7 Nov 2024 11:21:53 +0100 Subject: [PATCH] ImportMapping::PrimaryAlternativeExternal --- crates/next-core/src/next_build.rs | 1 - crates/next-core/src/next_import_map.rs | 16 ++-- .../src/next_shared/webpack_rules/mod.rs | 1 - .../crates/turbopack-core/src/resolve/mod.rs | 75 +++++++++-------- .../turbopack-core/src/resolve/options.rs | 84 ++++++++++++++----- .../crates/turbopack-resolve/src/resolve.rs | 28 ++----- .../crates/turbopack-tests/tests/execution.rs | 1 - 7 files changed, 112 insertions(+), 94 deletions(-) diff --git a/crates/next-core/src/next_build.rs b/crates/next-core/src/next_build.rs index 224ee1fec884b..a11557044f6d2 100644 --- a/crates/next-core/src/next_build.rs +++ b/crates/next-core/src/next_build.rs @@ -30,7 +30,6 @@ pub async fn get_external_next_compiled_package_mapping( Some(format!("next/dist/compiled/{}", &*package_name.await?).into()), ExternalType::CommonJs, ExternalTraced::Traced, - None, ) .resolved_cell()]) .cell()) diff --git a/crates/next-core/src/next_import_map.rs b/crates/next-core/src/next_import_map.rs index d055bfc612a8c..7d027f263322c 100644 --- a/crates/next-core/src/next_import_map.rs +++ b/crates/next-core/src/next_import_map.rs @@ -249,9 +249,8 @@ pub async fn get_next_build_import_map() -> Result> { next_js_fs().root().to_resolved().await?, ); - let external = - ImportMapping::External(None, ExternalType::CommonJs, ExternalTraced::Traced, None) - .resolved_cell(); + let external = ImportMapping::External(None, ExternalType::CommonJs, ExternalTraced::Traced) + .resolved_cell(); import_map.insert_exact_alias("next", external); import_map.insert_wildcard_alias("next/", external); @@ -262,7 +261,6 @@ pub async fn get_next_build_import_map() -> Result> { Some("styled-jsx/style.js".into()), ExternalType::CommonJs, ExternalTraced::Traced, - None, ) .resolved_cell(), ); @@ -334,7 +332,6 @@ pub async fn get_next_server_import_map( ExternalType::CommonJs, // TODO(arlyon): wiring up in a follow up PR ExternalTraced::Untraced, - None, ) .resolved_cell(); @@ -355,7 +352,6 @@ pub async fn get_next_server_import_map( Some("styled-jsx/style.js".into()), ExternalType::CommonJs, ExternalTraced::Traced, - None, ) .resolved_cell(), ); @@ -1143,11 +1139,11 @@ fn external_request_to_cjs_import_mapping( context_dir: ResolvedVc, request: &str, ) -> ResolvedVc { - ImportMapping::External( + ImportMapping::PrimaryAlternativeExternal( Some(request.into()), ExternalType::CommonJs, ExternalTraced::Traced, - Some(context_dir), + context_dir, ) .resolved_cell() } @@ -1158,11 +1154,11 @@ fn external_request_to_esm_import_mapping( context_dir: ResolvedVc, request: &str, ) -> ResolvedVc { - ImportMapping::External( + ImportMapping::PrimaryAlternativeExternal( Some(request.into()), ExternalType::EcmaScriptModule, ExternalTraced::Traced, - Some(context_dir), + context_dir, ) .resolved_cell() } diff --git a/crates/next-core/src/next_shared/webpack_rules/mod.rs b/crates/next-core/src/next_shared/webpack_rules/mod.rs index faf9764aebeba..9cb41113084b6 100644 --- a/crates/next-core/src/next_shared/webpack_rules/mod.rs +++ b/crates/next-core/src/next_shared/webpack_rules/mod.rs @@ -43,7 +43,6 @@ async fn loader_runner_package_mapping() -> Result> { Some("next/dist/compiled/loader-runner".into()), ExternalType::CommonJs, ExternalTraced::Untraced, - None, ) .resolved_cell()]) .cell()) diff --git a/turbopack/crates/turbopack-core/src/resolve/mod.rs b/turbopack/crates/turbopack-core/src/resolve/mod.rs index c4d8353cae83c..c29c482d406b3 100644 --- a/turbopack/crates/turbopack-core/src/resolve/mod.rs +++ b/turbopack/crates/turbopack-core/src/resolve/mod.rs @@ -2622,46 +2622,47 @@ async fn resolve_import_map_result( )) } } - ImportMapResult::External(name, ty, traced, primary_alt) => { - let result = Some( - ResolveResult::primary(ResolveResultItem::External { - name: name.clone(), - ty: *ty, - traced: *traced, - }) - .cell(), - ); - - if let Some(context_dir) = primary_alt { - let request = Request::parse_string(name.clone()); - - // We must avoid cycles during resolving - if request.resolve().await? == *original_request - && context_dir.to_resolved().await? == original_lookup_path - { - None - } else { - let resolve_internal = resolve_internal( - **context_dir, - request, - match ty { - ExternalType::Url => options, - // TODO is that root correct? - ExternalType::CommonJs => node_cjs_resolve_options(context_dir.root()), - ExternalType::EcmaScriptModule => { - node_esm_resolve_options(context_dir.root()) - } - }, - ); + ImportMapResult::External(name, ty, traced) => Some( + ResolveResult::primary(ResolveResultItem::External { + name: name.clone(), + ty: *ty, + traced: *traced, + }) + .cell(), + ), + ImportMapResult::AliasExternal(name, ty, traced, alias_lookup_path) => { + let request = Request::parse_string(name.clone()); - if *resolve_internal.is_unresolvable().await? { - None - } else { - result + // We must avoid cycles during resolving + if request.resolve().await? == *original_request + && alias_lookup_path.to_resolved().await? == original_lookup_path + { + None + } else if !(resolve_internal( + **alias_lookup_path, + request, + match ty { + ExternalType::Url => options, + // TODO is that root correct? + ExternalType::CommonJs => node_cjs_resolve_options(alias_lookup_path.root()), + ExternalType::EcmaScriptModule => { + node_esm_resolve_options(alias_lookup_path.root()) } - } + }, + ) + .await? + .is_unresolvable_ref()) + { + Some( + ResolveResult::primary(ResolveResultItem::External { + name: name.clone(), + ty: *ty, + traced: *traced, + }) + .cell(), + ) } else { - result + None } } ImportMapResult::Alternatives(list) => { diff --git a/turbopack/crates/turbopack-core/src/resolve/options.rs b/turbopack/crates/turbopack-core/src/resolve/options.rs index d9d6c09cd9d0d..c69c79198838b 100644 --- a/turbopack/crates/turbopack-core/src/resolve/options.rs +++ b/turbopack/crates/turbopack-core/src/resolve/options.rs @@ -94,14 +94,16 @@ pub enum ResolveInPackage { #[turbo_tasks::value(shared)] #[derive(Clone)] pub enum ImportMapping { - // If specified, the optional name overrides the request, importing that external instead - // If the last option is a path, this behaves like PrimaryAlternative, only making it external - // if the request is resolvable from the directory. - External( + /// If specified, the optional name overrides the request, importing that external instead. + External(Option, ExternalType, ExternalTraced), + /// Only make the request external if the request is resolvable from the specified directory. + /// + /// If specified, the optional name overrides the request, importing that external instead. + PrimaryAlternativeExternal( Option, ExternalType, ExternalTraced, - Option>, + ResolvedVc, ), /// An already resolved result that will be returned directly. Direct(ResolvedVc), @@ -120,11 +122,12 @@ pub enum ImportMapping { #[turbo_tasks::value(shared)] #[derive(Clone)] pub enum ReplacedImportMapping { - External( + External(Option, ExternalType, ExternalTraced), + PrimaryAlternativeExternal( Option, ExternalType, ExternalTraced, - Option>, + ResolvedVc, ), Direct(ResolvedVc), PrimaryAlternative(Pattern, Option>), @@ -173,11 +176,19 @@ impl AliasTemplate for Vc { Box::pin(async move { let this = &*self.await?; Ok(match this { - ImportMapping::External(name, ty, traced, primary_alt) => { - ReplacedImportMapping::External(name.clone(), *ty, *traced, *primary_alt) + ImportMapping::External(name, ty, traced) => { + ReplacedImportMapping::External(name.clone(), *ty, *traced) + } + ImportMapping::PrimaryAlternativeExternal(name, ty, traced, lookup_dir) => { + ReplacedImportMapping::PrimaryAlternativeExternal( + name.clone(), + *ty, + *traced, + *lookup_dir, + ) } - ImportMapping::PrimaryAlternative(name, context) => { - ReplacedImportMapping::PrimaryAlternative((*name).clone().into(), *context) + ImportMapping::PrimaryAlternative(name, lookup_dir) => { + ReplacedImportMapping::PrimaryAlternative((*name).clone().into(), *lookup_dir) } ImportMapping::Direct(v) => ReplacedImportMapping::Direct(*v), ImportMapping::Ignore => ReplacedImportMapping::Ignore, @@ -202,22 +213,38 @@ impl AliasTemplate for Vc { Box::pin(async move { let this = &*self.await?; Ok(match this { - ImportMapping::External(name, ty, traced, primary_alt) => { + ImportMapping::External(name, ty, traced) => { if let Some(name) = name { ReplacedImportMapping::External( capture.spread_into_star(name).as_string().map(|s| s.into()), *ty, *traced, - *primary_alt, ) } else { - ReplacedImportMapping::External(None, *ty, *traced, *primary_alt) + ReplacedImportMapping::External(None, *ty, *traced) } } - ImportMapping::PrimaryAlternative(name, context) => { + ImportMapping::PrimaryAlternativeExternal(name, ty, traced, lookup_dir) => { + if let Some(name) = name { + ReplacedImportMapping::PrimaryAlternativeExternal( + capture.spread_into_star(name).as_string().map(|s| s.into()), + *ty, + *traced, + *lookup_dir, + ) + } else { + ReplacedImportMapping::PrimaryAlternativeExternal( + None, + *ty, + *traced, + *lookup_dir, + ) + } + } + ImportMapping::PrimaryAlternative(name, lookup_dir) => { ReplacedImportMapping::PrimaryAlternative( capture.spread_into_star(name), - *context, + *lookup_dir, ) } ImportMapping::Direct(v) => ReplacedImportMapping::Direct(*v), @@ -342,11 +369,12 @@ pub struct ResolvedMap { #[derive(Clone)] pub enum ImportMapResult { Result(ResolvedVc), - External( + External(RcStr, ExternalType, ExternalTraced), + AliasExternal( RcStr, ExternalType, ExternalTraced, - Option>, + ResolvedVc, ), Alias(ResolvedVc, Option>), Alternatives(Vec), @@ -360,8 +388,19 @@ async fn import_mapping_to_result( ) -> Result { Ok(match &*mapping.await? { ReplacedImportMapping::Direct(result) => ImportMapResult::Result(*result), - ReplacedImportMapping::External(name, ty, traced, primary_alt) => { - ImportMapResult::External( + ReplacedImportMapping::External(name, ty, traced) => ImportMapResult::External( + if let Some(name) = name { + name.clone() + } else if let Some(request) = request.await?.request() { + request + } else { + bail!("Cannot resolve external reference without request") + }, + *ty, + *traced, + ), + ReplacedImportMapping::PrimaryAlternativeExternal(name, ty, traced, lookup_dir) => { + ImportMapResult::AliasExternal( if let Some(name) = name { name.clone() } else if let Some(request) = request.await?.request() { @@ -371,7 +410,7 @@ async fn import_mapping_to_result( }, *ty, *traced, - *primary_alt, + *lookup_dir, ) } ReplacedImportMapping::Ignore => ImportMapResult::Result( @@ -405,7 +444,8 @@ impl ValueToString for ImportMapResult { async fn to_string(&self) -> Result> { match self { ImportMapResult::Result(_) => Ok(Vc::cell("Resolved by import map".into())), - ImportMapResult::External(_, _, _, _) => Ok(Vc::cell("TODO external".into())), + ImportMapResult::External(_, _, _) => Ok(Vc::cell("TODO external".into())), + ImportMapResult::AliasExternal(_, _, _, _) => Ok(Vc::cell("TODO external".into())), ImportMapResult::Alias(request, context) => { let s = if let Some(path) = context { let path = path.to_string().await?; diff --git a/turbopack/crates/turbopack-resolve/src/resolve.rs b/turbopack/crates/turbopack-resolve/src/resolve.rs index 898f3dab7e1ab..4031bc18ff1f0 100644 --- a/turbopack/crates/turbopack-resolve/src/resolve.rs +++ b/turbopack/crates/turbopack-resolve/src/resolve.rs @@ -106,23 +106,13 @@ async fn base_resolve_options( for req in NODE_EXTERNALS { direct_mappings.insert( AliasPattern::exact(req), - ImportMapping::External( - None, - ExternalType::CommonJs, - ExternalTraced::Untraced, - None, - ) - .resolved_cell(), + ImportMapping::External(None, ExternalType::CommonJs, ExternalTraced::Untraced) + .resolved_cell(), ); direct_mappings.insert( AliasPattern::exact(format!("node:{req}")), - ImportMapping::External( - None, - ExternalType::CommonJs, - ExternalTraced::Untraced, - None, - ) - .resolved_cell(), + ImportMapping::External(None, ExternalType::CommonJs, ExternalTraced::Untraced) + .resolved_cell(), ); } } @@ -134,19 +124,13 @@ async fn base_resolve_options( Some(format!("node:{req}").into()), ExternalType::CommonJs, ExternalTraced::Untraced, - None, ) .resolved_cell(), ); direct_mappings.insert( AliasPattern::exact(format!("node:{req}")), - ImportMapping::External( - None, - ExternalType::CommonJs, - ExternalTraced::Untraced, - None, - ) - .resolved_cell(), + ImportMapping::External(None, ExternalType::CommonJs, ExternalTraced::Untraced) + .resolved_cell(), ); } } diff --git a/turbopack/crates/turbopack-tests/tests/execution.rs b/turbopack/crates/turbopack-tests/tests/execution.rs index 56f27a69c32de..5b20f6fc2e536 100644 --- a/turbopack/crates/turbopack-tests/tests/execution.rs +++ b/turbopack/crates/turbopack-tests/tests/execution.rs @@ -279,7 +279,6 @@ async fn run_test(prepared_test: Vc) -> Result> Some("*".into()), ExternalType::EcmaScriptModule, ExternalTraced::Untraced, - None, ) .resolved_cell(), );