fix(turbopack): Split ModulePart into eval vs non-eval (#70496)

Treat `baz` in the code below as a re-export.

```js
import { baz } from '../actions'

// Ensure side effects won't affect tree shaking and DCE
console.log(1)

export { baz }
```

It's required to fix tree shaking for client bundles.

x-ref: https://vercel.slack.com/archives/C07GX6S95UG/p1727270139934499

---------

Co-authored-by: Tobias Koppers <tobias.koppers@googlemail.com>

turbopack/crates/turbopack-core/src/resolve/mod.rs

@@ -2940,7 +2940,7 @@ impl ValueToString for ModulePart {
             ModulePart::RenamedNamespace { export } => {
                 format!("export * as {}", export.await?).into()
             }
-            ModulePart::Internal(id) => format!("internal part {}", id).into(),
+            ModulePart::Internal(id) => format!("internal part {}", id,).into(),
             ModulePart::Locals => "locals".into(),
             ModulePart::Exports => "exports".into(),
             ModulePart::Facade => "facade".into(),

turbopack/crates/turbopack-ecmascript/src/analyzer/imports.rs

@@ -201,6 +201,7 @@ pub(crate) enum ImportedSymbol {
     Symbol(JsWord),
     Exports,
     Part(u32),
+    PartEvaluation(u32),
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
@@ -373,7 +374,6 @@ impl Visit for Analyzer<'_> {
 
             self.data.imports.insert(local, (i, orig_sym));
         }
-
         if import.specifiers.is_empty() {
             if let Some(internal_symbol) = internal_symbol {
                 self.ensure_reference(
@@ -587,7 +587,8 @@ pub(crate) fn orig_name(n: &ModuleExportName) -> JsWord {
 
 fn parse_with(with: Option<&ObjectLit>) -> Option<ImportedSymbol> {
     find_turbopack_part_id_in_asserts(with?).map(|v| match v {
-        PartId::Internal(index) => ImportedSymbol::Part(index),
+        PartId::Internal(index, true) => ImportedSymbol::PartEvaluation(index),
+        PartId::Internal(index, false) => ImportedSymbol::Part(index),
         PartId::ModuleEvaluation => ImportedSymbol::ModuleEvaluation,
         PartId::Export(e) => ImportedSymbol::Symbol(e.as_str().into()),
         PartId::Exports => ImportedSymbol::Exports,

turbopack/crates/turbopack-ecmascript/src/references/mod.rs

@@ -599,10 +599,11 @@ pub(crate) async fn analyse_ecmascript_module_internal(
                         Some(ModulePart::evaluation())
                     }
                     ImportedSymbol::Symbol(name) => Some(ModulePart::export((&**name).into())),
-                    ImportedSymbol::Part(part_id) => {
+                    ImportedSymbol::PartEvaluation(part_id) => {
                         evaluation_references.push(i);
                         Some(ModulePart::internal(*part_id))
                     }
+                    ImportedSymbol::Part(part_id) => Some(ModulePart::internal(*part_id)),
                     ImportedSymbol::Exports => Some(ModulePart::exports()),
                 },
                 Some(TreeShakingMode::ReexportsOnly) => match &r.imported_symbol {
@@ -611,7 +612,7 @@ pub(crate) async fn analyse_ecmascript_module_internal(
                         Some(ModulePart::evaluation())
                     }
                     ImportedSymbol::Symbol(name) => Some(ModulePart::export((&**name).into())),
-                    ImportedSymbol::Part(_) => {
+                    ImportedSymbol::PartEvaluation(_) | ImportedSymbol::Part(_) => {
                         bail!("Internal imports doesn't exist in reexports only mode")
                     }
                     ImportedSymbol::Exports => None,

turbopack/crates/turbopack-ecmascript/src/tree_shake/asset.rs

@@ -5,7 +5,7 @@ use turbopack_core::{
     chunk::{AsyncModuleInfo, ChunkableModule, ChunkingContext, EvaluatableAsset},
     ident::AssetIdent,
     module::Module,
-    reference::{ModuleReferences, SingleModuleReference},
+    reference::{ModuleReference, ModuleReferences, SingleModuleReference},
     resolve::ModulePart,
 };
 
@@ -143,9 +143,20 @@ impl Module for EcmascriptModulePartAsset {
             SplitResult::Failed { .. } => return Ok(analyze.references),
         };
 
+        let part_dep = |part: Vc<ModulePart>| -> Vc<Box<dyn ModuleReference>> {
+            Vc::upcast(SingleModuleReference::new(
+                Vc::upcast(EcmascriptModulePartAsset::new(self.full_module, part)),
+                Vc::cell("ecmascript module part".into()),
+            ))
+        };
+
+        let mut references = analyze.references.await?.to_vec();
+
         // Facade depends on evaluation and re-exports
-        if matches!(&*self.part.await?, ModulePart::Facade | ModulePart::Exports) {
-            return Ok(analyze.references);
+        if matches!(&*self.part.await?, ModulePart::Facade) {
+            references.push(part_dep(ModulePart::evaluation()));
+            references.push(part_dep(ModulePart::exports()));
+            return Ok(Vc::cell(references));
         }
 
         let deps = {
@@ -159,28 +170,24 @@ impl Module for EcmascriptModulePartAsset {
             }
         };
 
-        let mut assets = deps
-            .iter()
-            .map(|part_id| {
-                Ok(Vc::upcast(SingleModuleReference::new(
-                    Vc::upcast(EcmascriptModulePartAsset::new(
-                        self.full_module,
-                        match part_id {
-                            PartId::Internal(part_id) => ModulePart::internal(*part_id),
-                            PartId::Export(name) => ModulePart::export(name.clone()),
-                            _ => unreachable!(
-                                "PartId other than Internal and Export should not be used here"
-                            ),
-                        },
-                    )),
-                    Vc::cell("ecmascript module part".into()),
-                )))
-            })
-            .collect::<Result<Vec<_>>>()?;
-
-        assets.extend(analyze.references.await?.iter().cloned());
-
-        Ok(Vc::cell(assets))
+        references.extend(
+            deps.iter()
+                .filter_map(|part_id| {
+                    Some(part_dep(match part_id {
+                        // This is an internal part that is not for evaluation, so we don't need to
+                        // force-add it.
+                        PartId::Internal(.., false) => return None,
+                        PartId::Internal(part_id, true) => ModulePart::internal(*part_id),
+                        PartId::Export(name) => ModulePart::export(name.clone()),
+                        _ => unreachable!(
+                            "PartId other than Internal and Export should not be used here"
+                        ),
+                    }))
+                })
+                .collect::<Vec<_>>(),
+        );
+
+        Ok(Vc::cell(references))
     }
 }
 

turbopack/crates/turbopack-ecmascript/src/tree_shake/graph.rs

@@ -301,6 +301,7 @@ impl DepGraph {
                 body: directives.to_vec(),
                 shebang: None,
             };
+            let mut part_deps_done = FxHashSet::default();
 
             let mut required_vars = group
                 .iter()
@@ -369,25 +370,6 @@ impl DepGraph {
                 }
             }
 
-            // Depend on direct dependencies so that they are executed before this module.
-            for dep in groups
-                .idx_graph
-                .neighbors_directed(ix as u32, petgraph::Direction::Outgoing)
-            {
-                chunk
-                    .body
-                    .push(ModuleItem::ModuleDecl(ModuleDecl::Import(ImportDecl {
-                        span: DUMMY_SP,
-                        specifiers: vec![],
-                        src: Box::new(TURBOPACK_PART_IMPORT_SOURCE.into()),
-                        type_only: false,
-                        with: Some(Box::new(create_turbopack_part_id_assert(PartId::Internal(
-                            dep,
-                        )))),
-                        phase: Default::default(),
-                    })));
-            }
-
             // Workaround for implcit export issue of server actions.
             //
             // Inline server actions require the generated `$$RSC_SERVER_0` to be **exported**.
@@ -473,10 +455,12 @@ impl DepGraph {
                     is_type_only: false,
                 })];
 
+                part_deps_done.insert(dep);
+
                 part_deps
                     .entry(ix as u32)
                     .or_default()
-                    .push(PartId::Internal(dep));
+                    .push(PartId::Internal(dep, false));
 
                 chunk
                     .body
@@ -486,7 +470,35 @@ impl DepGraph {
                         src: Box::new(TURBOPACK_PART_IMPORT_SOURCE.into()),
                         type_only: false,
                         with: Some(Box::new(create_turbopack_part_id_assert(PartId::Internal(
-                            dep,
+                            dep, false,
+                        )))),
+                        phase: Default::default(),
+                    })));
+            }
+
+            // Depend on direct dependencies so that they are executed before this module.
+            for dep in groups
+                .idx_graph
+                .neighbors_directed(ix as u32, petgraph::Direction::Outgoing)
+            {
+                if !part_deps_done.insert(dep) {
+                    continue;
+                }
+
+                part_deps
+                    .entry(ix as u32)
+                    .or_default()
+                    .push(PartId::Internal(dep, true));
+
+                chunk
+                    .body
+                    .push(ModuleItem::ModuleDecl(ModuleDecl::Import(ImportDecl {
+                        span: DUMMY_SP,
+                        specifiers: vec![],
+                        src: Box::new(TURBOPACK_PART_IMPORT_SOURCE.into()),
+                        type_only: false,
+                        with: Some(Box::new(create_turbopack_part_id_assert(PartId::Internal(
+                            dep, true,
                         )))),
                         phase: Default::default(),
                     })));
@@ -1290,7 +1302,8 @@ pub(crate) enum PartId {
     ModuleEvaluation,
     Exports,
     Export(RcStr),
-    Internal(u32),
+    /// `(part_id, is_for_eval)`
+    Internal(u32, bool),
 }
 
 pub(crate) fn create_turbopack_part_id_assert(dep: PartId) -> ObjectLit {
@@ -1303,7 +1316,15 @@ pub(crate) fn create_turbopack_part_id_assert(dep: PartId) -> ObjectLit {
                 PartId::ModuleEvaluation => "module evaluation".into(),
                 PartId::Exports => "exports".into(),
                 PartId::Export(e) => format!("export {e}").into(),
-                PartId::Internal(dep) => (dep as f64).into(),
+                PartId::Internal(dep, is_for_eval) => {
+                    let v = dep as f64;
+                    if is_for_eval {
+                        v
+                    } else {
+                        -v
+                    }
+                }
+                .into(),
             },
         })))],
     }
