improve dynamic reexporting (#5452)

### Description

This fixes some problems when `export * from "client component"`, as
`"use client"` creates a non-enumerable Proxy object, so we need to
really redirect property access.

* `__turbopack_export_namespace__` counts as dynamic
* use Proxy to correctly reexport non-enumerable or changing objects

---------

Co-authored-by: Alex Kirszenberg <alex.kirszenberg@vercel.com>

crates/turbopack-ecmascript-runtime/js/src/build/runtime.ts

@@ -37,7 +37,7 @@ interface TurbopackNodeBuildContext {
   f: RequireContextFactory;
   i: EsmImport;
   s: EsmExport;
-  j: typeof cjsExport;
+  j: typeof dynamicExport;
   v: ExportValue;
   n: typeof exportNamespace;
   m: Module;
@@ -182,7 +182,7 @@ function instantiateModule(id: ModuleId, source: SourceInfo): Module {
       f: requireContext.bind(null, module),
       i: esmImport.bind(null, module),
       s: esm.bind(null, module.exports),
-      j: cjsExport.bind(null, module.exports),
+      j: dynamicExport.bind(null, module),
       v: exportValue.bind(null, module),
       n: exportNamespace.bind(null, module),
       m: module,

crates/turbopack-ecmascript-runtime/js/src/dev/runtime/base/runtime-base.ts

@@ -38,7 +38,7 @@ interface TurbopackDevBaseContext {
   f: RequireContextFactory;
   i: EsmImport;
   s: EsmExport;
-  j: typeof cjsExport;
+  j: typeof dynamicExport;
   v: ExportValue;
   n: typeof exportNamespace;
   m: Module;
@@ -337,7 +337,7 @@ function instantiateModule(id: ModuleId, source: SourceInfo): Module {
           f: requireContext.bind(null, module),
           i: esmImport.bind(null, module),
           s: esmExport.bind(null, module),
-          j: cjsExport.bind(null, module.exports),
+          j: dynamicExport.bind(null, module),
           v: exportValue.bind(null, module),
           n: exportNamespace.bind(null, module),
           m: module,

crates/turbopack-ecmascript-runtime/js/src/shared/runtime-utils.ts

@@ -16,6 +16,8 @@ interface Exports {
 }
 type EsmNamespaceObject = Record<string, any>;
 
+const REEXPORTED_OBJECTS = Symbol("reexported objects");
+
 interface BaseModule {
   exports: Exports;
   error: Error | undefined;
@@ -24,6 +26,7 @@ interface BaseModule {
   children: ModuleId[];
   parents: ModuleId[];
   namespaceObject?: EsmNamespaceObject;
+  [REEXPORTED_OBJECTS]?: any[];
 }
 
 interface Module extends BaseModule {}
@@ -81,12 +84,39 @@ function esmExport(module: Module, getters: Record<string, () => any>) {
 }
 
 /**
- * Adds the props to the exports object
+ * Dynamically exports properties from an object
  */
-function cjsExport(exports: Exports, props: Record<string, any>) {
-  for (const key in props) {
-    defineProp(exports, key, { get: () => props[key], enumerable: true });
+function dynamicExport(module: Module, object: Record<string, any>) {
+  let reexportedObjects = module[REEXPORTED_OBJECTS];
+  if (!reexportedObjects) {
+    reexportedObjects = module[REEXPORTED_OBJECTS] = [];
+    module.namespaceObject = new Proxy(module.exports, {
+      get(target, prop) {
+        if (
+          hasOwnProperty.call(target, prop) ||
+          prop === "default" ||
+          prop === "__esModule"
+        ) {
+          return Reflect.get(target, prop);
+        }
+        for (const obj of reexportedObjects!) {
+          const value = Reflect.get(obj, prop);
+          if (value !== undefined) return value;
+        }
+        return undefined;
+      },
+      ownKeys(target) {
+        const keys = Reflect.ownKeys(target);
+        for (const obj of reexportedObjects!) {
+          for (const key of Reflect.ownKeys(obj)) {
+            if (key !== "default" && !keys.includes(key)) keys.push(key);
+          }
+        }
+        return keys;
+      },
+    });
   }
+  reexportedObjects.push(object);
 }
 
 function exportValue(module: Module, value: any) {

crates/turbopack-ecmascript/src/chunk/item.rs

@@ -83,7 +83,7 @@ impl EcmascriptChunkItemContentVc {
             "n: __turbopack_export_namespace__",
             "c: __turbopack_cache__",
             "l: __turbopack_load__",
-            "j: __turbopack_cjs__",
+            "j: __turbopack_dynamic__",
             "g: global",
             // HACK
             "__dirname",

crates/turbopack-ecmascript/src/chunk/placeable.rs

@@ -27,6 +27,7 @@ impl EcmascriptChunkPlaceablesVc {
 #[turbo_tasks::value(shared)]
 pub enum EcmascriptExports {
     EsmExports(EsmExportsVc),
+    DynamicNamespace,
     CommonJs,
     Value,
     None,

crates/turbopack-ecmascript/src/references/esm/export.rs

@@ -41,13 +41,13 @@ pub enum EsmExport {
 #[turbo_tasks::value]
 struct ExpandResults {
     star_exports: Vec<String>,
-    has_cjs_exports: bool,
+    has_dynamic_exports: bool,
 }
 
 #[turbo_tasks::function]
 async fn expand_star_exports(root_asset: EcmascriptChunkPlaceableVc) -> Result<ExpandResultsVc> {
     let mut set = HashSet::new();
-    let mut has_cjs_exports = false;
+    let mut has_dynamic_exports = false;
     let mut checked_assets = HashSet::new();
     checked_assets.insert(root_asset);
     let mut queue = vec![(root_asset, root_asset.get_exports())];
@@ -100,7 +100,7 @@ async fn expand_star_exports(root_asset: EcmascriptChunkPlaceableVc) -> Result<E
             .as_issue()
             .emit(),
             EcmascriptExports::CommonJs => {
-                has_cjs_exports = true;
+                has_dynamic_exports = true;
                 AnalyzeIssue {
                     code: None,
                     category: StringVc::cell("analyze".to_string()),
@@ -120,11 +120,14 @@ async fn expand_star_exports(root_asset: EcmascriptChunkPlaceableVc) -> Result<E
                 .as_issue()
                 .emit()
             }
+            EcmascriptExports::DynamicNamespace => {
+                has_dynamic_exports = true;
+            }
         }
     }
     Ok(ExpandResultsVc::cell(ExpandResults {
         star_exports: set.into_iter().collect(),
-        has_cjs_exports,
+        has_dynamic_exports,
     }))
 }
 
@@ -151,7 +154,7 @@ impl CodeGenerateable for EsmExports {
             .map(|(k, v)| (Cow::<str>::Borrowed(k), Cow::Borrowed(v)))
             .collect();
         let mut props = Vec::new();
-        let mut cjs_exports = Vec::<Box<Expr>>::new();
+        let mut dynamic_exports = Vec::<Box<Expr>>::new();
 
         for esm_ref in this.star_exports.iter() {
             if let ReferencedAsset::Some(asset) = &*esm_ref.get_referenced_asset().await? {
@@ -166,11 +169,11 @@ impl CodeGenerateable for EsmExports {
                     }
                 }
 
-                if export_info.has_cjs_exports {
+                if export_info.has_dynamic_exports {
                     let ident = ReferencedAsset::get_ident_from_placeable(asset).await?;
 
-                    cjs_exports.push(quote_expr!(
-                        "__turbopack_cjs__($arg)",
+                    dynamic_exports.push(quote_expr!(
+                        "__turbopack_dynamic__($arg)",
                         arg: Expr = Ident::new(ident.into(), DUMMY_SP).into()
                     ));
                 }
@@ -230,10 +233,10 @@ impl CodeGenerateable for EsmExports {
             span: DUMMY_SP,
             props,
         });
-        let cjs_stmt = if !cjs_exports.is_empty() {
+        let dynamic_stmt = if !dynamic_exports.is_empty() {
             Some(Stmt::Expr(ExprStmt {
                 span: DUMMY_SP,
-                expr: Expr::from_exprs(cjs_exports),
+                expr: Expr::from_exprs(dynamic_exports),
             }))
         } else {
             None
@@ -251,8 +254,8 @@ impl CodeGenerateable for EsmExports {
                     body.insert(0, stmt);
                 }
             }
-            if let Some(cjs_stmt) = cjs_stmt.clone() {
-                insert_hoisted_stmt(program, cjs_stmt);
+            if let Some(dynamic_stmt) = dynamic_stmt.clone() {
+                insert_hoisted_stmt(program, dynamic_stmt);
             }
         }));
 

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

@@ -588,27 +588,48 @@ pub(crate) async fn analyze_ecmascript_module(
 
         EcmascriptExports::EsmExports(esm_exports)
     } else if matches!(specified_type, SpecifiedModuleType::EcmaScript) {
-        if has_cjs_export(program) {
-            SpecifiedModuleTypeIssue {
-                path: source.ident().path(),
-                specified_type,
+        match detect_dynamic_export(program) {
+            DetectedDynamicExportType::CommonJs => {
+                SpecifiedModuleTypeIssue {
+                    path: source.ident().path(),
+                    specified_type,
+                }
+                .cell()
+                .as_issue()
+                .emit();
+                EcmascriptExports::EsmExports(
+                    EsmExports {
+                        exports: Default::default(),
+                        star_exports: Default::default(),
+                    }
+                    .cell(),
+                )
             }
-            .cell()
-            .as_issue()
-            .emit();
+            DetectedDynamicExportType::Namespace => EcmascriptExports::DynamicNamespace,
+            DetectedDynamicExportType::Value => EcmascriptExports::Value,
+            DetectedDynamicExportType::UsingModuleDeclarations
+            | DetectedDynamicExportType::None => EcmascriptExports::EsmExports(
+                EsmExports {
+                    exports: Default::default(),
+                    star_exports: Default::default(),
+                }
+                .cell(),
+            ),
         }
-
-        EcmascriptExports::EsmExports(
-            EsmExports {
-                exports: Default::default(),
-                star_exports: Default::default(),
-            }
-            .cell(),
-        )
-    } else if has_cjs_export(program) {
-        EcmascriptExports::CommonJs
     } else {
-        EcmascriptExports::None
+        match detect_dynamic_export(program) {
+            DetectedDynamicExportType::CommonJs => EcmascriptExports::CommonJs,
+            DetectedDynamicExportType::Namespace => EcmascriptExports::DynamicNamespace,
+            DetectedDynamicExportType::Value => EcmascriptExports::Value,
+            DetectedDynamicExportType::UsingModuleDeclarations => EcmascriptExports::EsmExports(
+                EsmExports {
+                    exports: Default::default(),
+                    star_exports: Default::default(),
+                }
+                .cell(),
+            ),
+            DetectedDynamicExportType::None => EcmascriptExports::None,
+        }
     };
 
     analysis.set_exports(exports);
@@ -2553,18 +2574,27 @@ pub struct AstPath(#[turbo_tasks(trace_ignore)] Vec<AstParentKind>);
 pub static TURBOPACK_HELPER: &str = "__turbopackHelper";
 
 pub fn is_turbopack_helper_import(import: &ImportDecl) -> bool {
-    import.asserts.as_ref().map_or(true, |asserts| {
+    import.asserts.as_ref().map_or(false, |asserts| {
         asserts.props.iter().any(|assert| {
             assert
                 .as_prop()
                 .and_then(|prop| prop.as_key_value())
                 .and_then(|kv| kv.key.as_ident())
-                .map_or(true, |ident| &*ident.sym != TURBOPACK_HELPER)
+                .map_or(false, |ident| &*ident.sym == TURBOPACK_HELPER)
         })
     })
 }
 
-fn has_cjs_export(p: &Program) -> bool {
+#[derive(Debug)]
+enum DetectedDynamicExportType {
+    CommonJs,
+    Namespace,
+    Value,
+    None,
+    UsingModuleDeclarations,
+}
+
+fn detect_dynamic_export(p: &Program) -> DetectedDynamicExportType {
     use swc_core::ecma::visit::{visit_obj_and_computed, Visit, VisitWith};
 
     if let Program::Module(m) = p {
@@ -2576,22 +2606,35 @@ fn has_cjs_export(p: &Program) -> bool {
                     .map_or(true, |import| !is_turbopack_helper_import(import))
             })
         }) {
-            return false;
+            return DetectedDynamicExportType::UsingModuleDeclarations;
         }
     }
 
     struct Visitor {
+        cjs: bool,
+        value: bool,
+        namespace: bool,
         found: bool,
     }
 
     impl Visit for Visitor {
         visit_obj_and_computed!();
 
         fn visit_ident(&mut self, i: &Ident) {
-            if &*i.sym == "module"
-                || &*i.sym == "exports"
-                || &*i.sym == "__turbopack_export_value__"
-            {
+            // The detection is not perfect, it might have some false positives, e. g. in
+            // cases where `module` is used in some other way. e. g. `const module = 42;`.
+            // But a false positive doesn't break anything, it only opts out of some
+            // optimizations, which is acceptable.
+            if &*i.sym == "module" || &*i.sym == "exports" {
+                self.cjs = true;
+                self.found = true;
+            }
+            if &*i.sym == "__turbopack_export_value__" {
+                self.value = true;
+                self.found = true;
+            }
+            if &*i.sym == "__turbopack_export_namespace__" {
+                self.namespace = true;
                 self.found = true;
             }
         }
@@ -2610,7 +2653,20 @@ fn has_cjs_export(p: &Program) -> bool {
         }
     }
 
-    let mut v = Visitor { found: false };
+    let mut v = Visitor {
+        cjs: false,
+        value: false,
+        namespace: false,
+        found: false,
+    };
     p.visit_with(&mut v);
-    v.found
+    if v.cjs {
+        DetectedDynamicExportType::CommonJs
+    } else if v.value {
+        DetectedDynamicExportType::Value
+    } else if v.namespace {
+        DetectedDynamicExportType::Namespace
+    } else {
+        DetectedDynamicExportType::None
+    }
 }