Turbopack: correct export usage in EcmascriptModulePartReference

turbopack/crates/turbopack-ecmascript/src/side_effect_optimization/facade/module.rs

@@ -34,22 +34,22 @@ use crate::{
 /// [EcmascriptModuleLocalsModule]. It allows to follow
 #[turbo_tasks::value]
 pub struct EcmascriptModuleFacadeModule {
-    pub module: ResolvedVc<Box<dyn EcmascriptChunkPlaceable>>,
-    pub ty: ModulePart,
-    pub remove_unused_exports: bool,
+    module: ResolvedVc<Box<dyn EcmascriptChunkPlaceable>>,
+    part: ModulePart,
+    remove_unused_exports: bool,
 }
 
 #[turbo_tasks::value_impl]
 impl EcmascriptModuleFacadeModule {
     #[turbo_tasks::function]
     pub fn new(
         module: ResolvedVc<Box<dyn EcmascriptChunkPlaceable>>,
-        ty: ModulePart,
+        part: ModulePart,
         remove_unused_exports: bool,
     ) -> Vc<Self> {
         EcmascriptModuleFacadeModule {
             module,
-            ty,
+            part,
             remove_unused_exports,
         }
         .cell()
@@ -81,7 +81,7 @@ impl EcmascriptModuleFacadeModule {
         ResolvedVc<EsmAssetReferences>,
         Vec<ResolvedVc<EcmascriptModulePartReference>>,
     )> {
-        Ok(match &self.ty {
+        Ok(match &self.part {
             ModulePart::Evaluation => {
                 let Some(module) =
                     ResolvedVc::try_sidecast::<Box<dyn EcmascriptAnalyzable>>(self.module)
@@ -150,15 +150,15 @@ impl EcmascriptModuleFacadeModule {
             ModulePart::RenamedNamespace { .. } => (
                 EsmAssetReferences::empty().to_resolved().await?,
                 vec![
-                    EcmascriptModulePartReference::new(*self.module, self.remove_unused_exports)
+                    EcmascriptModulePartReference::new_normal(*self.module, self.part.clone())
                         .to_resolved()
                         .await?,
                 ],
             ),
             ModulePart::RenamedExport { .. } => (
                 EsmAssetReferences::empty().to_resolved().await?,
                 vec![
-                    EcmascriptModulePartReference::new(*self.module, self.remove_unused_exports)
+                    EcmascriptModulePartReference::new_normal(*self.module, self.part.clone())
                         .to_resolved()
                         .await?,
                 ],
@@ -174,7 +174,7 @@ impl EcmascriptModuleFacadeModule {
 impl Module for EcmascriptModuleFacadeModule {
     #[turbo_tasks::function]
     fn ident(&self) -> Vc<AssetIdent> {
-        self.module.ident().with_part(self.ty.clone())
+        self.module.ident().with_part(self.part.clone())
     }
 
     #[turbo_tasks::function]
@@ -275,7 +275,7 @@ impl EcmascriptChunkPlaceable for EcmascriptModuleFacadeModule {
         let mut exports = BTreeMap::new();
         let mut star_exports = Vec::new();
 
-        match &self.ty {
+        match &self.part {
             ModulePart::Exports => {
                 let EcmascriptExports::EsmExports(esm_exports) = *self.module.get_exports().await?
                 else {
@@ -371,9 +371,9 @@ impl EcmascriptChunkPlaceable for EcmascriptModuleFacadeModule {
                     export.clone(),
                     EsmExport::ImportedBinding(
                         ResolvedVc::upcast(
-                            EcmascriptModulePartReference::new(
+                            EcmascriptModulePartReference::new_normal(
                                 *self.module,
-                                self.remove_unused_exports,
+                                self.part.clone(),
                             )
                             .to_resolved()
                             .await?,
@@ -387,12 +387,9 @@ impl EcmascriptChunkPlaceable for EcmascriptModuleFacadeModule {
                 exports.insert(
                     export.clone(),
                     EsmExport::ImportedNamespace(ResolvedVc::upcast(
-                        EcmascriptModulePartReference::new(
-                            *self.module,
-                            self.remove_unused_exports,
-                        )
-                        .to_resolved()
-                        .await?,
+                        EcmascriptModulePartReference::new_normal(*self.module, self.part.clone())
+                            .to_resolved()
+                            .await?,
                     )),
                 );
             }
@@ -415,7 +412,7 @@ impl EcmascriptChunkPlaceable for EcmascriptModuleFacadeModule {
         &self,
         side_effect_free_packages: Vc<Glob>,
     ) -> Result<Vc<bool>> {
-        Ok(match self.ty {
+        Ok(match self.part {
             ModulePart::Evaluation | ModulePart::Facade => self
                 .module
                 .is_marked_as_side_effect_free(side_effect_free_packages),

turbopack/crates/turbopack-ecmascript/src/side_effect_optimization/reference.rs

@@ -1,7 +1,8 @@
 use anyhow::{Context, Result, bail};
+use serde::{Deserialize, Serialize};
 use swc_core::{common::DUMMY_SP, ecma::ast::Ident, quote};
-use turbo_rcstr::{RcStr, rcstr};
-use turbo_tasks::{ResolvedVc, ValueToString, Vc};
+use turbo_rcstr::RcStr;
+use turbo_tasks::{NonLocalValue, ResolvedVc, ValueToString, Vc, trace::TraceRawVcs};
 use turbopack_core::{
     chunk::{
         ChunkableModuleReference, ChunkingContext, ChunkingType, ChunkingTypeOption,
@@ -21,17 +22,24 @@ use crate::{
     utils::module_id_to_lit,
 };
 
+#[derive(Debug, Clone, Eq, PartialEq, Hash, Serialize, Deserialize, NonLocalValue, TraceRawVcs)]
+enum EcmascriptModulePartReferenceMode {
+    Synthesize { remove_unused_exports: bool },
+    Normal,
+}
+
 /// A reference to the [EcmascriptModuleLocalsModule] variant of an original
 /// module.
 #[turbo_tasks::value]
 pub struct EcmascriptModulePartReference {
-    pub module: ResolvedVc<Box<dyn EcmascriptChunkPlaceable>>,
-    pub part: Option<ModulePart>,
-    pub remove_unused_exports: bool,
+    module: ResolvedVc<Box<dyn EcmascriptChunkPlaceable>>,
+    part: ModulePart,
+    mode: EcmascriptModulePartReferenceMode,
 }
 
 #[turbo_tasks::value_impl]
 impl EcmascriptModulePartReference {
+    // Create new [EcmascriptModuleFacadeModule]s as necessary
     #[turbo_tasks::function]
     pub fn new_part(
         module: ResolvedVc<Box<dyn EcmascriptChunkPlaceable>>,
@@ -40,21 +48,24 @@ impl EcmascriptModulePartReference {
     ) -> Vc<Self> {
         EcmascriptModulePartReference {
             module,
-            part: Some(part),
-            remove_unused_exports,
+            part,
+            mode: EcmascriptModulePartReferenceMode::Synthesize {
+                remove_unused_exports,
+            },
         }
         .cell()
     }
 
+    // A reference to the given module, without any intermediary synthesized modules.
     #[turbo_tasks::function]
-    pub fn new(
+    pub fn new_normal(
         module: ResolvedVc<Box<dyn EcmascriptChunkPlaceable>>,
-        remove_unused_exports: bool,
+        part: ModulePart,
     ) -> Vc<Self> {
         EcmascriptModulePartReference {
             module,
-            part: None,
-            remove_unused_exports,
+            part,
+            mode: EcmascriptModulePartReferenceMode::Normal,
         }
         .cell()
     }
@@ -63,51 +74,51 @@ impl EcmascriptModulePartReference {
 #[turbo_tasks::value_impl]
 impl ValueToString for EcmascriptModulePartReference {
     #[turbo_tasks::function]
-    async fn to_string(&self) -> Result<Vc<RcStr>> {
-        Ok(match &self.part {
-            Some(part) => Vc::cell(part.to_string().into()),
-            None => Vc::cell(rcstr!("module")),
-        })
+    async fn to_string(&self) -> Vc<RcStr> {
+        Vc::cell(self.part.to_string().into())
     }
 }
 
 #[turbo_tasks::value_impl]
 impl ModuleReference for EcmascriptModulePartReference {
     #[turbo_tasks::function]
     async fn resolve_reference(&self) -> Result<Vc<ModuleResolveResult>> {
-        let module = if let Some(part) = &self.part {
-            match part {
-                ModulePart::Locals => {
-                    let Some(module) = ResolvedVc::try_downcast_type(self.module) else {
+        let module = match self.mode {
+            EcmascriptModulePartReferenceMode::Synthesize {
+                remove_unused_exports,
+            } => {
+                match &self.part {
+                    ModulePart::Locals => {
+                        let Some(module) = ResolvedVc::try_downcast_type(self.module) else {
+                            bail!(
+                                "Expected EcmascriptModuleAsset for a \
+                                 EcmascriptModulePartReference with ModulePart::Locals"
+                            );
+                        };
+                        Vc::upcast::<Box<dyn Module>>(EcmascriptModuleLocalsModule::new(*module))
+                    }
+                    ModulePart::Exports
+                    | ModulePart::Evaluation
+                    | ModulePart::Facade
+                    | ModulePart::RenamedExport { .. }
+                    | ModulePart::RenamedNamespace { .. } => {
+                        Vc::upcast(EcmascriptModuleFacadeModule::new(
+                            *self.module,
+                            self.part.clone(),
+                            remove_unused_exports,
+                        ))
+                    }
+                    ModulePart::Export(..) | ModulePart::Internal(..) => {
                         bail!(
-                            "Expected EcmascriptModuleAsset for a EcmascriptModulePartReference \
-                             with ModulePart::Locals"
+                            "Unexpected ModulePart \"{}\" for EcmascriptModulePartReference",
+                            self.part
                         );
-                    };
-                    Vc::upcast::<Box<dyn Module>>(EcmascriptModuleLocalsModule::new(*module))
-                }
-                ModulePart::Exports
-                | ModulePart::Evaluation
-                | ModulePart::Facade
-                | ModulePart::RenamedExport { .. }
-                | ModulePart::RenamedNamespace { .. } => {
-                    Vc::upcast(EcmascriptModuleFacadeModule::new(
-                        *self.module,
-                        part.clone(),
-                        self.remove_unused_exports,
-                    ))
-                }
-                ModulePart::Export(..) | ModulePart::Internal(..) => {
-                    bail!(
-                        "Unexpected ModulePart \"{}\" for EcmascriptModulePartReference",
-                        part
-                    );
+                    }
                 }
+                .to_resolved()
+                .await?
             }
-            .to_resolved()
-            .await?
-        } else {
-            ResolvedVc::upcast(self.module)
+            EcmascriptModulePartReferenceMode::Normal => ResolvedVc::upcast(self.module),
         };
 
         Ok(*ModuleResolveResult::module(module))
@@ -127,8 +138,11 @@ impl ChunkableModuleReference for EcmascriptModulePartReference {
     #[turbo_tasks::function]
     fn export_usage(&self) -> Vc<ExportUsage> {
         match &self.part {
-            Some(ModulePart::Export(export)) => ExportUsage::named(export.clone()),
-            Some(ModulePart::Evaluation) => ExportUsage::evaluation(),
+            ModulePart::Export(export) => ExportUsage::named(export.clone()),
+            ModulePart::RenamedExport {
+                original_export, ..
+            } => ExportUsage::named(original_export.clone()),
+            ModulePart::Evaluation => ExportUsage::evaluation(),
             _ => ExportUsage::all(),
         }
     }

turbopack/crates/turbopack-tests/tests/execution.rs

@@ -241,8 +241,7 @@ async fn run_inner_operation(
 #[serde(rename_all = "camelCase", deny_unknown_fields)]
 struct TestOptions {
     tree_shaking_mode: Option<TreeShakingMode>,
-    #[serde(default)]
-    remove_unused_exports: bool,
+    remove_unused_exports: Option<bool>,
 }
 
 #[turbo_tasks::value]
@@ -361,6 +360,8 @@ async fn run_test_operation(prepared_test: ResolvedVc<PreparedTest>) -> Result<V
             .resolved_cell(),
     );
 
+    let remove_unused_exports = options.remove_unused_exports.unwrap_or(true);
+
     let asset_context: Vc<Box<dyn AssetContext>> = Vc::upcast(ModuleAssetContext::new(
         Default::default(),
         compile_time_info,
@@ -378,12 +379,12 @@ async fn run_test_operation(prepared_test: ResolvedVc<PreparedTest>) -> Result<V
                 ContextCondition::InDirectory("node_modules".into()),
                 ModuleOptionsContext {
                     tree_shaking_mode: options.tree_shaking_mode,
-                    remove_unused_exports: options.remove_unused_exports,
+                    remove_unused_exports,
                     ..Default::default()
                 }
                 .resolved_cell(),
             )],
-            remove_unused_exports: options.remove_unused_exports,
+            remove_unused_exports,
             ..Default::default()
         }
         .into(),

turbopack/crates/turbopack-tests/tests/snapshot.rs

@@ -308,10 +308,12 @@ async fn run_test_operation(resource: RcStr) -> Result<Vc<FileSystemPath>> {
             rules: vec![(
                 ContextCondition::InDirectory("node_modules".into()),
                 ModuleOptionsContext {
-                    remove_unused_exports: options.remove_unused_exports,
                     css: CssOptionsContext {
                         ..Default::default()
                     },
+                    preset_env_versions: Some(env),
+                    tree_shaking_mode: options.tree_shaking_mode,
+                    remove_unused_exports: options.remove_unused_exports,
                     ..Default::default()
                 }
                 .resolved_cell(),

turbopack/crates/turbopack-tests/tests/snapshot/intermediate-tree-shake/rename-side-effect-free-facade/input/index.js

@@ -0,0 +1,3 @@
+import { a0 } from 'lib'
+
+console.log(a0)

turbopack/crates/turbopack-tests/tests/snapshot/intermediate-tree-shake/rename-side-effect-free-facade/options.json

@@ -0,0 +1,4 @@
+{
+  "treeShakingMode": "reexports-only",
+  "removeUnusedExports": true
+}

turbopack/crates/turbopack-tests/tests/snapshot/intermediate-tree-shake/rename-side-effect-free-facade/output/2b719_intermediate-tree-shake_rename-side-effect-free-facade_input_index_e208a8b3.js

@@ -0,0 +1,6 @@
+(globalThis.TURBOPACK = globalThis.TURBOPACK || []).push([
+    "output/2b719_intermediate-tree-shake_rename-side-effect-free-facade_input_index_e208a8b3.js",
+    {},
+    {"otherChunks":["output/457d9_snapshot_intermediate-tree-shake_rename-side-effect-free-facade_input_71bae14d._.js"],"runtimeModuleIds":["[project]/turbopack/crates/turbopack-tests/tests/snapshot/intermediate-tree-shake/rename-side-effect-free-facade/input/index.js [test] (ecmascript)"]}
+]);
+// Dummy runtime