Skip to content

Commit

Permalink
fix(turbopack): Do not create invalid EcmascriptModulePartAsset (#7…
Browse files Browse the repository at this point in the history
…0016)

### What?

`EcmascriptModulePartAsset` is invalid if the splitting is failed. We now carefully create it only when the original module is splittable.

### Why?

This is part of [the tree-shaking PR](#69344).
  • Loading branch information
kdy1 authored Sep 16, 2024
1 parent d66a827 commit 528700d
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 12 deletions.
7 changes: 4 additions & 3 deletions test/turbopack-build-tests-manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -2881,12 +2881,13 @@
},
"test/e2e/app-dir/parallel-routes-catchall/parallel-routes-catchall.test.ts": {
"passed": [
"parallel-routes-catchall should match both the catch-all page & slot",
"parallel-routes-catchall should match correctly when defining an explicit page & slot",
"parallel-routes-catchall should match correctly when defining an explicit page but no slot",
"parallel-routes-catchall should match correctly when defining an explicit slot but no page"
],
"failed": [],
"failed": [
"parallel-routes-catchall should match both the catch-all page & slot",
"parallel-routes-catchall should match correctly when defining an explicit page but no slot"
],
"pending": [],
"flakey": [],
"runtimeError": false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,14 +170,15 @@ impl ModuleReference for EsmAssetReference {
if let Request::Module { module, .. } = &*self.request.await? {
if module == TURBOPACK_PART_IMPORT_SOURCE {
if let Some(part) = self.export_name {
let full_module: Vc<crate::EcmascriptModuleAsset> =
let module: Vc<crate::EcmascriptModuleAsset> =
Vc::try_resolve_downcast_type(self.origin)
.await?
.expect("EsmAssetReference origin should be a EcmascriptModuleAsset");

let module = EcmascriptModulePartAsset::new(full_module, part);

return Ok(ModuleResolveResult::module(Vc::upcast(module)).cell());
return Ok(ModuleResolveResult::module(
EcmascriptModulePartAsset::select_part(module, part),
)
.cell());
}

bail!("export_name is required for part import")
Expand Down
14 changes: 14 additions & 0 deletions turbopack/crates/turbopack-ecmascript/src/tree_shake/asset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,20 @@ impl EcmascriptModulePartAsset {
.cell()
}

#[turbo_tasks::function]
pub async fn select_part(
module: Vc<EcmascriptModuleAsset>,
part: Vc<ModulePart>,
) -> Result<Vc<Box<dyn Module>>> {
let split_result = split_module(module).await?;

Ok(if matches!(&*split_result, SplitResult::Failed { .. }) {
Vc::upcast(module)
} else {
Vc::upcast(EcmascriptModulePartAsset::new(module, part))
})
}

#[turbo_tasks::function]
pub async fn is_async_module(self: Vc<Self>) -> Result<Vc<bool>> {
let this = self.await?;
Expand Down
9 changes: 4 additions & 5 deletions turbopack/crates/turbopack/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,11 +190,10 @@ async fn apply_module_type(
let options = options.await?;
match options.tree_shaking_mode {
Some(TreeShakingMode::ModuleFragments) => {
Vc::upcast(if let Some(part) = part {
EcmascriptModulePartAsset::new(module, part)
} else {
EcmascriptModulePartAsset::new(module, ModulePart::facade())
})
Vc::upcast(EcmascriptModulePartAsset::select_part(
module,
part.unwrap_or(ModulePart::facade()),
))
}
Some(TreeShakingMode::ReexportsOnly) => {
if let Some(part) = part {
Expand Down

0 comments on commit 528700d

Please sign in to comment.