From 26f07b09e65103a5e4b13f92eeab4d7c1a2b8dd1 Mon Sep 17 00:00:00 2001 From: Vitali Zaidman Date: Wed, 19 Aug 2026 02:44:03 -0700 Subject: [PATCH 1/3] ESM live bindings 1/n: mirror own-export reassignments into `exports` Summary: # Context - live bindings Metro's `experimentalImportSupport` transform gives ES module imports snapshot semantics. An imported binding is read once, so a later reassignment in the source module is never observed: ```js // counter.js export let count = 0; export function bump() { count++; } // consumer.js import {count, bump} from './counter'; bump(); console.log(count); // 0 under Metro today, 1 per spec ``` This is wrong per spec, and it's observable in two scenarios - mutation of an export (as above), which isn't common, and in cycles (below). ## Example - dependency cycle Cycles are valid in ESM and the following should work as it reads. ``` // a.js import { B } from './b.js'; export const A = 1; export function readB() { return B; } ``` ``` // b.js import { A } from './a.js'; export const B = 2; export function readA() { return A; } ``` ``` // main.js import { readB } from './a.js'; import { readA } from './b.js'; console.log(readA(), readB()); // ESM: 1 2 ``` **But with Metro's snapshotting transform**, `a.js` is still evaluating (requiring `b.js`, before it assigns `exports.A`) when `b.js` snapshots its (empty) exports. ``` // a.js const { B } = require('./b.js'); // b.js completes, 2 is assigned to B exports.A = 1; exports.readB = () => B; ``` ``` // b.js const { A } = require('./a.js'); // a.js in progress, its module.exports is still empty {} exports.B = 2; exports.readA = () => A; // snapshotted as `undefined` ``` This is an insidious class of bugs that escapes detection by type checkers. ## Performance Making imports live means a read has to go back to the source on every access rather than binding a value once, so it costs bytes and potentially time. This stack implements liveness first, and then builds on that with optimisations to bring us back to neutral or better. Liveness stays behind `unstable_liveBindings`, off by default through this stack. # This diff The producer half: makes a module's own exports live, so a reassignment after initialisation is visible to importers. Babel does this by defining an accessor on `exports` for every exported name. Instead this walks each exported binding's `constantViolations` and mirrors the reassignment back as a plain data-property write: | source | emitted | | --- | --- | | `x = v` | `exports.x = (x = v)` | | `x += v` | `exports.x = (x += v)` | | `++x` | `exports.x = ++x` | | `x++` (value unused) | `exports.x = ++x` | | `x++` (value used) | `(_x = x++, exports.x = x, _x)` | Reads stay on the plain-property fast path and the cost lands only where a reassignment actually happens. Only 7 modules in the Wilde graph ever reassign an exported binding, so this is close to free in practice. ## Why mirroring rather than accessors Costs land in different places. Accessors pay per exported *name* at init and make every read a getter call. Mirroring pays per *reassignment* and leaves reads as plain property loads. Measured on the stable SH compiler with the production invocation, and on the release (opt) VM: | | mirroring | accessors | | --- | --- | --- | | HBC per exported name | 26.29 B | 86.03 B | | HBC per reassignment | 14.59 B | 0 B | | read | 7.45 ns | 40.80 ns | | write | 26.80 ns | 6.00 ns | Mirroring wins on size while a module averages fewer than **4.09 reassignments per exported name**. Across FB-app graph only **7 modules of 44,621** reassign an exported binding at all, so essentially every module pays the 26 B seed and nothing more. It also puts the CPU cost on the rare operation. Reads outnumber reassignments heavily, and accessors make every one of them 5.5x more expensive. ## Re-exports use getters, not mirroring Re-exports (`export {x} from './y'`, `export {default as D} from './y'`, `export * from './y'`) require liveness but cannot use mirroring: the reassignment happens in the source module, not this one, so there is no local binding site to hook onto. Snapshotting at re-export time (`exports.x = require('./y').x`) would freeze the value at first read. The plugin installs a getter: ```js Object.defineProperty(exports, 'x', { enumerable: true, configurable: true, get: function () { return require('./y').x; }, }); ``` ## Correctness tradeoff The cost is that `exports.x` remains an ordinary, writable data property (incorrect, but not a regression vs Metro's current output). Because this throws under real ESM, it's a pattern that should not exist in the wild. Flow and TS already error on it. Reviewed By: huntie Differential Revision: D111529091 --- .../__tests__/import-export-plugin-test.js | 245 ++++++++++++++++ .../src/import-export-plugin.js | 277 +++++++++++++++++- 2 files changed, 513 insertions(+), 9 deletions(-) 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 12e46ac6ef..6f1e6abb1c 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 @@ -25,6 +25,12 @@ const opts = { importDefault: '_$$_IMPORT_DEFAULT', }; +const liveOpts = { + importAll: '_$$_IMPORT_ALL', + importDefault: '_$$_IMPORT_DEFAULT', + liveBindings: true, +}; + test('correctly transforms and extracts "import" statements', () => { const code = ` import v from 'foo'; @@ -532,6 +538,245 @@ test('re-export dependencies evaluate before module body at runtime', () => { expect(context.exports.star).toBe('bar star'); }); +describe('unstable_liveBindings', () => { + test('the import side is untouched by this option', () => { + const code = ` + import v from 'foo'; + import {default as w} from 'bar'; + import {x} from 'baz'; + `; + + const expected = ` + var v = _$$_IMPORT_DEFAULT('foo'); + var w = _$$_IMPORT_DEFAULT('bar'); + var x = require('baz').x; + `; + + compare([importExportPlugin], code, expected, liveOpts); + }); + + test('reassigned named exports are mirrored into exports', () => { + const code = ` + export let x = 1; + x = 2; + x += 3; + x++; + ++x; + `; + + const expected = ` + Object.defineProperty(exports, '__esModule', { + value: true + }); + let x = 1; + exports.x = x = 2; + exports.x = x += 3; + exports.x = ++x; + exports.x = ++x; + exports.x = x; + `; + + compare([importExportPlugin], code, expected, liveOpts); + }); + + test('postfix update in value position preserves the old value', () => { + const code = ` + export let x = 1; + export const y = x++; + `; + + const expected = ` + var _x; + Object.defineProperty(exports, '__esModule', { + value: true + }); + let x = 1; + const y = (_x = x++, exports.x = x, _x); + exports.x = x; + exports.y = y; + `; + + compare([importExportPlugin], code, expected, liveOpts); + }); + + test('postfix update value and mirrored export agree at runtime', () => { + const transformedCode = generate( + transformToAst( + [importExportPlugin], + ` + export let x = 0; + export function postfix() { return x++; } + export function prefix() { return ++x; } + `, + liveOpts, + ), + ).code; + + const context = { + exports: {} as {[string]: $FlowFixMe}, + require: () => ({}), + }; + + vm.runInNewContext(transformedCode, context); + + // `x++` must evaluate to the pre-increment value while still publishing the + // post-increment value to `exports`. + expect(context.exports.postfix()).toBe(0); + expect(context.exports.x).toBe(1); + expect(context.exports.prefix()).toBe(2); + expect(context.exports.x).toBe(2); + }); + + test('exports aliased under multiple remote names are all mirrored', () => { + const code = ` + let x = 1; + export {x, x as y}; + x = 2; + `; + + const expected = ` + Object.defineProperty(exports, '__esModule', { + value: true + }); + let x = 1; + exports.y = exports.x = x = 2; + exports.x = x; + exports.y = x; + `; + + compare([importExportPlugin], code, expected, liveOpts); + }); + + test('destructuring reassignment targets are left untouched (deferred)', () => { + const code = ` + export let x = 1; + ({x} = {x: 2}); + `; + + const expected = ` + Object.defineProperty(exports, '__esModule', { + value: true + }); + let x = 1; + ({ + x + } = { + x: 2 + }); + exports.x = x; + `; + + compare([importExportPlugin], code, expected, liveOpts); + }); + + test('mutable named exports are observable at runtime', () => { + const transformedCode = generate( + transformToAst( + [importExportPlugin], + ` + export let counter = 0; + export function increment() { counter++; } + export function setCounter(v) { counter = v; } + `, + liveOpts, + ), + ).code; + + const context = { + exports: {} as {[string]: $FlowFixMe}, + require: () => ({}), + }; + + vm.runInNewContext(transformedCode, context); + + expect(context.exports.counter).toBe(0); + context.exports.increment(); + expect(context.exports.counter).toBe(1); + context.exports.setCounter(42); + expect(context.exports.counter).toBe(42); + }); + + test('named re-exports forward via live getters', () => { + const code = `export {x} from './foo';`; + const expected = ` + Object.defineProperty(exports, '__esModule', { + value: true + }); + Object.defineProperty(exports, "x", { + enumerable: true, + configurable: true, + get: function () { + return require('./foo').x; + } + }); + `; + compare([importExportPlugin], code, expected, liveOpts); + }); + + test('re-exported named binding is observed live at runtime', () => { + const transformedCode = generate( + transformToAst( + [importExportPlugin], + `export {counter} from './source';`, + liveOpts, + ), + ).code; + + const sourceExports = {counter: 1} as {[string]: $FlowFixMe}; + const context = { + exports: {} as {[string]: $FlowFixMe}, + require: (id: string) => { + if (id !== './source') { + throw new Error(`Unexpected module: ${id}`); + } + return sourceExports; + }, + }; + + vm.runInNewContext(transformedCode, context); + + expect(context.exports.counter).toBe(1); + // Reassignment in the source module is observed through the re-export. + sourceExports.counter = 42; + expect(context.exports.counter).toBe(42); + }); + + test('export * forwards live and respects explicit-export precedence', () => { + const transformedCode = generate( + transformToAst( + [importExportPlugin], + ` + export * from './source'; + export const own = 'own'; + `, + liveOpts, + ), + ).code; + + const sourceExports = { + a: 1, + own: 'star should not win', + default: 'star default', + __esModule: true, + } as {[string]: $FlowFixMe}; + const context = { + exports: {} as {[string]: $FlowFixMe}, + require: (_id: string) => sourceExports, + }; + + vm.runInNewContext(transformedCode, context); + + expect(context.exports.a).toBe(1); + // Explicit export wins over `export *`. + expect(context.exports.own).toBe('own'); + // `export *` never forwards `default` or `__esModule`. + expect(context.exports.default).toBeUndefined(); + // Forwarded names are live. + sourceExports.a = 2; + expect(context.exports.a).toBe(2); + }); +}); + test('enables module exporting when something is exported', () => { const code = ` foo(); diff --git a/packages/metro-transform-plugins/src/import-export-plugin.js b/packages/metro-transform-plugins/src/import-export-plugin.js index e9d1809c46..1ef3f0b199 100644 --- a/packages/metro-transform-plugins/src/import-export-plugin.js +++ b/packages/metro-transform-plugins/src/import-export-plugin.js @@ -35,13 +35,21 @@ import nullthrows from 'nullthrows'; export type Options = Readonly<{ importDefault: string, importAll: string, + liveBindings?: boolean, resolve: boolean, out?: {isESModule: boolean, ...}, }>; type State = { exportAll: Array<{file: string, loc: ?SourceLocation, ...}>, + exportAllLive: Array<{source: Node, loc: ?SourceLocation, ...}>, exportDefault: Array<{local: string, loc: ?SourceLocation, ...}>, + exportGetters: Array<{ + remote: string, + value: Expression, + loc: ?SourceLocation, + ... + }>, exportNamed: Array<{ local: string, remote: string, @@ -101,6 +109,64 @@ const exportTemplate = template.statement(` exports.REMOTE = LOCAL; `); +/** + * Live re-export forwarding ("export {x} from '...'"): defines a getter on + * exports so that reads observe the current value in the source module, which + * may change after this module is evaluated. + */ +const exportGetterTemplate = template.statement(` + Object.defineProperty(exports, REMOTE, { + enumerable: true, + configurable: true, + get: function () { + return VALUE; + }, + }); +`); + +/** + * Reads a named binding from a required module, used inside a live re-export + * getter. + */ +const requireMemberTemplate = template.expression(` + require(FILE).REMOTE +`); + +/** + * Calls an import helper, used inside a live default re-export getter. + */ +const importCallTemplate = template.expression(` + IMPORT(FILE) +`); + +/** + * Live "export all" ("export * from '...'"): defines a getter for each of the + * source module's own enumerable names, except "default"/"__esModule" and names + * already exported by this module (explicit exports take precedence). Reads stay + * live. + */ +const exportAllLiveTemplate = template.statements(` + var REQUIRED = require(FILE); + + Object.keys(REQUIRED).forEach(function (KEY) { + if ( + KEY === "default" || + KEY === "__esModule" || + Object.prototype.hasOwnProperty.call(exports, KEY) + ) { + return; + } + + Object.defineProperty(exports, KEY, { + enumerable: true, + configurable: true, + get: function () { + return REQUIRED[KEY]; + }, + }); + }); +`); + /** * Flags the exported module as a transpiled ES module. Needs to be kept in 1:1 * compatibility with Babel. @@ -179,14 +245,24 @@ export default function importExportPlugin({ loc, }); - withLocation( - exportAllTemplate({ - FILE: resolvePath(t.cloneNode(file), state.opts.resolve), - REQUIRED: path.scope.generateUidIdentifier(file.value), - KEY: path.scope.generateUidIdentifier('key'), - }), - loc, - ).forEach(node => state.imports.push({node})); + if (state.opts.liveBindings === true) { + // Defer emission to Program.exit so explicit exports (which take + // precedence) are already defined on `exports` when the live getters + // are installed. + state.exportAllLive.push({ + source: resolvePath(t.cloneNode(file), state.opts.resolve), + loc, + }); + } else { + withLocation( + exportAllTemplate({ + FILE: resolvePath(t.cloneNode(file), state.opts.resolve), + REQUIRED: path.scope.generateUidIdentifier(file.value), + KEY: path.scope.generateUidIdentifier('key'), + }), + loc, + ).forEach(node => state.imports.push({node})); + } path.remove(); }, @@ -294,6 +370,39 @@ export default function importExportPlugin({ const local = s.local; if (path.node.source) { + const source = nullthrows(path.node.source); + + if (state.opts.liveBindings === true) { + // Re-export forwarding must be live: the source binding can be + // reassigned after this module is evaluated, so we install a + // getter rather than snapshotting the value. + const value: Expression = + // $FlowFixMe[incompatible-use] + local.name === 'default' + ? importCallTemplate({ + IMPORT: t.cloneNode(state.importDefault), + FILE: resolvePath( + t.cloneNode(source), + state.opts.resolve, + ), + }) + : requireMemberTemplate({ + FILE: resolvePath( + t.cloneNode(source), + state.opts.resolve, + ), + // $FlowFixMe[incompatible-call] + REMOTE: t.cloneNode(local), + }); + + state.exportGetters.push({ + remote: remote.name, + value, + loc, + }); + return; + } + // $FlowFixMe[incompatible-use] const temp = path.scope.generateUidIdentifier(local.name); @@ -511,7 +620,9 @@ export default function importExportPlugin({ Program: { enter(path: NodePath, state: State): void { state.exportAll = []; + state.exportAllLive = []; state.exportDefault = []; + state.exportGetters = []; state.exportNamed = []; state.imports = []; @@ -564,10 +675,48 @@ export default function importExportPlugin({ }, ); + // Live re-export forwarding getters (named/default `export … from`). + // Emitted after the explicit data-property exports above so that, by + // the time the live `export *` loops below run, `exports` already owns + // every explicitly-exported name. + state.exportGetters.forEach( + (e: { + remote: string, + value: Expression, + loc: ?SourceLocation, + ... + }) => { + body.push( + withLocation( + exportGetterTemplate({ + REMOTE: t.stringLiteral(e.remote), + VALUE: e.value, + }), + e.loc, + ), + ); + }, + ); + + // Live `export * from` forwarding loops. + state.exportAllLive.forEach( + (e: {source: Node, loc: ?SourceLocation, ...}) => { + withLocation( + exportAllLiveTemplate({ + REQUIRED: path.scope.generateUidIdentifier('exportAll'), + FILE: e.source, + KEY: path.scope.generateUidIdentifier('key'), + }), + e.loc, + ).forEach(node => body.push(node)); + }, + ); + if ( state.exportDefault.length || state.exportAll.length || - state.exportNamed.length + state.exportNamed.length || + state.exportGetters.length ) { body.unshift(esModuleExportTemplate()); if (state.opts.out) { @@ -576,6 +725,116 @@ export default function importExportPlugin({ } else if (state.opts.out) { state.opts.out.isESModule = false; } + + if (state.opts.liveBindings === true) { + // Recompute scope information now that import/export declarations + // have been rewritten, so that `constantViolations` reflect the + // final tree. + path.scope.crawl(); + + // Map each exported local binding to the remote name(s) it is + // exposed as. + const localToRemotes: Map> = new Map(); + const addLocalRemote = (local: string, remote: string): void => { + const remotes = localToRemotes.get(local); + if (remotes != null) { + remotes.push(remote); + } else { + localToRemotes.set(local, [remote]); + } + }; + state.exportNamed.forEach(e => addLocalRemote(e.local, e.remote)); + state.exportDefault.forEach(e => + addLocalRemote(e.local, 'default'), + ); + + const exportsMember = (remote: string) => + t.memberExpression(t.identifier('exports'), t.identifier(remote)); + + // value -> exports.r1 = exports.r2 = ... = value + const mirrorInto = ( + remotes: Array, + value: Expression, + ): Expression => { + let expr: Expression = value; + for (const remote of remotes) { + expr = t.assignmentExpression('=', exportsMember(remote), expr); + } + return expr; + }; + + // True where the update expression's own value cannot be observed, + // so a postfix update may be rewritten without preserving it. + const isValueDiscarded = (violation: NodePath<>): boolean => { + const parent = violation.parentPath; + if (parent == null) { + return false; + } + return ( + parent.isExpressionStatement() || + (parent.isForStatement() && + parent.node.update === violation.node) + ); + }; + + for (const [local, remotes] of localToRemotes) { + const binding = path.scope.getBinding(local); + if (binding == null) { + continue; + } + for (const violation of binding.constantViolations) { + const vnode = violation.node; + if (t.isAssignmentExpression(vnode)) { + if (!t.isIdentifier(vnode.left, {name: local})) { + // Deferred: destructuring / non-identifier assignment + // targets. + continue; + } + // x = v -> exports.r1 = exports.r2 = (x = v) + violation.replaceWith(mirrorInto(remotes, vnode)); + violation.skip(); + } else if (t.isUpdateExpression(vnode)) { + if (!t.isIdentifier(vnode.argument, {name: local})) { + continue; + } + if (vnode.prefix === true || isValueDiscarded(violation)) { + // ++x -> exports.r1 = ++x + // + // Postfix takes this path too where its value is + // unobservable: prefix and postfix have identical side + // effects, so switching form avoids needing a temporary. + violation.replaceWith( + mirrorInto( + remotes, + t.updateExpression( + vnode.operator, + vnode.argument, + true, + ), + ), + ); + violation.skip(); + continue; + } + // x++ -> (t = x++, exports.r1 = x, t) + // + // Postfix evaluates to the *old* value, so it must be held in + // a temporary: mirroring reads the new value, and the outer + // expression has to keep yielding the old one. + const temp = path.scope.generateUidIdentifier(local); + path.scope.push({id: t.cloneNode(temp)}); + violation.replaceWith( + t.sequenceExpression([ + t.assignmentExpression('=', t.cloneNode(temp), vnode), + mirrorInto(remotes, t.identifier(local)), + t.cloneNode(temp), + ]), + ); + violation.skip(); + } + } + } + } }, }, }, From 4271a293345564e754d99006de8a84e7a5711cc9 Mon Sep 17 00:00:00 2001 From: Vitali Zaidman Date: Wed, 19 Aug 2026 02:44:03 -0700 Subject: [PATCH 2/3] ESM live bindings 2/n: optional experimentalMode arg on importDefault runtime helper for namespace-shaped return MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Adds an optional `experimentalMode` argument to the `importDefault` runtime helper, selecting the return shape. ## Usage Existing, unchanged - helper returns the value of the default export ```js const x = importDefault(id); // exports.default for ESM, exports for CJS ``` `1` — returns a namespace, not memoised: ```js const ns = importDefault(id, 1); // exports for ESM, {default: exports} for CJS ns.default; ``` ## Why this is important for liveness The namespace shape lets a default import bind once and read `.default` per use, which is what the `unstable_liveBindings` emission needs to stay live under inline-requires. Mode 1 skips memoisation because its CJS wrapper is allocated per call — caching it would pin `.default` to the exports object seen on the first call and hide a later `module.exports = X`. ## Why two modes? *The primary purpose of supporting both is temporary experimentation* - I'm hoping the live stack proves end-to-end perf neutral while being more correct, and we can drop the 2-arg form. ## Why not a new helper, or separate function (`require.importDefaultNamespace`, etc) - Adding another helper arg to the module wrapper is a lot of churn and inflated HBC on the production (control) path. - Switching the behaviour of the existing module helper at runtime is viable, but it'd have to be a check inside `require.js`, with `unstable_liveBindings` passed through as a global via prelude or similar. It works but it's intrusive. - Exposing this functionality other than by a direct module wrapper arg call muddles any perf or HBC size comparison with non-live baseline - `require.()` is slower and bigger than `()`, and resolving a variable from a higher scope is slower than a local scope. In the proposed design, we keep it *almost* equivalent, bar the extra arg we expect to disappear. Reviewed By: huntie Differential Revision: D115566590 --- .../src/polyfills/__tests__/require-test.js | 210 ++++++++++++++++++ .../metro-runtime/src/polyfills/require.js | 18 ++ 2 files changed, 228 insertions(+) diff --git a/packages/metro-runtime/src/polyfills/__tests__/require-test.js b/packages/metro-runtime/src/polyfills/__tests__/require-test.js index 1a5bd67ee0..1ca4ab5391 100644 --- a/packages/metro-runtime/src/polyfills/__tests__/require-test.js +++ b/packages/metro-runtime/src/polyfills/__tests__/require-test.js @@ -904,6 +904,216 @@ describe('require', () => { moduleSystem.__r(0); }); + test('mode=1 returns exports directly for ES6 modules (namespace-shaped return)', () => { + createModuleSystem(moduleSystem, false, ''); + + createModule( + moduleSystem, + 0, + 'foo.js', + (global, require, importDefault, importAll, module, exports) => { + const ns = importDefault(1, 1); + expect(ns.default).toEqual({bar: 'bar'}); + // For ESM, the helper returns exports itself (which has .default). + expect(ns).toBe(require(1)); + }, + ); + + createModule( + moduleSystem, + 1, + 'bar.js', + (global, require, importDefault, importAll, module, exports) => { + exports.__esModule = true; + exports.default = {bar: 'bar'}; + exports.other = 'other'; + }, + ); + + expect.assertions(2); + moduleSystem.__r(0); + }); + + test('mode=1 wraps CJS exports as {default: exports} (namespace-shaped return)', () => { + createModuleSystem(moduleSystem, false, ''); + + createModule( + moduleSystem, + 0, + 'foo.js', + (global, require, importDefault, importAll, module, exports) => { + const ns = importDefault(1, 1); + expect(ns.default).toEqual({bar: 'bar'}); + // Wrapper's .default IS the module's exports (for CJS). + expect(ns.default).toBe(require(1)); + }, + ); + + createModule( + moduleSystem, + 1, + 'bar.js', + (global, require, importDefault, importAll, module, exports) => { + module.exports = {bar: 'bar'}; + }, + ); + + expect.assertions(2); + moduleSystem.__r(0); + }); + + test('mode=1 preserves CJS liveness for module.exports=X post-init reassignment', () => { + createModuleSystem(moduleSystem, false, ''); + + let saved; + createModule( + moduleSystem, + 0, + 'foo.js', + (global, require, importDefault, importAll, module, exports) => { + // First read - captures the initial exports. + const first = importDefault(1, 1).default; + expect(first).toEqual({tag: 'initial'}); + + // Reassign source module.exports via saved reference (mimics a + // captured `module` in a lazy handler). + saved.exports = {tag: 'REASSIGNED'}; + + // Second read - must see the fresh exports, not the cached wrapper. + // The helper is non-memoising: each call re-invokes metroRequire + // (which returns publicModule.exports fresh) and rewraps. + const second = importDefault(1, 1).default; + expect(second).toEqual({tag: 'REASSIGNED'}); + }, + ); + + createModule( + moduleSystem, + 1, + 'bar.js', + (global, require, importDefault, importAll, module, exports) => { + module.exports = {tag: 'initial'}; + saved = module; + }, + ); + + expect.assertions(2); + moduleSystem.__r(0); + }); + + test('mode=0/undefined keeps memoising, unchanged by the mode argument', () => { + // The legacy 1-arg path is deliberately left as it was: the first + // resolution is cached on the module definition and reused. This is the + // contrast to the mode=1 test above, and pins the fact that adding the + // mode argument did not alter behaviour for existing callers. + createModuleSystem(moduleSystem, false, ''); + + let saved; + createModule( + moduleSystem, + 0, + 'foo.js', + (global, require, importDefault, importAll, module, exports) => { + const first = importDefault(1); + expect(first).toEqual({tag: 'initial'}); + + saved.exports = {tag: 'REASSIGNED'}; + + // Memoised: still the value captured on the first call. + const second = importDefault(1); + expect(second).toEqual({tag: 'initial'}); + }, + ); + + createModule( + moduleSystem, + 1, + 'bar.js', + (global, require, importDefault, importAll, module, exports) => { + module.exports = {tag: 'initial'}; + saved = module; + }, + ); + + expect.assertions(2); + moduleSystem.__r(0); + }); + + test('mode=1 for ESM preserves live default reassignment', () => { + createModuleSystem(moduleSystem, false, ''); + + let sourceExports; + createModule( + moduleSystem, + 0, + 'foo.js', + (global, require, importDefault, importAll, module, exports) => { + const first = importDefault(1, 1).default; + expect(first).toBe('a'); + + sourceExports.default = 'b'; + + // ESM path: helper returns exports directly. `.default` on that is + // a live property read. + const second = importDefault(1, 1).default; + expect(second).toBe('b'); + }, + ); + + createModule( + moduleSystem, + 1, + 'bar.js', + (global, require, importDefault, importAll, module, exports) => { + exports.__esModule = true; + exports.default = 'a'; + sourceExports = exports; + }, + ); + + expect.assertions(2); + moduleSystem.__r(0); + }); + + test('mode=0/undefined preserves the legacy value-shaped return', () => { + createModuleSystem(moduleSystem, false, ''); + + createModule( + moduleSystem, + 0, + 'foo.js', + (global, require, importDefault, importAll, module, exports) => { + // No mode arg = legacy behaviour = returns the value directly. + expect(importDefault(1)).toEqual({bar: 'bar'}); + expect(importDefault(1, 0)).toEqual({bar: 'bar'}); + expect(importDefault(2)).toBe(null); + expect(importDefault(2, 0)).toBe(null); + }, + ); + + createModule( + moduleSystem, + 1, + 'bar.js', + (global, require, importDefault, importAll, module, exports) => { + exports.__esModule = true; + exports.default = {bar: 'bar'}; + }, + ); + + createModule( + moduleSystem, + 2, + 'nullcjs.js', + (global, require, importDefault, importAll, module, exports) => { + module.exports = null; + }, + ); + + expect.assertions(4); + moduleSystem.__r(0); + }); + test('supports named imports', () => { createModuleSystem(moduleSystem, false, ''); diff --git a/packages/metro-runtime/src/polyfills/require.js b/packages/metro-runtime/src/polyfills/require.js index a81100c854..abffabf382 100644 --- a/packages/metro-runtime/src/polyfills/require.js +++ b/packages/metro-runtime/src/polyfills/require.js @@ -239,6 +239,7 @@ function shouldPrintRequireCycle(modules: ReadonlyArray): boolean { function metroImportDefault( moduleId: ModuleID | VerboseModuleNameForDev, + experimentalMode?: number, ): any | Exports { if (__DEV__ && typeof moduleId === 'string') { const verboseName = moduleId; @@ -248,6 +249,23 @@ function metroImportDefault( //$FlowFixMe[incompatible-type]: at this point we know that moduleId is a number const moduleIdReallyIsNumber: number = moduleId; + if (experimentalMode === 1) { + // Mode 1: namespace-shaped return. Consumers do `ns.default` at each read + // site (the default-import emission under `unstable_liveBindings`). For + // ESM the exports object already has `.default`; for CJS we wrap it as + // `{default: exports}` so the accessor resolves to the module's exports, + // which for CJS is the default. + // + // Deliberately not memoised, unlike the value-shaped path below. The CJS + // wrapper is allocated per call, and caching it would pin `.default` to + // whichever exports object was current on the first call - hiding a later + // `module.exports = X`. Re-resolving keeps the read live. Under the + // serialiser rewrite for proven-classified deps this helper is bypassed + // entirely, so only the unclassified-fallback slice reaches here. + const exports: Exports = metroRequire(moduleIdReallyIsNumber); + return exports && exports.__esModule ? exports : {default: exports}; + } + const maybeInitializedModule = modules.get(moduleIdReallyIsNumber); if ( From 2cc6ebe481db4c45f6fea557f63f68e0c57d4435 Mon Sep 17 00:00:00 2001 From: Vitali Zaidman Date: Wed, 19 Aug 2026 02:44:03 -0700 Subject: [PATCH 3/3] ESM live bindings 3/n: Accept mode-1 helper shape in dependency analysis Summary: Two related relaxations in the dependency-analysis pipeline that together let it accept a mode-selecting second argument on Metro's inlineable helper calls, and observe the underlying usage through the namespace-shaped wrapper the mode-1 form returns. ## 1. `collectDependencies.getModuleNameFromCallArgs` arity Previously rejected any inlineable-helper call with != 1 argument. Now accepts 1 or 2 args. The dependency name is always the first arg; the optional second arg is metadata for the runtime helper (a mode selector - see the sibling change in `metro-runtime/src/polyfills/require.js`). Behaviour unchanged for existing 1-arg callers. 3+ args still rejected. ## 2. `visitDependencyUses.walkReferences` walks through `.default` on ## mode-1 helper results The mode-1 form of `_$$_IMPORT_DEFAULT(id, 1)` returns a namespace-shaped wrapper `{default: }`. Under `unstable_liveBindings`, downstream code accesses this via `_$$_IMPORT_DEFAULT(id, 1).default`. The `.default` accessor is a shape convention, not a semantic operation - analysers built on `visitDependencyUses` (for example `extractSoundResources`, which extracts sound-file names from `sx(name)` call sites) need to observe the OUTER usage, not the intermediate accessor. `walkReferences` now walks through `.default` accessors on mode-1 results, propagating mode-1 provenance through constant `var _foo = helper(id, 1)` bindings. This means: - `_$$_IMPORT_DEFAULT(id, 1).default(arg)` - the visitor sees the outer call and dispatches `visitCall`. - `var _foo = _$$_IMPORT_DEFAULT(id, 1); _foo.default(arg)` - same, via the hoisted binding. - `require('X').default` (arbitrary CJS namespace access, NOT mode-1) - unchanged, `.default` is preserved as the observable operation. The mode-1 detection is a static structural check: the helper call carries a numeric literal `1` as its second argument. ## Why one diff, before the plugin change These two fixes are prerequisites for D115038400 (the plugin change that starts emitting the mode-1 shape under `unstable_liveBindings`). Together they let the dep-analysis pipeline handle the shape end to end - relaxing this once, in isolation from the plugin change, means each subsequent diff stands alone and builds cleanly. Reviewed By: huntie Differential Revision: D115860462 --- .../__tests__/collectDependencies-test.js | 133 ++++++++++++++++++ .../ModuleGraph/worker/collectDependencies.js | 78 +++++++++- .../ModuleGraph/worker/visitDependencyUses.js | 70 ++++++++- 3 files changed, 272 insertions(+), 9 deletions(-) diff --git a/packages/metro/src/ModuleGraph/worker/__tests__/collectDependencies-test.js b/packages/metro/src/ModuleGraph/worker/__tests__/collectDependencies-test.js index 668963bbd1..9f8ec874d4 100644 --- a/packages/metro/src/ModuleGraph/worker/__tests__/collectDependencies-test.js +++ b/packages/metro/src/ModuleGraph/worker/__tests__/collectDependencies-test.js @@ -1184,6 +1184,139 @@ test('collects imports', () => { ]); }); +test('accepts a mode argument only on the configured helper', () => { + // The two-argument form is reserved for the importDefault helper's + // return-shape mode selector, and only when the caller opts in via + // `unstable_modeArgHelper` (set under `unstable_liveBindings`). The extra + // arg is runtime metadata; the dep name still comes from the first arg. + const ast = astFromCode(` + importDefault('a-mod', 1); + importDefault('c-mod'); + importAll('d-mod'); + require('e-mod'); + `); + const {dependencies} = collectDependencies(ast, { + ...opts, + inlineableCalls: ['importDefault', 'importAll'], + unstable_modeArgHelper: 'importDefault', + }); + expect(dependencies.map(d => d.name)).toEqual([ + 'a-mod', + 'c-mod', + 'd-mod', + 'e-mod', + ]); +}); + +test('rejects a mode argument when no helper is configured', () => { + const ast = astFromCode(` + importDefault('a-mod', 1); + `); + expect(() => + collectDependencies(ast, { + ...opts, + inlineableCalls: ['importDefault', 'importAll'], + }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid call at line 2: importDefault('a-mod', 1)"`, + ); +}); + +test('rejects a mode argument on a helper other than the configured one', () => { + const ast = astFromCode(` + importAll('b-mod', 1); + `); + expect(() => + collectDependencies(ast, { + ...opts, + inlineableCalls: ['importDefault', 'importAll'], + unstable_modeArgHelper: 'importDefault', + }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid call at line 2: importAll('b-mod', 1)"`, + ); +}); + +test('preserves the mode argument through inlining, in place of the debug name', () => { + // `opts` sets keepRequireNames, so this covers the branch where a mode-1 + // call site keeps its mode argument and gives up the debug-name argument - + // the two occupy the same slot. Non-mode calls still get the debug name. + const ast = astFromCode(` + importDefault('a-mod', 1); + require('e-mod'); + `); + collectDependencies(ast, { + ...opts, + inlineableCalls: ['importDefault', 'importAll'], + unstable_modeArgHelper: 'importDefault', + }); + expect(codeFromAst(ast)).toEqual( + comparableCode(` + importDefault(_dependencyMap[0], 1); + require(_dependencyMap[1], "e-mod"); + `), + ); +}); + +test('rejects a second argument that is not the mode literal', () => { + const ast = astFromCode(` + importDefault('a-mod', 'x'); + `); + expect(() => + collectDependencies(ast, { + ...opts, + inlineableCalls: ['importDefault', 'importAll'], + unstable_modeArgHelper: 'importDefault', + }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid call at line 2: importDefault('a-mod', 'x')"`, + ); +}); + +test('rejects a second argument on a call that is not a helper', () => { + const ast = astFromCode(` + require('e-mod', 'anything'); + `); + expect(() => + collectDependencies(ast, { + ...opts, + inlineableCalls: ['importDefault', 'importAll'], + unstable_modeArgHelper: 'importDefault', + }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid call at line 2: require('e-mod', 'anything')"`, + ); +}); + +test('rejects a numeric second argument other than 1', () => { + const ast = astFromCode(` + importDefault('a-mod', 2); + `); + expect(() => + collectDependencies(ast, { + ...opts, + inlineableCalls: ['importDefault', 'importAll'], + unstable_modeArgHelper: 'importDefault', + }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid call at line 2: importDefault('a-mod', 2)"`, + ); +}); + +test('rejects three or more arguments on inlineable helper calls', () => { + const ast = astFromCode(` + importDefault('a-mod', 1, 'oops'); + `); + expect(() => + collectDependencies(ast, { + ...opts, + inlineableCalls: ['importDefault', 'importAll'], + }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid call at line 2: importDefault('a-mod', 1, 'oops')"`, + ); +}); + test('collects export from', () => { const ast = astFromCode(` export type {Apple} from 'Apple'; diff --git a/packages/metro/src/ModuleGraph/worker/collectDependencies.js b/packages/metro/src/ModuleGraph/worker/collectDependencies.js index 18aca305be..fa2a90b8c2 100644 --- a/packages/metro/src/ModuleGraph/worker/collectDependencies.js +++ b/packages/metro/src/ModuleGraph/worker/collectDependencies.js @@ -92,6 +92,7 @@ export type State = { dependencyTransformer: DependencyTransformer, dynamicRequires: DynamicRequiresBehavior, dependencyMapIdentifier: ?Identifier, + modeArgHelper: ?string, keepRequireNames: boolean, allowOptionalDependencies: AllowOptionalDependencies, /** Enable `require.context` statements which can be used to import multiple files in a directory. */ @@ -104,6 +105,15 @@ export type Options = Readonly<{ dependencyMapName: ?string, dynamicRequires: DynamicRequiresBehavior, inlineableCalls: ReadonlyArray, + /** + * Name of the one inlineable helper permitted to carry a second, + * mode-selecting argument (`helper(id, 1)`). Set only when + * `unstable_liveBindings` is enabled, and only to the importDefault helper. + * Left unset, the two-argument form is rejected exactly as before, so no + * other call - `require`, `import()`, `resolveWeak`, importAll - can + * silently acquire a second argument. + */ + unstable_modeArgHelper?: ?string, keepRequireNames: boolean, allowOptionalDependencies: AllowOptionalDependencies, dependencyTransformer?: DependencyTransformer, @@ -166,6 +176,7 @@ export default function collectDependencies( dependencyTransformer: options.dependencyTransformer ?? DefaultDependencyTransformer, dependencyMapIdentifier: null, + modeArgHelper: options.unstable_modeArgHelper ?? null, dynamicRequires: options.dynamicRequires, keepRequireNames: options.keepRequireNames, allowOptionalDependencies: options.allowOptionalDependencies, @@ -443,7 +454,7 @@ function processResolveWeakCall( path: NodePath, state: State, ): void { - const name = getModuleNameFromCallArgs(path); + const name = getModuleNameFromCallArgs(path, state.modeArgHelper); if (name == null) { throw new InvalidRequireCallError(path); @@ -493,7 +504,7 @@ function processImportCall( state: State, options: ImportDependencyOptions, ): void { - const name = getModuleNameFromCallArgs(path); + const name = getModuleNameFromCallArgs(path, state.modeArgHelper); if (name == null) { throw new InvalidRequireCallError(path); @@ -534,7 +545,7 @@ function processRequireCall( path: NodePath, state: State, ): void { - const name = getModuleNameFromCallArgs(path); + const name = getModuleNameFromCallArgs(path, state.modeArgHelper); const transformer = state.dependencyTransformer; @@ -721,9 +732,26 @@ function isNonNullishCallbackArg(arg: Node): boolean { return true; } -function getModuleNameFromCallArgs(path: NodePath): ?string { +function getModuleNameFromCallArgs( + path: NodePath, + modeArgHelper?: ?string, +): ?string { const args = path.get('arguments'); - if (!Array.isArray(args) || args.length !== 1) { + if (!Array.isArray(args) || args.length < 1) { + throw new InvalidRequireCallError(path); + } + + // Exactly one helper may carry a second argument, and only a literal `1`: + // the mode selector consumed by the runtime importDefault helper under + // `unstable_liveBindings`. Everything else - `require`, `import()`, + // `resolveWeak`, importAll - keeps the strict single-argument contract, so + // a malformed call like `require('x', 'anything')` still throws. Callers + // that omit `modeArgHelper` - including anyone using the public re-export - + // get that strict single-argument contract for every call. + // + // `isModeArgCall` only admits exactly two arguments, so three or more are + // already rejected here. + if (args.length > 1 && !isModeArgCall(path, modeArgHelper)) { throw new InvalidRequireCallError(path); } @@ -736,6 +764,31 @@ function getModuleNameFromCallArgs(path: NodePath): ?string { return null; } +/** + * True when this call is the configured mode-arg helper invoked as + * `helper(id, 1)`. Both the callee name and the literal value are checked, so + * neither an unrelated two-argument call nor a different numeric mode is + * mistaken for it. + */ +function isModeArgCall( + path: NodePath, + modeArgHelper: ?string, +): boolean { + if (modeArgHelper == null) { + return false; + } + const callee = path.node.callee; + if (callee.type !== 'Identifier' || callee.name !== modeArgHelper) { + return false; + } + const args = path.node.arguments; + return ( + args.length === 2 && + args[1].type === 'NumericLiteral' && + args[1].value === 1 + ); +} + collectDependencies.getModuleNameFromCallArgs = getModuleNameFromCallArgs; class InvalidRequireCallError extends Error { @@ -807,11 +860,22 @@ const DefaultDependencyTransformer: DependencyTransformer = { state: State, ): void { const moduleIDExpression = createModuleIDExpression(dependency, state); + const originalArgs = path.node.arguments; + // Decided before the arguments are replaced below, and through the same + // predicate `getModuleNameFromCallArgs` validates with, so the accepting + // and preserving sides cannot drift apart. In particular this is gated on + // the configured helper and callee name, not merely on a trailing `1`. + const hasModeArg = isModeArgCall(path, state.modeArgHelper); path.node.arguments = [moduleIDExpression] as Array< Expression | SpreadElement | ArgumentPlaceholder, >; - // Always add the debug name argument last - if (state.keepRequireNames) { + if (hasModeArg) { + // The runtime helper needs the mode arg at every call site; dropping it + // here would silently revert the namespace-shaped return to the + // value-shaped one after dependency inlining. + path.node.arguments.push(originalArgs[1]); + } else if (state.keepRequireNames) { + // Debug-name argument (dev builds only). path.node.arguments.push(types.stringLiteral(dependency.name)); } }, diff --git a/packages/metro/src/ModuleGraph/worker/visitDependencyUses.js b/packages/metro/src/ModuleGraph/worker/visitDependencyUses.js index f6c2d76822..d91f05d114 100644 --- a/packages/metro/src/ModuleGraph/worker/visitDependencyUses.js +++ b/packages/metro/src/ModuleGraph/worker/visitDependencyUses.js @@ -95,6 +95,9 @@ export default function visitDependencyUses( importAllParamBinding, depMapParamBinding, } = bindModuleIRElements(file); + // `importDefaultParamBinding` is a getter that redoes a scope lookup on each + // access, and this is read once per reference below - so resolve it once. + const modeArgHelperName = importDefaultParamBinding.identifier.name; for (const path of requireParamBinding.referencePaths.concat( importDefaultParamBinding.referencePaths, )) { @@ -107,7 +110,10 @@ export default function visitDependencyUses( if (dependencyFilter != null && !dependencyFilter(dep)) { continue; } - for (const {path: referencePath} of walkReferences(req.exprPath)) { + for (const {path: referencePath} of walkReferences( + req.exprPath, + modeArgHelperName, + )) { if ( referencePath.parentPath && referencePath.parentPath.node.type === 'CallExpression' && @@ -130,18 +136,40 @@ export default function visitDependencyUses( if (dependencyFilter != null && !dependencyFilter(dep)) { continue; } - for (const {path: referencePath} of walkReferences(req.exprPath)) { + for (const {path: referencePath} of walkReferences( + req.exprPath, + modeArgHelperName, + )) { visitOther(referencePath, dep); } } } +/** + * `modeArgHelperName` is the importDefault helper's local name - the only + * callee that may carry a mode argument. It is passed for `require` and + * importAll references too; those simply never match the callee check, which + * is what keeps the mode-1 walk from applying to them. + */ function* walkReferences( initialCandidateUse: NodePath<>, + modeArgHelperName: string, ): Iterable<{path: NodePath<>}> { const candidateUses = new Map>([ [initialCandidateUse.node, initialCandidateUse], ]); + // References that came from - directly or transitively via a constant + // binding - a mode-1 helper call (`_$_IMPORT_DEFAULT(id, 1)`). For these + // we walk through the `.default` accessor because the accessor holds the + // actual value; the wrapper's `.default` slot is a shape convention, not a + // meaningful semantic operation. We do NOT walk `.default` on arbitrary + // require results (e.g. `require("X").default` on a CJS module) because + // there the accessor IS the observable semantic operation and downstream + // analysers rely on seeing it. + const mode1Refs = new Set(); + if (isMode1HelperCall(initialCandidateUse.node, modeArgHelperName)) { + mode1Refs.add(initialCandidateUse.node); + } for (const p of candidateUses.values()) { const parentPath = nullthrows(p.parentPath); if ( @@ -154,6 +182,7 @@ function* walkReferences( parentPath.scope.getBinding(varIdNode.name), ); if (depBinding.constant) { + const isMode1Source = mode1Refs.has(p.node); for (const depRefPath of depBinding.referencePaths) { if (depRefPath.node === varIdNode) { continue; @@ -162,14 +191,51 @@ function* walkReferences( continue; } candidateUses.set(depRefPath.node, depRefPath); + if (isMode1Source) { + mode1Refs.add(depRefPath.node); + } } continue; } } + if ( + mode1Refs.has(p.node) && + parentPath.node.type === 'MemberExpression' && + parentPath.node.object === p.node && + !parentPath.node.computed && + parentPath.node.property.type === 'Identifier' && + parentPath.node.property.name === 'default' + ) { + if (!candidateUses.has(parentPath.node)) { + // Deliberately not added to `mode1Refs`. The accessor's value is the + // imported binding, not a further mode-1 wrapper, so a chained + // `helper(id, 1).default.default` must not have its second accessor + // walked through as well. + candidateUses.set(parentPath.node, parentPath); + } + continue; + } yield {path: p}; } } +/** + * True only for the importDefault helper invoked as `helper(id, 1)`. The + * callee is checked as well as the literal, so an unrelated two-argument call + * ending in `1` - `require(id)(x, 1)`, say - is not mistaken for the mode-1 + * shape and does not have a genuine `.default` swallowed. + */ +function isMode1HelperCall(node: Node, modeArgHelperName: string): boolean { + return ( + node.type === 'CallExpression' && + node.callee.type === 'Identifier' && + node.callee.name === modeArgHelperName && + node.arguments.length === 2 && + node.arguments[1].type === 'NumericLiteral' && + node.arguments[1].value === 1 + ); +} + function bindRequireCallElements( path: NodePath<>, {depMapParamBinding}: Readonly<{depMapParamBinding: ?Binding}>,