@@ -1314,7 +1335,10 @@ pub(crate) fn find_turbopack_part_id_in_asserts(asserts: &ObjectLit) -> Option<P
         PropOrSpread::Prop(box Prop::KeyValue(KeyValueProp {
             key: PropName::Ident(key),
             value: box Expr::Lit(Lit::Num(chunk_id)),
-        })) if &*key.sym == ASSERT_CHUNK_KEY => Some(PartId::Internal(chunk_id.value as u32)),
+        })) if &*key.sym == ASSERT_CHUNK_KEY => Some(PartId::Internal(
+            chunk_id.value.abs() as u32,
+            chunk_id.value.is_sign_positive(),
+        )),
 
         PropOrSpread::Prop(box Prop::KeyValue(KeyValueProp {
             key: PropName::Ident(key),

turbopack/crates/turbopack-ecmascript/src/tree_shake/merge.rs

@@ -53,7 +53,7 @@ where
                         .as_deref()
                         .and_then(find_turbopack_part_id_in_asserts);
 
-                    if let Some(PartId::Internal(part_id)) = part_id {
+                    if let Some(PartId::Internal(part_id, _)) = part_id {
                         if self.done.insert((import.src.value.clone(), part_id)) {
                             if let Some(dep) = self.loader.load(&import.src.value, part_id)? {
                                 let mut dep = self.merge_recursively(dep)?;