[turbopack] Optimize ESM exports

test/development/acceptance-app/error-recovery.test.ts

@@ -302,7 +302,7 @@ describe('Error recovery app', () => {
            "Index.useCallback[increment] index.js (7:11)",
            "button <anonymous>",
            "Index index.js (12:7)",
-           "Page index.js (8:16)",
+           "Page index.js (10:5)",
          ],
        }
       `)

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

@@ -36,6 +36,9 @@ const getOrInstantiateModuleFromParent: GetOrInstantiateModuleFromParent<
   const module = moduleCache[id]
 
   if (module) {
+    if (module.error) {
+      throw module.error
+    }
     return module
   }
 
@@ -68,7 +71,6 @@ function instantiateModule(
     throw error
   }
 
-  module.loaded = true
   if (module.namespaceObject && module.exports !== module.namespaceObject) {
     // in case of a circular dependency: cjs1 -> esm2 -> cjs1
     interopEsm(module.exports, module.namespaceObject)

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

@@ -148,6 +148,10 @@ const getOrInstantiateModuleFromParent: GetOrInstantiateModuleFromParent<
   }
 
   if (module) {
+    if (module.error) {
+      throw module.error
+    }
+
     if (module.parents.indexOf(sourceModule.id) === -1) {
       module.parents.push(sourceModule.id)
     }
@@ -234,7 +238,6 @@ function instantiateModule(
     throw error
   }
 
-  module.loaded = true
   if (module.namespaceObject && module.exports !== module.namespaceObject) {
     // in case of a circular dependency: cjs1 -> esm2 -> cjs1
     interopEsm(module.exports, module.namespaceObject)

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

@@ -115,7 +115,7 @@ function factoryNotAvailable(
   moduleId: ModuleId,
   sourceType: SourceType,
   sourceData: SourceData
-) {
+): never {
   let instantiationReason
   switch (sourceType) {
     case SourceType.Runtime:

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

@@ -296,6 +296,10 @@ function getOrInstantiateModuleFromParent(
   const module = moduleCache[id]
 
   if (module) {
+    if (module.error) {
+      throw module.error
+    }
+
     return module
   }
 

turbopack/crates/turbopack-ecmascript-runtime/js/src/shared/runtime-types.d.ts

@@ -101,7 +101,6 @@ type ExternalImport = (
 interface Module {
   exports: Function | Exports | Promise<Exports> | AsyncModulePromise
   error: Error | undefined
-  loaded: boolean
   id: ModuleId
   namespaceObject?:
     | EsmNamespaceObject

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

@@ -89,7 +89,6 @@ function createModuleObject(id: ModuleId): Module {
   return {
     exports: {},
     error: undefined,
-    loaded: false,
     id,
     namespaceObject: undefined,
     [REEXPORTED_OBJECTS]: undefined,
@@ -101,20 +100,24 @@ function createModuleObject(id: ModuleId): Module {
  */
 function esm(
   exports: Exports,
-  getters: Record<string, (() => any) | [() => any, (v: any) => void]>
+  getters: Array<string | (() => unknown) | ((v: unknown) => void)>
 ) {
   defineProp(exports, '__esModule', { value: true })
   if (toStringTag) defineProp(exports, toStringTag, { value: 'Module' })
-  for (const key in getters) {
-    const item = getters[key]
-    if (Array.isArray(item)) {
-      defineProp(exports, key, {
-        get: item[0],
-        set: item[1],
+  let i = 0
+  while (i < getters.length) {
+    const propName = getters[i++] as string
+    // TODO(luke.sandberg): we could support raw values here, but would need a discriminator beyond 'not a function'
+    const getter = getters[i++] as () => unknown
+    if (typeof getters[i] === 'function') {
+      // a setter
+      defineProp(exports, propName, {
+        get: getter,
+        set: getters[i++] as (v: unknown) => void,
         enumerable: true,
       })
     } else {
-      defineProp(exports, key, { get: item, enumerable: true })
+      defineProp(exports, propName, { get: getter, enumerable: true })
     }
   }
   Object.seal(exports)
@@ -125,16 +128,19 @@ function esm(
  */
 function esmExport(
   this: TurbopackBaseContext<Module>,
-  getters: Record<string, () => any>,
+  getters: Array<string | (() => unknown) | ((v: unknown) => void)>,
   id: ModuleId | undefined
 ) {
-  let module = this.m
-  let exports = this.e
+  let module: Module
+  let exports: Module['exports']
   if (id != null) {
     module = getOverwrittenModule(this.c, id)
     exports = module.exports
+  } else {
+    module = this.m
+    exports = this.e
   }
-  module.namespaceObject = module.exports
+  module.namespaceObject = exports
   esm(exports, getters)
 }
 contextPrototype.s = esmExport
@@ -246,22 +252,32 @@ function interopEsm(
   ns: EsmNamespaceObject,
   allowExportDefault?: boolean
 ) {
-  const getters: { [s: string]: () => any } = Object.create(null)
+  const getters: Array<string | (() => unknown) | ((v: unknown) => void)> = []
+  // The index of the `default` export if any
+  let defaultLocation = -1
   for (
     let current = raw;
     (typeof current === 'object' || typeof current === 'function') &&
     !LEAF_PROTOTYPES.includes(current);
     current = getProto(current)
   ) {
     for (const key of Object.getOwnPropertyNames(current)) {
-      getters[key] = createGetter(raw, key)
+      getters.push(key, createGetter(raw, key))
+      if (defaultLocation === -1 && key === 'default') {
+        defaultLocation = getters.length - 1
+      }
     }
   }
 
   // this is not really correct
   // we should set the `default` getter if the imported module is a `.cjs file`
-  if (!(allowExportDefault && 'default' in getters)) {
-    getters['default'] = () => raw
+  if (!(allowExportDefault && defaultLocation >= 0)) {
+    // Replace the binding with one for the namespace itself in order to preserve iteration order.
+    if (defaultLocation >= 0) {
+      getters[defaultLocation] = () => raw
+    } else {
+      getters.push('default', () => raw)
+    }
   }
 
   esm(ns, getters)
@@ -283,7 +299,6 @@ function esmImport(
   id: ModuleId
 ): Exclude<Module['namespaceObject'], undefined> {
   const module = getOrInstantiateModuleFromParent(id, this.m)
-  if (module.error) throw module.error
 
   // any ES module has to have `module.namespaceObject` defined.
   if (module.namespaceObject) return module.namespaceObject
@@ -325,9 +340,7 @@ function commonJsRequire(
   this: TurbopackBaseContext<Module>,
   id: ModuleId
 ): Exports {
-  const module = getOrInstantiateModuleFromParent(id, this.m)
-  if (module.error) throw module.error
-  return module.exports
+  return getOrInstantiateModuleFromParent(id, this.m).exports
 }
 contextPrototype.r = commonJsRequire
 

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

@@ -3,11 +3,11 @@ use std::{borrow::Cow, collections::BTreeMap, ops::ControlFlow};
 use anyhow::{Result, bail};
 use rustc_hash::FxHashSet;
 use serde::{Deserialize, Serialize};
+use smallvec::{SmallVec, smallvec};
 use swc_core::{
     common::{DUMMY_SP, SyntaxContext},
     ecma::ast::{
-        AssignTarget, Expr, ExprStmt, Ident, KeyValueProp, ObjectLit, Prop, PropName, PropOrSpread,
-        SimpleAssignTarget, Stmt, Str,
+        ArrayLit, AssignTarget, Expr, ExprOrSpread, ExprStmt, Ident, SimpleAssignTarget, Stmt, Str,
     },
     quote, quote_expr,
 };
@@ -614,10 +614,10 @@ impl EsmExports {
 
         let mut getters = Vec::new();
         for (exported, local) in &expanded.exports {
-            let expr = match local {
-                EsmExport::Error => Some(quote!(
+            let exprs: SmallVec<[Expr; 1]> = match local {
+                EsmExport::Error => smallvec![quote!(
                     "(() => { throw new Error(\"Failed binding. See build errors!\"); })" as Expr,
-                )),
+                )],
                 EsmExport::LocalBinding(name, mutable) => {
                     // TODO ideally, this information would just be stored in
                     // EsmExport::LocalBinding and we wouldn't have to re-correlated this
@@ -645,16 +645,20 @@ impl EsmExports {
                     });
 
                     if *mutable {
-                        Some(quote!(
-                            "([() => $local, ($new) => $local = $new])" as Expr,
-                            local = Ident::new(local.into(), DUMMY_SP, ctxt),
-                            new = Ident::new(format!("new_{name}").into(), DUMMY_SP, ctxt),
-                        ))
+                        let local = Ident::new(local.into(), DUMMY_SP, ctxt);
+                        smallvec![
+                            quote!("() => $local" as Expr, local = local.clone()),
+                            quote!(
+                                "($new) => $local = $new" as Expr,
+                                local = local,
+                                new = Ident::new(format!("new_{name}").into(), DUMMY_SP, ctxt),
+                            )
+                        ]
                     } else {
-                        Some(quote!(
-                            "(() => $local)" as Expr,
+                        smallvec![quote!(
+                            "() => $local" as Expr,
                             local = Ident::new((name as &str).into(), DUMMY_SP, ctxt)
-                        ))
+                        )]
                     }
                 }
                 EsmExport::ImportedBinding(esm_ref, name, mutable) => {
@@ -666,9 +670,13 @@ impl EsmExports {
                         .map(|ident| {
                             let expr = ident.as_expr_individual(DUMMY_SP);
                             if *mutable {
+                                smallvec![
                                 quote!(
-                                    "([() => $expr, ($new) => $lhs = $new])" as Expr,
+                                    "() => $expr" as Expr,
                                     expr: Expr = expr.clone().map_either(Expr::from, Expr::from).into_inner(),
+                                ),
+                                quote!(
+                                    "($new) => $lhs = $new" as Expr,
                                     lhs: AssignTarget = AssignTarget::Simple(
                                         expr.map_either(|i| SimpleAssignTarget::Ident(i.into()), SimpleAssignTarget::Member).into_inner()),
                                     new = Ident::new(
@@ -677,13 +685,14 @@ impl EsmExports {
                                         Default::default()
                                     ),
                                 )
+                                ]
                             } else {
-                                quote!(
+                                smallvec![quote!(
                                     "(() => $expr)" as Expr,
                                     expr: Expr = expr.map_either(Expr::from, Expr::from).into_inner()
-                                )
+                                )]
                             }
-                        })
+                        }).unwrap_or_default()
                 }
                 EsmExport::ImportedNamespace(esm_ref) => {
                     let referenced_asset =
@@ -692,27 +701,29 @@ impl EsmExports {
                         .get_ident(chunking_context, None, scope_hoisting_context)
                         .await?
                         .map(|ident| {
-                            quote!(
+                            smallvec![quote!(
                                 "(() => $imported)" as Expr,
                                 imported: Expr = ident.as_expr(DUMMY_SP, false)
-                            )
+                            )]
                         })
+                        .unwrap_or_default()
                 }
             };
-            if let Some(expr) = expr {
-                getters.push(PropOrSpread::Prop(Box::new(Prop::KeyValue(KeyValueProp {
-                    key: PropName::Str(Str {
+            if !exprs.is_empty() {
+                getters.push(Some(
+                    Expr::Lit(swc_core::ecma::ast::Lit::Str(Str {
                         span: DUMMY_SP,
                         value: exported.as_str().into(),
                         raw: None,
-                    }),
-                    value: Box::new(expr),
-                }))));
+                    }))
+                    .into(),
+                ));
+                getters.extend(exprs.into_iter().map(|e| Some(ExprOrSpread::from(e))));
             }
         }
-        let getters = Expr::Object(ObjectLit {
+        let getters = Expr::Array(ArrayLit {
             span: DUMMY_SP,
-            props: getters,
+            elems: getters,
         });
         let dynamic_stmt = if !dynamic_exports.is_empty() {
             Some(Stmt::Expr(ExprStmt {

turbopack/crates/turbopack-ecmascript/src/runtime_functions.rs

@@ -110,11 +110,10 @@ pub const TURBOPACK_WASM_MODULE: &TurbopackRuntimeFunctionShortcut =
 
 /// Adding an entry to this list will automatically ensure that `__turbopack_XXX__` can be called
 /// from user code (by inserting a replacement into free_var_references)
-pub const TURBOPACK_RUNTIME_FUNCTION_SHORTCUTS: [(&str, &TurbopackRuntimeFunctionShortcut); 23] = [
+pub const TURBOPACK_RUNTIME_FUNCTION_SHORTCUTS: [(&str, &TurbopackRuntimeFunctionShortcut); 22] = [
     ("__turbopack_require__", TURBOPACK_REQUIRE),
     ("__turbopack_module_context__", TURBOPACK_MODULE_CONTEXT),
     ("__turbopack_import__", TURBOPACK_IMPORT),
-    ("__turbopack_esm__", TURBOPACK_ESM),
     ("__turbopack_export_value__", TURBOPACK_EXPORT_VALUE),
     ("__turbopack_export_namespace__", TURBOPACK_EXPORT_NAMESPACE),
     ("__turbopack_cache__", TURBOPACK_CACHE),

turbopack/crates/turbopack-tests/tests/execution/turbopack/basic/esm-interop/input/index.js

@@ -6,12 +6,15 @@ it('should allow to access non-enumerable inherited properties', () => {
   expect(test.default).toMatchObject({
     named: 'named',
     default: 'default',
+    base: 'base',
   })
   expect(test).toMatchObject({
     named: 'named',
+    base: 'base',
     default: expect.objectContaining({
       named: 'named',
       default: 'default',
+      base: 'base',
     }),
   })
 })

turbopack/crates/turbopack-tests/tests/execution/turbopack/basic/esm-interop/input/non-enumerable.js

@@ -1,4 +1,17 @@
-class X {
+class Y {
+  get base() {
+    return 'base'
+  }
+
+  get named() {
+    return 'base-named'
+  }
+
+  get default() {
+    return 'base-default'
+  }
+}
+class X extends Y {
   get named() {
     return 'named'
   }