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 ( 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(); + } + } + } + } }, }, }, 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}>,