Skip to content

Commit

Permalink
generate correct async module handling for side effects optimization (#…
Browse files Browse the repository at this point in the history
…7720)

### Description

Async module handling was missing for side effects optimized modules

### Testing Instructions

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


Closes PACK-2733
  • Loading branch information
sokra authored Mar 13, 2024
1 parent a139056 commit 6c57d42
Show file tree
Hide file tree
Showing 15 changed files with 144 additions and 19 deletions.
2 changes: 1 addition & 1 deletion crates/turbopack-ecmascript/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,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(Vc::cell(*async_module.is_self_async().await?))
Ok(async_module.is_self_async())
} else {
Ok(Vc::cell(false))
}
Expand Down
13 changes: 9 additions & 4 deletions crates/turbopack-ecmascript/src/references/async_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use swc_core::{
use turbo_tasks::{trace::TraceRawVcs, TryFlatJoinIterExt, TryJoinIterExt, Vc};
use turbopack_core::{
chunk::{AsyncModuleInfo, ChunkableModule},
reference::{ModuleReference, ModuleReferences},
resolve::ExternalType,
};

Expand All @@ -17,7 +18,7 @@ use crate::{
chunk::{EcmascriptChunkPlaceable, EcmascriptChunkingContext},
code_gen::{CodeGenerateableWithAsyncModuleInfo, CodeGeneration},
create_visitor,
references::esm::{base::insert_hoisted_stmt, EsmAssetReference},
references::esm::base::insert_hoisted_stmt,
};

/// Information needed for generating the async module wrapper for
Expand Down Expand Up @@ -47,7 +48,7 @@ impl OptionAsyncModuleOptions {
#[turbo_tasks::value(shared)]
pub struct AsyncModule {
pub placeable: Vc<Box<dyn EcmascriptChunkPlaceable>>,
pub references: IndexSet<Vc<EsmAssetReference>>,
pub references: Vc<ModuleReferences>,
pub has_top_level_await: bool,
pub import_externals: bool,
}
Expand Down Expand Up @@ -92,9 +93,11 @@ impl AsyncModule {

let reference_idents = self
.references
.await?
.iter()
.map(|r| async {
let referenced_asset = r.get_referenced_asset().await?;
let referenced_asset =
ReferencedAsset::from_resolve_result(r.resolve_reference()).await?;
Ok(match &*referenced_asset {
ReferencedAsset::External(
_,
Expand Down Expand Up @@ -140,9 +143,11 @@ impl AsyncModule {
self.import_externals
&& self
.references
.await?
.iter()
.map(|r| async {
let referenced_asset = r.get_referenced_asset().await?;
let referenced_asset =
ReferencedAsset::from_resolve_result(r.resolve_reference()).await?;
Ok(matches!(
&*referenced_asset,
ReferencedAsset::External(
Expand Down
2 changes: 1 addition & 1 deletion crates/turbopack-ecmascript/src/references/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -736,7 +736,7 @@ 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: import_references.iter().copied().collect(),
references: Vc::cell(import_references.iter().map(|&r| Vc::upcast(r)).collect()),
has_top_level_await,
import_externals,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ use crate::{

/// The chunk item for [EcmascriptModuleFacadeModule].
#[turbo_tasks::value(shared)]
pub struct EcmascriptModuleReexportsChunkItem {
pub struct EcmascriptModuleFacadeChunkItem {
pub(crate) module: Vc<EcmascriptModuleFacadeModule>,
pub(crate) chunking_context: Vc<Box<dyn EcmascriptChunkingContext>>,
}

#[turbo_tasks::value_impl]
impl EcmascriptChunkItem for EcmascriptModuleReexportsChunkItem {
impl EcmascriptChunkItem for EcmascriptModuleFacadeChunkItem {
#[turbo_tasks::function]
fn content(self: Vc<Self>) -> Vc<EcmascriptChunkItemContent> {
panic!("content() should never be called");
Expand Down Expand Up @@ -68,8 +68,9 @@ impl EcmascriptChunkItem for EcmascriptModuleReexportsChunkItem {

let mut code = RopeBuilder::default();

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

code_gens.push(
self.module
.async_module()
.code_generation(chunking_context, async_module_info),
);
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 @@ -150,7 +156,7 @@ impl EcmascriptChunkItem for EcmascriptModuleReexportsChunkItem {
}

#[turbo_tasks::value_impl]
impl ChunkItem for EcmascriptModuleReexportsChunkItem {
impl ChunkItem for EcmascriptModuleFacadeChunkItem {
#[turbo_tasks::function]
fn references(&self) -> Vc<ModuleReferences> {
self.module.references()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ use turbopack_core::{
resolve::ModulePart,
};

use super::chunk_item::EcmascriptModuleReexportsChunkItem;
use super::chunk_item::EcmascriptModuleFacadeChunkItem;
use crate::{
chunk::{EcmascriptChunkPlaceable, EcmascriptChunkingContext, EcmascriptExports},
references::{
async_module::OptionAsyncModule,
async_module::{AsyncModule, OptionAsyncModule},
esm::{EsmExport, EsmExports},
},
side_effect_optimization::reference::EcmascriptModulePartReference,
Expand All @@ -38,6 +38,17 @@ impl EcmascriptModuleFacadeModule {
pub fn new(module: Vc<Box<dyn EcmascriptChunkPlaceable>>, ty: Vc<ModulePart>) -> Vc<Self> {
EcmascriptModuleFacadeModule { module, ty }.cell()
}

#[turbo_tasks::function]
pub fn async_module(self: Vc<Self>) -> Vc<AsyncModule> {
AsyncModule {
placeable: Vc::upcast(self),
references: self.references(),
has_top_level_await: false,
import_externals: false,
}
.cell()
}
}

#[turbo_tasks::value_impl]
Expand Down Expand Up @@ -247,8 +258,8 @@ impl EcmascriptChunkPlaceable for EcmascriptModuleFacadeModule {
}

#[turbo_tasks::function]
fn get_async_module(&self) -> Vc<OptionAsyncModule> {
self.module.get_async_module()
fn get_async_module(self: Vc<Self>) -> Vc<OptionAsyncModule> {
Vc::cell(Some(self.async_module()))
}
}

Expand All @@ -267,7 +278,7 @@ impl ChunkableModule for EcmascriptModuleFacadeModule {
EcmascriptModuleFacadeModule",
)?;
Ok(Vc::upcast(
EcmascriptModuleReexportsChunkItem {
EcmascriptModuleFacadeChunkItem {
module: self,
chunking_context,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,4 +100,13 @@ impl ChunkItem for EcmascriptModuleLocalsChunkItem {
fn module(&self) -> Vc<Box<dyn Module>> {
Vc::upcast(self.module)
}

#[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())
} else {
Ok(Vc::cell(false))
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@ use turbopack_core::{
use super::chunk_item::EcmascriptModuleLocalsChunkItem;
use crate::{
chunk::{EcmascriptChunkPlaceable, EcmascriptChunkingContext, EcmascriptExports},
references::esm::{EsmExport, EsmExports},
references::{
async_module::OptionAsyncModule,
esm::{EsmExport, EsmExports},
},
EcmascriptModuleAsset,
};

Expand Down Expand Up @@ -94,6 +97,11 @@ impl EcmascriptChunkPlaceable for EcmascriptModuleLocalsModule {
fn is_marked_as_side_effect_free(&self) -> Vc<bool> {
self.module.is_marked_as_side_effect_free()
}

#[turbo_tasks::function]
fn get_async_module(&self) -> Vc<OptionAsyncModule> {
self.module.get_async_module()
}
}

#[turbo_tasks::value_impl]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ use swc_core::{
};
use turbo_tasks::{ValueToString, Vc};
use turbopack_core::{
chunk::{ChunkItemExt, ChunkableModule, ChunkableModuleReference, ModuleId},
chunk::{
ChunkItemExt, ChunkableModule, ChunkableModuleReference, ChunkingType, ChunkingTypeOption,
ModuleId,
},
reference::ModuleReference,
resolve::{ModulePart, ModuleResolveResult},
};
Expand Down Expand Up @@ -95,7 +98,12 @@ impl ModuleReference for EcmascriptModulePartReference {
}

#[turbo_tasks::value_impl]
impl ChunkableModuleReference for EcmascriptModulePartReference {}
impl ChunkableModuleReference for EcmascriptModulePartReference {
#[turbo_tasks::function]
fn chunking_type(self: Vc<Self>) -> Vc<ChunkingTypeOption> {
Vc::cell(Some(ChunkingType::ParallelInheritAsync))
}
}

#[turbo_tasks::value_impl]
impl CodeGenerateable for EcmascriptModulePartReference {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,3 +87,61 @@ it("should handle globs in sideEffects field", () => {
expect(b9).toBe("b");
expect(effects).toEqual(["file.side.js", "dir/file.js"]);
});

it("should generate a correct facade from async modules", async () => {
expect(await import("tla/local")).toEqual(
expect.objectContaining({
tla: "tla",
reexported: "reexported",
reexported2: "reexported",
})
);
expect(await import("tla/reexport")).toEqual(
expect.objectContaining({
local: "local",
tlaReexported: "tla-reexported",
tlaReexported2: "tla-reexported",
})
);
expect(await import("tla/both")).toEqual(
expect.objectContaining({
tla: "tla",
tlaReexported: "tla-reexported",
tlaReexported2: "tla-reexported",
})
);
});

import * as tlaLocal from "tla/local";
import * as tlaReexport from "tla/reexport";
import * as tlaBoth from "tla/both";
it("should generate a correct namespace object from async modules", async () => {
expect(tlaLocal).toEqual(
expect.objectContaining({
tla: "tla",
reexported: "reexported",
reexported2: "reexported",
})
);
expect(tlaReexport).toEqual(
expect.objectContaining({
local: "local",
tlaReexported: "tla-reexported",
tlaReexported2: "tla-reexported",
})
);
expect(tlaBoth).toEqual(
expect.objectContaining({
tla: "tla",
tlaReexported: "tla-reexported",
tlaReexported2: "tla-reexported",
})
);
});

import { tlaReexported2 as tlaReexported } from "tla/reexport";
import { tlaReexported2 as tlaReexportedBoth } from "tla/both";
it("should generate correct renaming facades from async modules", async () => {
expect(tlaReexported).toBe("tla-reexported");
expect(tlaReexportedBoth).toBe("tla-reexported");
});

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 6c57d42

Please sign in to comment.