Skip to content

Commit

Permalink
fix externals in side-effect optimized modules (vercel/turborepo#7797)
Browse files Browse the repository at this point in the history
### Description

fixes a problem where an async module wrapper is generated for external
modules in a "locals" module part when there is usage of the external
module in the locals part

fixes #63485

### Testing Instructions

<!--
  Give a quick description of steps to test your changes.
-->


Closes PACK-2810

fixes PACK-2805
  • Loading branch information
sokra authored Mar 25, 2024
1 parent f70dd0e commit 3e52ab7
Show file tree
Hide file tree
Showing 24 changed files with 203 additions and 48 deletions.
11 changes: 10 additions & 1 deletion crates/turbopack-ecmascript/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,7 @@ impl EcmascriptModuleAsset {
chunking_context,
analyze.references,
analyze.code_generation,
analyze.async_module,
analyze.source_map,
analyze.exports,
async_module_info,
Expand Down Expand Up @@ -660,7 +661,7 @@ impl ChunkItem for ModuleChunkItem {
#[turbo_tasks::function]
async fn is_self_async(&self) -> Result<Vc<bool>> {
if let Some(async_module) = *self.module.get_async_module().await? {
Ok(async_module.is_self_async())
Ok(async_module.is_self_async(self.module.failsafe_analyze().await?.references))
} else {
Ok(Vc::cell(false))
}
Expand Down Expand Up @@ -727,6 +728,7 @@ impl EcmascriptModuleContent {
chunking_context: Vc<Box<dyn EcmascriptChunkingContext>>,
references: Vc<ModuleReferences>,
code_generation: Vc<CodeGenerateables>,
async_module: Vc<OptionAsyncModule>,
source_map: Vc<OptionSourceMap>,
exports: Vc<EcmascriptExports>,
async_module_info: Option<Vc<AsyncModuleInfo>>,
Expand All @@ -744,6 +746,13 @@ impl EcmascriptModuleContent {
code_gens.push(code_gen.code_generation(chunking_context));
}
}
if let Some(async_module) = *async_module.await? {
code_gens.push(async_module.code_generation(
chunking_context,
async_module_info,
references,
));
}
for c in code_generation.await?.iter() {
match c {
CodeGen::CodeGenerateable(c) => {
Expand Down
52 changes: 34 additions & 18 deletions crates/turbopack-ecmascript/src/references/async_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,17 @@ use swc_core::{
ecma::ast::{ArrayLit, ArrayPat, Expr, Ident, Program},
quote,
};
use turbo_tasks::{trace::TraceRawVcs, TryFlatJoinIterExt, TryJoinIterExt, Vc};
use turbo_tasks::{trace::TraceRawVcs, ReadRef, TryFlatJoinIterExt, TryJoinIterExt, Vc};
use turbopack_core::{
chunk::{AsyncModuleInfo, ChunkableModule},
chunk::{AsyncModuleInfo, ChunkableModule, ChunkableModuleReference, ChunkingType},
reference::{ModuleReference, ModuleReferences},
resolve::ExternalType,
};

use super::esm::base::ReferencedAsset;
use crate::{
chunk::{EcmascriptChunkPlaceable, EcmascriptChunkingContext},
code_gen::{CodeGenerateableWithAsyncModuleInfo, CodeGeneration},
code_gen::CodeGeneration,
create_visitor,
references::esm::base::insert_hoisted_stmt,
};
Expand Down Expand Up @@ -48,7 +48,6 @@ impl OptionAsyncModuleOptions {
#[turbo_tasks::value(shared)]
pub struct AsyncModule {
pub placeable: Vc<Box<dyn EcmascriptChunkPlaceable>>,
pub references: Vc<ModuleReferences>,
pub has_top_level_await: bool,
pub import_externals: bool,
}
Expand Down Expand Up @@ -81,23 +80,41 @@ impl OptionAsyncModule {
#[turbo_tasks::value(transparent)]
struct AsyncModuleIdents(IndexSet<String>);

async fn get_inherit_async_referenced_asset(
r: Vc<Box<dyn ModuleReference>>,
) -> Result<Option<ReadRef<ReferencedAsset>>> {
let Some(r) = Vc::try_resolve_downcast::<Box<dyn ChunkableModuleReference>>(r).await? else {
return Ok(None);
};
let Some(ty) = *r.chunking_type().await? else {
return Ok(None);
};
if !matches!(ty, ChunkingType::ParallelInheritAsync) {
return Ok(None);
};
let referenced_asset: turbo_tasks::ReadRef<ReferencedAsset> =
ReferencedAsset::from_resolve_result(r.resolve_reference()).await?;
Ok(Some(referenced_asset))
}

#[turbo_tasks::value_impl]
impl AsyncModule {
#[turbo_tasks::function]
async fn get_async_idents(
&self,
chunking_context: Vc<Box<dyn EcmascriptChunkingContext>>,
async_module_info: Vc<AsyncModuleInfo>,
references: Vc<ModuleReferences>,
) -> Result<Vc<AsyncModuleIdents>> {
let async_module_info = async_module_info.await?;

let reference_idents = self
.references
let reference_idents = references
.await?
.iter()
.map(|r| async {
let referenced_asset =
ReferencedAsset::from_resolve_result(r.resolve_reference()).await?;
let Some(referenced_asset) = get_inherit_async_referenced_asset(*r).await? else {
return Ok(None);
};
Ok(match &*referenced_asset {
ReferencedAsset::External(
_,
Expand Down Expand Up @@ -134,20 +151,21 @@ impl AsyncModule {
}

#[turbo_tasks::function]
pub(crate) async fn is_self_async(&self) -> Result<Vc<bool>> {
pub(crate) async fn is_self_async(&self, references: Vc<ModuleReferences>) -> Result<Vc<bool>> {
if self.has_top_level_await {
return Ok(Vc::cell(true));
}

Ok(Vc::cell(
self.import_externals
&& self
.references
&& references
.await?
.iter()
.map(|r| async {
let referenced_asset =
ReferencedAsset::from_resolve_result(r.resolve_reference()).await?;
let Some(referenced_asset) = get_inherit_async_referenced_asset(*r).await?
else {
return Ok(false);
};
Ok(matches!(
&*referenced_asset,
ReferencedAsset::External(
Expand Down Expand Up @@ -177,21 +195,19 @@ impl AsyncModule {
has_top_level_await: self.await?.has_top_level_await,
})))
}
}

#[turbo_tasks::value_impl]
impl CodeGenerateableWithAsyncModuleInfo for AsyncModule {
#[turbo_tasks::function]
async fn code_generation(
pub async fn code_generation(
self: Vc<Self>,
chunking_context: Vc<Box<dyn EcmascriptChunkingContext>>,
async_module_info: Option<Vc<AsyncModuleInfo>>,
references: Vc<ModuleReferences>,
) -> Result<Vc<CodeGeneration>> {
let mut visitors = Vec::new();

if let Some(async_module_info) = async_module_info {
let async_idents = self
.get_async_idents(chunking_context, async_module_info)
.get_async_idents(chunking_context, async_module_info, references)
.await?;

if !async_idents.is_empty() {
Expand Down
4 changes: 1 addition & 3 deletions crates/turbopack-ecmascript/src/references/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@ pub(crate) async fn analyse_ecmascript_module_internal(
*r = r.resolve().await?;
}
for r in import_references.iter() {
// `add_reference` will avoid adding duplicate references
// `add_import_reference` will avoid adding duplicate references
analysis.add_import_reference(*r);
}
for i in evaluation_references {
Expand Down Expand Up @@ -750,13 +750,11 @@ pub(crate) async fn analyse_ecmascript_module_internal(
if eval_context.is_esm() || specified_type == SpecifiedModuleType::EcmaScript {
let async_module = AsyncModule {
placeable: Vc::upcast(module),
references: Vc::cell(import_references.iter().map(|&r| Vc::upcast(r)).collect()),
has_top_level_await,
import_externals,
}
.cell();
analysis.set_async_module(async_module);
analysis.add_code_gen_with_availability_info(async_module);
} else if let Some(span) = top_level_await_span {
AnalyzeIssue {
code: None,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,10 @@ impl EcmascriptChunkItem for EcmascriptModuleFacadeChunkItem {

let mut code = RopeBuilder::default();

let references = self.module.references().await?;
let mut code_gens = Vec::with_capacity(references.len() + 2);
for r in references.iter() {
let references = self.module.references();
let references_ref = references.await?;
let mut code_gens = Vec::with_capacity(references_ref.len() + 2);
for r in references_ref.iter() {
let r = r.resolve().await?;
if let Some(code_gen) =
Vc::try_resolve_sidecast::<Box<dyn CodeGenerateableWithAsyncModuleInfo>>(r).await?
Expand All @@ -82,12 +83,11 @@ impl EcmascriptChunkItem for EcmascriptModuleFacadeChunkItem {
code_gens.push(code_gen.code_generation(chunking_context));
}
}

code_gens.push(
self.module
.async_module()
.code_generation(chunking_context, async_module_info),
);
code_gens.push(self.module.async_module().code_generation(
chunking_context,
async_module_info,
references,
));
code_gens.push(exports.code_generation(chunking_context));
let code_gens = code_gens.into_iter().try_join().await?;
let code_gens = code_gens.iter().map(|cg| &**cg).collect::<Vec<_>>();
Expand Down Expand Up @@ -183,4 +183,18 @@ impl ChunkItem for EcmascriptModuleFacadeChunkItem {
fn module(&self) -> Vc<Box<dyn Module>> {
Vc::upcast(self.module)
}

#[turbo_tasks::function]
async fn is_self_async(&self) -> Result<Vc<bool>> {
let module = self.module;
let async_module = module.async_module();
let references = module.references();
let is_self_async = async_module
.resolve()
.await?
.is_self_async(references.resolve().await?)
.resolve()
.await?;
Ok(is_self_async)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,19 @@ impl EcmascriptModuleFacadeModule {
}

#[turbo_tasks::function]
pub fn async_module(self: Vc<Self>) -> Vc<AsyncModule> {
AsyncModule {
pub async fn async_module(self: Vc<Self>) -> Result<Vc<AsyncModule>> {
let import_externals =
if let Some(async_module) = *self.await?.module.get_async_module().await? {
async_module.await?.import_externals
} else {
false
};
Ok(AsyncModule {
placeable: Vc::upcast(self),
references: self.references(),
has_top_level_await: false,
import_externals: false,
import_externals,
}
.cell()
.cell())
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,12 @@ impl EcmascriptChunkItem for EcmascriptModuleLocalsChunkItem {
let chunking_context = self.chunking_context;
let exports = self.module.get_exports();
let original_module = module.module;
let async_module_options = original_module
.get_async_module()
.module_options(async_module_info);
let parsed = original_module.parse().resolve().await?;

let analyze_result = original_module.analyze().await?.clone_value();
let analyze_result = original_module.analyze().await?;
let async_module_options = analyze_result
.async_module
.module_options(async_module_info);

let module_type_result = *original_module.determine_module_type().await?;

Expand All @@ -55,6 +55,7 @@ impl EcmascriptChunkItem for EcmascriptModuleLocalsChunkItem {
chunking_context,
analyze_result.local_references,
analyze_result.code_generation,
analyze_result.async_module,
analyze_result.source_map,
exports,
async_module_info,
Expand Down Expand Up @@ -103,8 +104,11 @@ impl ChunkItem for EcmascriptModuleLocalsChunkItem {

#[turbo_tasks::function]
async fn is_self_async(&self) -> Result<Vc<bool>> {
if let Some(async_module) = *self.module.get_async_module().await? {
Ok(async_module.is_self_async())
let module = self.module.await?;
let analyze = module.module.analyze().await?;
if let Some(async_module) = *analyze.async_module.await? {
let is_self_async = async_module.is_self_async(analyze.local_references);
Ok(is_self_async)
} else {
Ok(Vc::cell(false))
}
Expand Down
10 changes: 4 additions & 6 deletions crates/turbopack-ecmascript/src/tree_shake/chunk_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ use turbopack_core::{
use super::{asset::EcmascriptModulePartAsset, part_of_module, split_module};
use crate::{
chunk::{
placeable::EcmascriptChunkPlaceable, EcmascriptChunkItem, EcmascriptChunkItemContent,
EcmascriptChunkType, EcmascriptChunkingContext,
EcmascriptChunkItem, EcmascriptChunkItemContent, EcmascriptChunkType,
EcmascriptChunkingContext,
},
EcmascriptModuleContent,
};
Expand Down Expand Up @@ -40,15 +40,12 @@ impl EcmascriptChunkItem for EcmascriptModulePartChunkItem {
) -> Result<Vc<EcmascriptChunkItemContent>> {
let this = self.await?;
let module = this.module.await?;
let async_module_options = module
.full_module
.get_async_module()
.module_options(async_module_info);

let split_data = split_module(module.full_module);
let parsed = part_of_module(split_data, module.part);

let analyze = this.module.analyze().await?;
let async_module_options = analyze.async_module.module_options(async_module_info);

let module_type_result = *module.full_module.determine_module_type().await?;

Expand All @@ -59,6 +56,7 @@ impl EcmascriptChunkItem for EcmascriptModulePartChunkItem {
this.chunking_context,
analyze.references,
analyze.code_generation,
analyze.async_module,
analyze.source_map,
analyze.exports,
async_module_info,
Expand Down
12 changes: 12 additions & 0 deletions crates/turbopack-tests/tests/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ use turbopack_core::{
file_source::FileSource,
issue::{Issue, IssueDescriptionExt},
reference_type::{EntryReferenceSubType, ReferenceType},
resolve::{
options::{ImportMap, ImportMapping},
ExternalType,
},
source::Source,
};
use turbopack_ecmascript_runtime::RuntimeType;
Expand Down Expand Up @@ -256,13 +260,20 @@ async fn run_test(prepared_test: Vc<PreparedTest>) -> Result<Vc<RunTestResult>>
)
.cell();

let mut import_map = ImportMap::empty();
import_map.insert_wildcard_alias(
"esm-external/",
ImportMapping::External(Some("*".to_string()), ExternalType::EcmaScriptModule).cell(),
);

let asset_context: Vc<Box<dyn AssetContext>> = Vc::upcast(ModuleAssetContext::new(
Vc::cell(HashMap::new()),
compile_time_info,
ModuleOptionsContext {
enable_typescript_transform: Some(Default::default()),
preset_env_versions: Some(env),
tree_shaking_mode: options.tree_shaking_mode,
import_externals: true,
rules: vec![(
ContextCondition::InDirectory("node_modules".to_string()),
ModuleOptionsContext {
Expand Down Expand Up @@ -290,6 +301,7 @@ async fn run_test(prepared_test: Vc<PreparedTest>) -> Result<Vc<RunTestResult>>
)],
browser: true,
module: true,
import_map: Some(import_map.cell()),
..Default::default()
}
.cell(),
Expand Down
Loading

0 comments on commit 3e52ab7

Please sign in to comment.