Skip to content

experimentalImportSupport: Fix export * from incorrectly re-exporting the default export - #1777

Closed
robhogan wants to merge 1 commit into
mainfrom
export-D111910097
Closed

robhogan wants to merge 1 commit into
mainfrom
export-D111910097

Conversation

@robhogan

Copy link
Copy Markdown
Collaborator

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

…e default export

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
@meta-codesync

meta-codesync Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

@robhogan has exported this pull request. If you are a Meta employee, you can view the originating Diff in D111910097.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 14, 2026
@robhogan robhogan changed the title experimentalImportSupport: Fix export * incorrectly re-exporting the default export experimentalImportSupport: Fix export * from incorrectly re-exporting the default export Jul 14, 2026
@meta-codesync meta-codesync Bot closed this in 11ad083 Jul 18, 2026
@meta-codesync meta-codesync Bot added the Merged label Jul 18, 2026
@meta-codesync

meta-codesync Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

This pull request has been merged in 11ad083.

meta-codesync Bot pushed a commit that referenced this pull request Sep 11, 2026
Summary:
Publish Metro 0.87.1 from `main`.

Changelog: [Internal]

Pull Request resolved: #1926

Test Plan:
```
node scripts/updateVersion.js 0.87.1
sl diff --stat
```

Only the 17 `packages/*/package.json` manifests change, and every `0.87.0` reference is replaced.

Draft release notes (from the 91 commits in v0.87.0...main - reverted and internal-only changes omitted):

```
 - **[Feature]**: Add `resolver.schemeResolvers` to resolve URI-scheme-prefixed specifiers (eg `foo:bar`) with custom resolvers (#1804 by robhogan)
 - **[Feature]**: Resolve `metro:babel-runtime/<path>` imports to Metro's own `babel/runtime` dependency (8388f71 by robhogan)
 - **[Feature]**: `metro-file-map`: Add `crawlerFactory` to optionally replace the built-in Watchman / Node crawlers (68022f0 by vzaidman)
 - **[Feature]**: `metro-babel-transformer`: Pass `inlinePlatform` to Babel presets via `caller` (6bbe095 by robhogan)
 - **[Fix]**: Replace `image-size` dependency with vendored parsers, fix CVE alerts (#1860 by robhogan)
 - **[Fix]**: Include `charset=utf-8` in the `Content-Type` of text assets and source files served by the dev server (#1888 by robhogan)
 - **[Fix]**: Preserve `Platform.OS` write targets during constant inlining (#1890 by OskarEichler)
 - **[Fix]**: Inline the last duplicate key from static `Platform.select` object literals (#1889 by OskarEichler)
 - **[Fix]**: `FallbackWatcher` no longer misses files written to a directory while it is being crawled (#1907 by robhogan)
 - **[Fix]**: `HttpStore`: Handle socket errors during writes, so they're retried rather than crashing the process (fad3b88)
 - **[Fix]**: Treat `CI=false` and `CI=0` as not-CI when defaulting `watch`. CI is now detected from `process.env.CI` only, dropping the `ci-info` dependency (61e4592 by robhogan)
 - **[Fix]**: Fix Fast Refresh hitting undefined modules when using lazy (segmented) bundles in dev (c70d0ae by robhogan)
 - **[Performance]**: `FallbackWatcher`: Drop `walker` dependency, reduce crawl RSS by ~34% and peak heap by ~41% (#1906 by robhogan)
 - **[Performance]**: Don't emit redundant empty dependency map arrays for modules with no dependencies (#1858 by robhogan)
 - **[Types]**: Restore publishing of private (underscore-prefixed) fields in TypeScript declarations (#1876 by robhogan)
 - **[Types]**: Fix async functions being inferred as returning `void` (not `Promise<void>`), eg `MetroServer#end` (#1909 by robhogan)
 - **[Types]**: Tidy up generated types, allow nullable props to be omitted in more places (#1885 by robhogan)

> NOTE: Experimental features are not covered by semver and can change at any time.

 - **[Experimental]**: Add `serializer.unstable_inlineDependencyMap` to inline module IDs at serialisation time (#1786 by robhogan)
 - **[Experimental]**: Add `serializer.unstable_getAsyncDependencyPath` to supply custom `paths` to framework-defined `__loadBundleAsync` (#1855 by robhogan)
 - **[Experimental]**: Remove `transformer.unstable_renameRequire` - `require` is never renamed (42577f7 by robhogan)
 - **[Experimental]**: `experimentalImportSupport`: Fix `export * from` re-exporting the source module's default export (#1777 by robhogan)

 **Full Changelog**: v0.87.0...v0.87.1
```

Reviewed By: zeyap

Differential Revision: D119675927

Pulled By: GijsWeterings

fbshipit-source-id: daf17f3501b4725c21c39b4dfd5c82b95e01dca5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged meta-exported

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant