From 6170cebdb01a2ef02e3485e862d6f7638f793e75 Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Tue, 14 Jul 2026 08:32:05 -0700 Subject: [PATCH] experimentalImportSupport: Fix `export *` incorrectly re-exporting the default export MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Fix a spec conformance bug in `export * from 'foo'` under `experimentalImportSupport` (`import-export-plugin`). Currently, Metro re-exports all exports including the source module's default export. ## Spec https://tc39.es/ecma262/multipage/ecmascript-language-scripts-and-modules.html#sec-getexportednames `8.d.i` ``` 8. For each ExportEntry Record exportEntry of module.[[StarExportEntries]], do a. Assert: exportEntry.[[ModuleRequest]] is not null. b. Let requestedModule be GetImportedModule(module, exportEntry.[[ModuleRequest]]). c. Let starNames be requestedModule.GetExportedNames(exportStarSet). d. For each element name of starNames, do -> i. If name is not "default", then 1. If exportedNames does not contain name, then a. Append name to exportedNames. ``` Node.js, Babel's `transform-modules-commonjs`, and browsers all conform to this spec. ## Fix Trivially `continue` through a key named "default" in our `exportAllTemplate`. NOTE: `export * as ns from 'foo'` is unaffected: it goes through the `ExportNamespaceSpecifier` / `importAll` path, and per spec the namespace object still exposes the source's default. ## Possible alternatives + benchmarks Compiled release HBC under SH trunk (`hermesc -O -emit-binary`) and run in the `hermes` VM. - One "op" = one full re-export of a module with 20 named exports + `default` (which every form must skip). - Throughput = median of 9 runs × 250k ops. - Bytecode = marginal HBC size of the isolated `f(exports, REQUIRED)` body vs an empty-body baseline. | Form | Input | Throughput (ops/s) | stddev | rel | HBC bytes (Δ) | |---|---|--:|--:|--:|--:| | `for-in`, no `default` guard (re-exports default) | 20 + `default` | 99,602 | 1,041 | 0.95× | +96 | | `for-in` + `if (KEY === "default") continue;` (current) | 20 + `default` | 104,778 | 1,025 | 1.00× | +126 | | `for-in` + `if (KEY !== "default") { ... }` | 20 + `default` | 104,515 | 879 | 1.00× | +127 | | object spread + `delete default` | 20 + `default` | 91,709 | 583 | 0.88× | +148 | | `Object.keys` + indexed for-loop | 20 + `default` | 90,351 | 672 | 0.86× | +206 | | `Object.getOwnPropertyNames` + for-loop | 20 + `default` | 89,831 | 680 | 0.86× | +224 | | `for-in` + `hasOwnProperty` guard | 20 + `default` | 78,939 | 862 | 0.75× | +239 | | `Object.keys().forEach` | 20 + `default` | 73,746 | 346 | 0.70× | +273 | | `Object.assign` + `delete default` | 20 + `default` | 61,395 | 567 | 0.59× | +139 | | `for-in` + `default` guard (guard is pure overhead) | 20, **no** `default` | 104,080 | 538 | 0.99× | +126 | | `for-in`, no guard | 20, **no** `default` | 104,866 | 634 | 1.00× | +96 | **Conclusion:** both `for-in` guard styles are fastest and smallest of all semantically-equivalent forms (differing by ~3% throughput / 1 byte — within noise). - The `default` guard's runtime effect flips with the input: - **default present:** guard is **+5.2%** faster (104,778 vs 99,602) — skipping the `default` property write outweighs the per-key comparisons. - **default absent:** guard is **−0.7%** slower (104,080 vs 104,866) — pure overhead, nothing to skip. - The guard is order of magnitudes cheaper than prop assignment, adding it has negligible effect. - Adding the guard adds 5 bytes of HBC per occurrence (plus one-time 16 bytes for the "default" string table entry - which is there in a real app anyway). ## Changelog ``` - **[Experimental]** experimentalImportSupport: Fix `export * from` incorrectly re-exporting default exports ``` Differential Revision: D111910097 --- .../__tests__/import-export-plugin-test.js | 78 +++++++++++++++++++ .../src/import-export-plugin.js | 1 + 2 files changed, 79 insertions(+) diff --git a/packages/metro-transform-plugins/src/__tests__/import-export-plugin-test.js b/packages/metro-transform-plugins/src/__tests__/import-export-plugin-test.js index 0852228a04..2995a584ec 100644 --- a/packages/metro-transform-plugins/src/__tests__/import-export-plugin-test.js +++ b/packages/metro-transform-plugins/src/__tests__/import-export-plugin-test.js @@ -328,6 +328,7 @@ test('exports members of another module directly from an import (as all)', () => var _bar = require('bar'); for (var _key in _bar) { + if (_key === "default") continue; exports[_key] = _bar[_key]; } `; @@ -376,6 +377,7 @@ test('places export all above explicit exports', () => { var _foo = require('foo'); for (var _key in _foo) { + if (_key === "default") continue; exports[_key] = _foo[_key]; } @@ -425,6 +427,82 @@ test('explicit exports override export all at runtime', () => { expect(context.exports.sourceOnly).toBe('source only'); }); +test('export all does not re-export the default of the source', () => { + const transformedCode = generate( + transformToAst( + [importExportPlugin], + ` + export * from 'foo'; + `, + opts, + ), + ).code; + const context = { + exports: {} as {[string]: unknown}, + require: (id: string) => { + if (id !== 'foo') { + throw new Error(`Unexpected module: ${id}`); + } + return { + default: 'star default', + named: 'star named', + }; + }, + }; + + vm.runInNewContext(transformedCode, context); + + // Per the ES spec (GetExportedNames) and Node.js, `export *` re-exports + // named exports but never the source module's default export. + expect(context.exports.named).toBe('star named'); + expect('default' in context.exports).toBe(false); +}); + +test('export all as namespace includes the default of the source', () => { + const transformedCode = generate( + transformToAst( + [importExportPlugin], + ` + export * as ns from 'foo'; + `, + opts, + ), + ).code; + const source = { + __esModule: true, + default: 'star default', + named: 'star named', + }; + // Faithful stand-in for metroImportAll: resolve the module via require, + // then expose an ES module's namespace as-is (default included). + const requireMock = (id: string) => { + if (id !== 'foo') { + throw new Error(`Unexpected module: ${id}`); + } + return source; + }; + const exportsObj: {[string]: unknown} = {}; + const context: {[string]: unknown} = { + exports: exportsObj, + require: requireMock, + }; + context[opts.importAll] = (id: string) => { + const mod = requireMock(id); + return mod.__esModule === true ? mod : {...mod, default: mod}; + }; + + vm.runInNewContext(transformedCode, context); + + // `export * as ns` (ExportNamespaceSpecifier) creates a namespace object, + // which per the ES spec DOES expose the source module's default export - + // unlike bare `export *`. + expect(exportsObj.ns).toEqual({ + __esModule: true, + default: 'star default', + named: 'star named', + }); +}); + test('re-export dependencies evaluate before module body at runtime', () => { const transformedCode = generate( transformToAst( diff --git a/packages/metro-transform-plugins/src/import-export-plugin.js b/packages/metro-transform-plugins/src/import-export-plugin.js index 5d941bab5f..fac55bcdd3 100644 --- a/packages/metro-transform-plugins/src/import-export-plugin.js +++ b/packages/metro-transform-plugins/src/import-export-plugin.js @@ -87,6 +87,7 @@ const exportAllTemplate = template.statements(` var REQUIRED = require(FILE); for (var KEY in REQUIRED) { + if (KEY === "default") continue; exports[KEY] = REQUIRED[KEY]; } `);