From f35992c450de70858e08eb3d70c673db886406ba Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Fri, 6 Sep 2024 03:42:39 -0700 Subject: [PATCH] Implement module-level memoization of inlined requires (#1346) Summary: Pull Request resolved: https://github.com/facebook/metro/pull/1346 A performance optimisation for `inlineRequries`, where we replace `const foo = require('./foo')` at top level with `var foo`, and replace subsequent references to `foo` with `(foo || foo = require('./foo')`. This differs from plain `inlineRequires` by the use of the memo variable, which (for modules that don't return a falsy value) saves calling into `require`, backed by `metroRequire`, and retrieving the previously-initialised module from a `Map` by its numeric ID. ``` - **[Experimental]:** Implement `transformer.unstable_memoizeInlineRequires` to optimise inlined access. ``` Reviewed By: javache Differential Revision: D62125134 fbshipit-source-id: 1ddbd8051d4a00b0ec358be8838f43eb8b0144a0 --- .../__snapshots__/loadConfig-test.js.snap | 4 + packages/metro-config/src/defaults/index.js | 1 + .../inline-requires-plugin-test.js.snap | 280 ++++++++++++++++- .../__tests__/inline-requires-plugin-test.js | 290 +++++++++--------- .../src/inline-requires-plugin.js | 159 +++++++++- packages/metro-transform-worker/src/index.js | 4 + .../metro-transform-worker/types/index.d.ts | 2 + 7 files changed, 572 insertions(+), 168 deletions(-) diff --git a/packages/metro-config/src/__tests__/__snapshots__/loadConfig-test.js.snap b/packages/metro-config/src/__tests__/__snapshots__/loadConfig-test.js.snap index 97e0e9a54f..0d182cdcbe 100644 --- a/packages/metro-config/src/__tests__/__snapshots__/loadConfig-test.js.snap +++ b/packages/metro-config/src/__tests__/__snapshots__/loadConfig-test.js.snap @@ -152,6 +152,7 @@ Object { "unstable_dependencyMapReservedName": null, "unstable_disableModuleWrapping": false, "unstable_disableNormalizePseudoGlobals": false, + "unstable_memoizeInlineRequires": false, "unstable_renameRequire": true, "unstable_workerThreads": false, "workerPath": "metro/src/DeltaBundler/Worker", @@ -334,6 +335,7 @@ Object { "unstable_dependencyMapReservedName": null, "unstable_disableModuleWrapping": false, "unstable_disableNormalizePseudoGlobals": false, + "unstable_memoizeInlineRequires": false, "unstable_renameRequire": true, "unstable_workerThreads": false, "workerPath": "metro/src/DeltaBundler/Worker", @@ -516,6 +518,7 @@ Object { "unstable_dependencyMapReservedName": null, "unstable_disableModuleWrapping": false, "unstable_disableNormalizePseudoGlobals": false, + "unstable_memoizeInlineRequires": false, "unstable_renameRequire": true, "unstable_workerThreads": false, "workerPath": "metro/src/DeltaBundler/Worker", @@ -698,6 +701,7 @@ Object { "unstable_dependencyMapReservedName": null, "unstable_disableModuleWrapping": false, "unstable_disableNormalizePseudoGlobals": false, + "unstable_memoizeInlineRequires": false, "unstable_renameRequire": true, "unstable_workerThreads": false, "workerPath": "metro/src/DeltaBundler/Worker", diff --git a/packages/metro-config/src/defaults/index.js b/packages/metro-config/src/defaults/index.js index 4af8af254f..a480e33153 100644 --- a/packages/metro-config/src/defaults/index.js +++ b/packages/metro-config/src/defaults/index.js @@ -137,6 +137,7 @@ const getDefaultValues = (projectRoot: ?string): ConfigT => ({ unstable_disableNormalizePseudoGlobals: false, unstable_renameRequire: true, unstable_compactOutput: false, + unstable_memoizeInlineRequires: false, unstable_workerThreads: false, }, watcher: { diff --git a/packages/metro-transform-plugins/src/__tests__/__snapshots__/inline-requires-plugin-test.js.snap b/packages/metro-transform-plugins/src/__tests__/__snapshots__/inline-requires-plugin-test.js.snap index 63eef5195d..a1742ded0d 100644 --- a/packages/metro-transform-plugins/src/__tests__/__snapshots__/inline-requires-plugin-test.js.snap +++ b/packages/metro-transform-plugins/src/__tests__/__snapshots__/inline-requires-plugin-test.js.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`inline-requires delete unused requires: delete unused requires 1`] = ` +exports[`memoizeCalls=false: inline-requires delete unused requires: delete unused requires 1`] = ` " var foo = require(\\"foo\\"); @@ -10,7 +10,7 @@ var foo = require(\\"foo\\"); " `; -exports[`inline-requires ensures that the inlined require still points to the global require function even if local require is not called: ensures that the inlined require still points to the global require function even if local require is not called 1`] = ` +exports[`memoizeCalls=false: inline-requires ensures that the inlined require still points to the global require function even if local require is not called: ensures that the inlined require still points to the global require function even if local require is not called 1`] = ` " const foo = require('foo'); @@ -37,7 +37,7 @@ function test() { " `; -exports[`inline-requires ensures that the inlined require still points to the global require function with inlineableCalls options: ensures that the inlined require still points to the global require function with inlineableCalls options 1`] = ` +exports[`memoizeCalls=false: inline-requires ensures that the inlined require still points to the global require function with inlineableCalls options: ensures that the inlined require still points to the global require function with inlineableCalls options 1`] = ` " const foo = customStuff('foo'); @@ -64,7 +64,7 @@ function test() { " `; -exports[`inline-requires ensures that the inlined require still points to the global require function: ensures that the inlined require still points to the global require function 1`] = ` +exports[`memoizeCalls=false: inline-requires ensures that the inlined require still points to the global require function: ensures that the inlined require still points to the global require function 1`] = ` " const foo = require('foo'); @@ -91,7 +91,7 @@ function test() { " `; -exports[`inline-requires ignores require properties (as identifiers) that are re-assigned: ignores require properties (as identifiers) that are re-assigned 1`] = ` +exports[`memoizeCalls=false: inline-requires ignores require properties (as identifiers) that are re-assigned: ignores require properties (as identifiers) that are re-assigned 1`] = ` " var X = require(\\"X\\"); var origA = X.a @@ -108,7 +108,7 @@ require(\\"X\\").a = function () { " `; -exports[`inline-requires ignores require properties (as strings) that are re-assigned: ignores require properties (as strings) that are re-assigned 1`] = ` +exports[`memoizeCalls=false: inline-requires ignores require properties (as strings) that are re-assigned: ignores require properties (as strings) that are re-assigned 1`] = ` " var X = require(\\"X\\"); var origA = X[\\"a\\"] @@ -125,7 +125,7 @@ require(\\"X\\")[\\"a\\"] = function () { " `; -exports[`inline-requires inlines any number of variable declarations: inlines any number of variable declarations 1`] = ` +exports[`memoizeCalls=false: inline-requires inlines any number of variable declarations: inlines any number of variable declarations 1`] = ` " var foo = require(\\"foo\\"), bar = require(\\"bar\\"), baz = 4; foo.method() @@ -137,7 +137,7 @@ require(\\"foo\\").method(); " `; -exports[`inline-requires inlines functions provided via \`inlineableCalls\`: inlines functions provided via \`inlineableCalls\` 1`] = ` +exports[`memoizeCalls=false: inline-requires inlines functions provided via \`inlineableCalls\`: inlines functions provided via \`inlineableCalls\` 1`] = ` " const inlinedCustom = customStuff(\\"foo\\"); const inlinedRequire = require(\\"bar\\"); @@ -152,7 +152,7 @@ require(\\"bar\\")(); " `; -exports[`inline-requires inlines multiple usages: inlines multiple usages 1`] = ` +exports[`memoizeCalls=false: inline-requires inlines multiple usages: inlines multiple usages 1`] = ` " var foo = require(\\"foo\\"); foo.bar() @@ -165,7 +165,7 @@ require(\\"foo\\").baz(); " `; -exports[`inline-requires inlines require properties: inlines require properties 1`] = ` +exports[`memoizeCalls=false: inline-requires inlines require properties: inlines require properties 1`] = ` " var tmp = require(\\"./a\\"); var a = tmp.a @@ -183,7 +183,7 @@ var D = { " `; -exports[`inline-requires inlines require.resolve calls: inlines require.resolve calls 1`] = ` +exports[`memoizeCalls=false: inline-requires inlines require.resolve calls: inlines require.resolve calls 1`] = ` " const a = require(require.resolve(\\"Foo\\")).bar; @@ -195,7 +195,7 @@ require(require.resolve(\\"Foo\\")).bar(); " `; -exports[`inline-requires inlines requires that are referenced before the require statement: inlines requires that are referenced before the require statement 1`] = ` +exports[`memoizeCalls=false: inline-requires inlines requires that are referenced before the require statement: inlines requires that are referenced before the require statement 1`] = ` " function foo() { bar(); @@ -214,7 +214,7 @@ require(\\"baz\\")(); " `; -exports[`inline-requires inlines single usage: inlines single usage 1`] = ` +exports[`memoizeCalls=false: inline-requires inlines single usage: inlines single usage 1`] = ` " var foo = require(\\"foo\\"); foo.bar() @@ -225,7 +225,7 @@ require(\\"foo\\").bar(); " `; -exports[`inline-requires inlines with multiple arguments: inlines with multiple arguments 1`] = ` +exports[`memoizeCalls=false: inline-requires inlines with multiple arguments: inlines with multiple arguments 1`] = ` " const a = require(\\"Foo\\", \\"Bar\\", 47); @@ -236,3 +236,255 @@ a(); require(\\"Foo\\", \\"Bar\\", 47)(); " `; + +exports[`memoizeCalls=true: inline-requires delete unused requires: delete unused requires 1`] = ` +" +var foo = require(\\"foo\\"); + + ↓ ↓ ↓ ↓ ↓ ↓ + + +" +`; + +exports[`memoizeCalls=true: inline-requires ensures that the inlined require still points to the global require function even if local require is not called: ensures that the inlined require still points to the global require function even if local require is not called 1`] = ` +" +const foo = require('foo'); + +function test() { + function require(condition) { + if (!condition) { + throw new Error('Condition is falsy'); + } + } + + foo.isOnline(); +} + + ↓ ↓ ↓ ↓ ↓ ↓ + +var foo; +function test() { + function _require(condition) { + if (!condition) { + throw new Error('Condition is falsy'); + } + } + (foo || (foo = require('foo'))).isOnline(); +} +" +`; + +exports[`memoizeCalls=true: inline-requires ensures that the inlined require still points to the global require function with inlineableCalls options: ensures that the inlined require still points to the global require function with inlineableCalls options 1`] = ` +" +const foo = customStuff('foo'); + +function test() { + function customStuff(condition) { + if (!condition) { + throw new Error('Condition is falsy'); + } + } + + customStuff(foo.isOnline()); +} + + ↓ ↓ ↓ ↓ ↓ ↓ + +var foo; +function test() { + function _customStuff(condition) { + if (!condition) { + throw new Error('Condition is falsy'); + } + } + _customStuff((foo || (foo = customStuff('foo'))).isOnline()); +} +" +`; + +exports[`memoizeCalls=true: inline-requires ensures that the inlined require still points to the global require function: ensures that the inlined require still points to the global require function 1`] = ` +" +const foo = require('foo'); + +function test() { + function require(condition) { + if (!condition) { + throw new Error('Condition is falsy'); + } + } + + require(foo.isOnline()); +} + + ↓ ↓ ↓ ↓ ↓ ↓ + +var foo; +function test() { + function _require(condition) { + if (!condition) { + throw new Error('Condition is falsy'); + } + } + _require((foo || (foo = require('foo'))).isOnline()); +} +" +`; + +exports[`memoizeCalls=true: inline-requires ignores require properties (as identifiers) that are re-assigned: ignores require properties (as identifiers) that are re-assigned 1`] = ` +" +var X = require(\\"X\\"); +var origA = X.a +X.a = function() { + origA(); +}; + + ↓ ↓ ↓ ↓ ↓ ↓ + +var X; +var origA = (X || (X = require(\\"X\\"))).a; +X.a = function () { + origA(); +}; +" +`; + +exports[`memoizeCalls=true: inline-requires ignores require properties (as strings) that are re-assigned: ignores require properties (as strings) that are re-assigned 1`] = ` +" +var X = require(\\"X\\"); +var origA = X[\\"a\\"] +X[\\"a\\"] = function() { + origA(); +}; + + ↓ ↓ ↓ ↓ ↓ ↓ + +var X; +var origA = (X || (X = require(\\"X\\")))[\\"a\\"]; +X[\\"a\\"] = function () { + origA(); +}; +" +`; + +exports[`memoizeCalls=true: inline-requires inlines any number of variable declarations: inlines any number of variable declarations 1`] = ` +" +var foo = require(\\"foo\\"), bar = require(\\"bar\\"), baz = 4; +foo.method() + + ↓ ↓ ↓ ↓ ↓ ↓ + +var foo; +var baz = 4; +(foo || (foo = require(\\"foo\\"))).method(); +" +`; + +exports[`memoizeCalls=true: inline-requires inlines functions provided via \`inlineableCalls\`: inlines functions provided via \`inlineableCalls\` 1`] = ` +" +const inlinedCustom = customStuff(\\"foo\\"); +const inlinedRequire = require(\\"bar\\"); + +inlinedCustom(); +inlinedRequire(); + + ↓ ↓ ↓ ↓ ↓ ↓ + +var inlinedRequire; +var inlinedCustom; +(inlinedCustom || (inlinedCustom = customStuff(\\"foo\\")))(); +(inlinedRequire || (inlinedRequire = require(\\"bar\\")))(); +" +`; + +exports[`memoizeCalls=true: inline-requires inlines multiple usages: inlines multiple usages 1`] = ` +" +var foo = require(\\"foo\\"); +foo.bar() +foo.baz() + + ↓ ↓ ↓ ↓ ↓ ↓ + +var foo; +(foo || (foo = require(\\"foo\\"))).bar(); +foo.baz(); +" +`; + +exports[`memoizeCalls=true: inline-requires inlines require properties: inlines require properties 1`] = ` +" +var tmp = require(\\"./a\\"); +var a = tmp.a +var D = { + b: function(c) { c ? a(c.toString()) : a(\\"No c!\\"); }, +}; + + ↓ ↓ ↓ ↓ ↓ ↓ + +var tmp; +var a = (tmp || (tmp = require(\\"./a\\"))).a; +var D = { + b: function (c) { + c ? a(c.toString()) : a(\\"No c!\\"); + } +}; +" +`; + +exports[`memoizeCalls=true: inline-requires inlines require.resolve calls: inlines require.resolve calls 1`] = ` +" +const a = require(require.resolve(\\"Foo\\")).bar; + +a(); + + ↓ ↓ ↓ ↓ ↓ ↓ + +var a; +(a || (a = require(require.resolve(\\"Foo\\")).bar))(); +" +`; + +exports[`memoizeCalls=true: inline-requires inlines requires that are referenced before the require statement: inlines requires that are referenced before the require statement 1`] = ` +" +function foo() { + bar(); +} +var bar = require(\\"baz\\"); +foo(); +bar(); + + ↓ ↓ ↓ ↓ ↓ ↓ + +var bar; +function foo() { + (bar || (bar = require(\\"baz\\")))(); +} +foo(); +(bar || (bar = require(\\"baz\\")))(); +" +`; + +exports[`memoizeCalls=true: inline-requires inlines single usage: inlines single usage 1`] = ` +" +var foo = require(\\"foo\\"); +foo.bar() + + ↓ ↓ ↓ ↓ ↓ ↓ + +var foo; +(foo || (foo = require(\\"foo\\"))).bar(); +" +`; + +exports[`memoizeCalls=true: inline-requires inlines with multiple arguments: inlines with multiple arguments 1`] = ` +" +const a = require(\\"Foo\\", \\"Bar\\", 47); + +a(); + + ↓ ↓ ↓ ↓ ↓ ↓ + +var a; +(a || (a = require(\\"Foo\\", \\"Bar\\", 47)))(); +" +`; diff --git a/packages/metro-transform-plugins/src/__tests__/inline-requires-plugin-test.js b/packages/metro-transform-plugins/src/__tests__/inline-requires-plugin-test.js index 9dfa84e828..1f21e12eb4 100644 --- a/packages/metro-transform-plugins/src/__tests__/inline-requires-plugin-test.js +++ b/packages/metro-transform-plugins/src/__tests__/inline-requires-plugin-test.js @@ -11,6 +11,7 @@ 'use strict'; import type {PluginOptions, State} from '../inline-requires-plugin'; +import type {PluginTesterOptions} from 'babel-plugin-tester'; const inlineRequiresPlugin = require('../inline-requires-plugin'); const validateOutputAst = require('./validateOutputAst'); @@ -18,53 +19,45 @@ const babel = require('@babel/core'); const pluginTester = require('babel-plugin-tester'); const nullthrows = require('nullthrows'); -pluginTester({ - babelOptions: { - babelrc: false, - configFile: false, - }, - plugin: inlineRequiresPlugin, - pluginOptions: { - ignoredRequires: ['CommonFoo'], - inlineableCalls: ['customStuff'], +type TestCases = PluginTesterOptions['tests']; + +const TEST_CASES: TestCases = { + 'inlines single usage': { + code: ['var foo = require("foo");', 'foo.bar()'].join('\n'), + snapshot: true, }, - tests: { - 'inlines single usage': { - code: ['var foo = require("foo");', 'foo.bar()'].join('\n'), - snapshot: true, - }, - 'inlines multiple usages': { - code: ['var foo = require("foo");', 'foo.bar()', 'foo.baz()'].join('\n'), - snapshot: true, - }, + 'inlines multiple usages': { + code: ['var foo = require("foo");', 'foo.bar()', 'foo.baz()'].join('\n'), + snapshot: true, + }, - 'inlines any number of variable declarations': { - code: [ - 'var foo = require("foo"), bar = require("bar"), baz = 4;', - 'foo.method()', - ].join('\n'), - snapshot: true, - }, + 'inlines any number of variable declarations': { + code: [ + 'var foo = require("foo"), bar = require("bar"), baz = 4;', + 'foo.method()', + ].join('\n'), + snapshot: true, + }, - 'ignores requires that are not assigned': { - code: ['require("foo");'].join('\n'), - snapshot: false, - }, + 'ignores requires that are not assigned': { + code: ['require("foo");'].join('\n'), + snapshot: false, + }, - 'delete unused requires': { - code: ['var foo = require("foo");'].join('\n'), - snapshot: true, - }, + 'delete unused requires': { + code: ['var foo = require("foo");'].join('\n'), + snapshot: true, + }, - 'ignores requires that are re-assigned': { - code: ['var foo = require("foo");', 'foo = "bar";'].join('\n'), - snapshot: false, - }, + 'ignores requires that are re-assigned': { + code: ['var foo = require("foo");', 'foo = "bar";'].join('\n'), + snapshot: false, + }, - 'ensures that the inlined require still points to the global require function': - { - code: ` + 'ensures that the inlined require still points to the global require function': + { + code: ` const foo = require('foo'); function test() { @@ -77,12 +70,12 @@ pluginTester({ require(foo.isOnline()); } `, - snapshot: true, - }, + snapshot: true, + }, - 'ensures that the inlined require still points to the global require function with inlineableCalls options': - { - code: ` + 'ensures that the inlined require still points to the global require function with inlineableCalls options': + { + code: ` const foo = customStuff('foo'); function test() { @@ -95,12 +88,12 @@ pluginTester({ customStuff(foo.isOnline()); } `, - snapshot: true, - }, + snapshot: true, + }, - 'ensures that the inlined require still points to the global require function even if local require is not called': - { - code: ` + 'ensures that the inlined require still points to the global require function even if local require is not called': + { + code: ` const foo = require('foo'); function test() { @@ -113,12 +106,12 @@ pluginTester({ foo.isOnline(); } `, - snapshot: true, - }, + snapshot: true, + }, - 'does not transform require calls if require is redeclared in the same declaration scope': - { - code: ` + 'does not transform require calls if require is redeclared in the same declaration scope': + { + code: ` function require(condition) { if (!condition) { throw new Error('Condition is falsy'); @@ -127,12 +120,12 @@ pluginTester({ const foo = require('foo'); console.log(foo.test); `, - snapshot: false, - }, + snapshot: false, + }, - 'does not transform require calls if require is redeclared in the global scope': - { - code: ` + 'does not transform require calls if require is redeclared in the global scope': + { + code: ` function require(condition) { if (!condition) { throw new Error('Condition is falsy'); @@ -143,11 +136,11 @@ pluginTester({ console.log(foo.test); } `, - snapshot: false, - }, + snapshot: false, + }, - 'does not transform require calls that are already inline': { - code: ` + 'does not transform require calls that are already inline': { + code: ` function test() { function require(condition) { if (!condition) { @@ -157,93 +150,108 @@ pluginTester({ require('test'); } `, - snapshot: false, - }, + snapshot: false, + }, - 'inlines requires that are referenced before the require statement': { - code: [ - 'function foo() {', - ' bar();', - '}', - 'var bar = require("baz");', - 'foo();', - 'bar();', - ].join('\n'), - snapshot: true, - }, + 'inlines requires that are referenced before the require statement': { + code: [ + 'function foo() {', + ' bar();', + '}', + 'var bar = require("baz");', + 'foo();', + 'bar();', + ].join('\n'), + snapshot: true, + }, - 'inlines require properties': { - code: [ - 'var tmp = require("./a");', - 'var a = tmp.a', - 'var D = {', - ' b: function(c) { c ? a(c.toString()) : a("No c!"); },', - '};', - ].join('\n'), - snapshot: true, - }, + 'inlines require properties': { + code: [ + 'var tmp = require("./a");', + 'var a = tmp.a', + 'var D = {', + ' b: function(c) { c ? a(c.toString()) : a("No c!"); },', + '};', + ].join('\n'), + snapshot: true, + }, - 'ignores require properties (as identifiers) that are re-assigned': { - code: [ - 'var X = require("X");', - 'var origA = X.a', - 'X.a = function() {', - ' origA();', - '};', - ].join('\n'), - snapshot: true, - }, + 'ignores require properties (as identifiers) that are re-assigned': { + code: [ + 'var X = require("X");', + 'var origA = X.a', + 'X.a = function() {', + ' origA();', + '};', + ].join('\n'), + snapshot: true, + }, - 'ignores require properties (as strings) that are re-assigned': { - code: [ - 'var X = require("X");', - 'var origA = X["a"]', - 'X["a"] = function() {', - ' origA();', - '};', - ].join('\n'), - snapshot: true, - }, + 'ignores require properties (as strings) that are re-assigned': { + code: [ + 'var X = require("X");', + 'var origA = X["a"]', + 'X["a"] = function() {', + ' origA();', + '};', + ].join('\n'), + snapshot: true, + }, - 'inlines functions provided via `inlineableCalls`': { - code: [ - 'const inlinedCustom = customStuff("foo");', - 'const inlinedRequire = require("bar");', - '', - 'inlinedCustom();', - 'inlinedRequire();', - ].join('\n'), - snapshot: true, - }, + 'inlines functions provided via `inlineableCalls`': { + code: [ + 'const inlinedCustom = customStuff("foo");', + 'const inlinedRequire = require("bar");', + '', + 'inlinedCustom();', + 'inlinedRequire();', + ].join('\n'), + snapshot: true, + }, - 'ignores requires in `ignoredRequires`': { - code: ['const CommonFoo = require("CommonFoo");', 'CommonFoo();'].join( - '\n', - ), - snapshot: false, - }, + 'ignores requires in `ignoredRequires`': { + code: ['const CommonFoo = require("CommonFoo");', 'CommonFoo();'].join( + '\n', + ), + snapshot: false, + }, - 'ignores destructured properties of requires in `ignoredRequires`': { - code: [ - 'const tmp = require("CommonFoo");', - 'const a = require("CommonFoo").a;', - 'a();', - ].join('\n'), - snapshot: false, - }, + 'ignores destructured properties of requires in `ignoredRequires`': { + code: [ + 'const tmp = require("CommonFoo");', + 'const a = require("CommonFoo").a;', + 'a();', + ].join('\n'), + snapshot: false, + }, - 'inlines require.resolve calls': { - code: ['const a = require(require.resolve("Foo")).bar;', '', 'a();'].join( - '\n', - ), - snapshot: true, - }, + 'inlines require.resolve calls': { + code: ['const a = require(require.resolve("Foo")).bar;', '', 'a();'].join( + '\n', + ), + snapshot: true, + }, - 'inlines with multiple arguments': { - code: ['const a = require("Foo", "Bar", 47);', '', 'a();'].join('\n'), - snapshot: true, - }, + 'inlines with multiple arguments': { + code: ['const a = require("Foo", "Bar", 47);', '', 'a();'].join('\n'), + snapshot: true, }, +}; + +describe.each([true, false])('memoizeCalls=%s:', memoizeCalls => { + pluginTester({ + babelOptions: { + babelrc: false, + configFile: false, + }, + plugin: inlineRequiresPlugin, + pluginOptions: { + ignoredRequires: ['CommonFoo'], + inlineableCalls: ['customStuff'], + memoizeCalls, + }, + tests: TEST_CASES, + }); }); describe('inline-requires', () => { diff --git a/packages/metro-transform-plugins/src/inline-requires-plugin.js b/packages/metro-transform-plugins/src/inline-requires-plugin.js index 97e3ae7821..082c273ce6 100644 --- a/packages/metro-transform-plugins/src/inline-requires-plugin.js +++ b/packages/metro-transform-plugins/src/inline-requires-plugin.js @@ -12,12 +12,14 @@ import type {PluginObj} from '@babel/core'; import typeof * as Babel from '@babel/core'; -import type {NodePath} from '@babel/traverse'; +import type {NodePath, Scope} from '@babel/traverse'; import type {Program} from '@babel/types'; +type Types = Babel['types']; export type PluginOptions = $ReadOnly<{ ignoredRequires?: $ReadOnlyArray, inlineableCalls?: $ReadOnlyArray, + memoizeCalls?: boolean, }>; export type State = { @@ -63,6 +65,7 @@ module.exports = ({types: t, traverse}: Babel): PluginObj => ({ exit(path: NodePath, state: State): void { const ignoredRequires = new Set(); const inlineableCalls = new Set(['require']); + let memoizeCalls = false; const opts = state.opts; if (opts != null) { @@ -76,8 +79,13 @@ module.exports = ({types: t, traverse}: Babel): PluginObj => ({ inlineableCalls.add(name); } } + memoizeCalls = opts.memoizeCalls ?? false; } + const programNode = path.scope.block; + if (programNode.type !== 'Program') { + return; + } path.scope.crawl(); path.traverse( { @@ -89,22 +97,26 @@ module.exports = ({types: t, traverse}: Babel): PluginObj => ({ if (parseResult == null) { return; } - const {declarationPath, moduleName, requireFnName} = parseResult; - const init = declarationPath.node.init; + const maybeInit = declarationPath.node.init; const name = declarationPath.node.id ? declarationPath.node.id.name : null; const binding = name == null ? null : declarationPath.scope.getBinding(name); - if (binding == null || binding.constantViolations.length > 0) { + if ( + maybeInit == null || + !t.isExpression(maybeInit) || + binding == null || + binding.constantViolations.length > 0 + ) { return; } - + const init: BabelNodeExpression = maybeInit; const initPath = declarationPath.get('init'); - if (init == null || Array.isArray(initPath)) { + if (Array.isArray(initPath)) { return; } @@ -117,14 +129,70 @@ module.exports = ({types: t, traverse}: Babel): PluginObj => ({ }); let thrown = false; + const memoVarName = parseResult.identifierName; + + // Whether the module has a "var foo" at program scope, used to + // store the result of a require call if memoizeCalls is true. + let hasMemoVar = false; + if ( + memoizeCalls && + // Don't add a var init statement if there are no references to + // the lvalue of the require assignment. + binding.referencePaths.length > 0 + ) { + // create var init statement + const varInitStmt = t.variableDeclaration('var', [ + t.variableDeclarator(t.identifier(memoVarName)), + ]); + // Must remove the declaration path + declarationPath.remove(); + hasMemoVar = addStmtToBlock(programNode, varInitStmt, 0); + } + + function getMemoOrCallExpr() { + const refExpr = t.cloneDeep(init); + // $FlowFixMe[prop-missing] + refExpr.METRO_INLINE_REQUIRES_INIT_LOC = initLoc; + return t.logicalExpression( + '||', + t.identifier(memoVarName), + t.assignmentExpression( + '=', + t.identifier(memoVarName), + refExpr, + ), + ); + } + + const scopesWithInlinedRequire = new Set(); for (const referencePath of binding.referencePaths) { excludeMemberAssignment(moduleName, referencePath, state); try { referencePath.scope.rename(requireFnName); - const refExpr = t.cloneDeep(init); - // $FlowFixMe[prop-missing] - refExpr.METRO_INLINE_REQUIRES_INIT_LOC = initLoc; - referencePath.replaceWith(refExpr); + if (hasMemoVar) { + referencePath.scope.rename(memoVarName); + // Swap the local reference with (v || v = require(m)), + // unless it is directly enclosed. + if (!isDirectlyEnclosedByBlock(t, referencePath)) { + referencePath.replaceWith(getMemoOrCallExpr()); + continue; + } + // if the current scope already has a (v || v = require(m)) + // expression for module m, use identifier reference v + // instead. Else use the full (v || v = require(m)) and + // register the current scope for subsequent references. + if (scopesWithInlinedRequire.has(referencePath.scope)) { + referencePath.replaceWith(t.identifier(memoVarName)); + } else { + referencePath.replaceWith(getMemoOrCallExpr()); + scopesWithInlinedRequire.add(referencePath.scope); + } + } else { + const refExpr = t.cloneDeep(init); + // $FlowFixMe[prop-missing] + refExpr.METRO_INLINE_REQUIRES_INIT_LOC = initLoc; + referencePath.replaceWith(refExpr); + } } catch (error) { thrown = true; } @@ -132,7 +200,7 @@ module.exports = ({types: t, traverse}: Babel): PluginObj => ({ // If a replacement failed (e.g. replacing a type annotation), // avoid removing the initial require just to be safe. - if (!thrown) { + if (!thrown && declarationPath.node != null) { declarationPath.remove(); } }, @@ -209,6 +277,7 @@ function parseInlineableAlias( declarationPath: NodePath, moduleName: string, requireFnName: string, + identifierName: string, } { const module = getInlineableModule(path, state); if (module == null) { @@ -225,9 +294,19 @@ function parseInlineableAlias( return null; } + if (path.parent.type !== 'VariableDeclarator') { + return null; + } + + const variableDeclarator = path.parent; + + if (variableDeclarator.id.type !== 'Identifier') { + return null; + } + + const identifier = variableDeclarator.id; + const isValid = - path.parent.type === 'VariableDeclarator' && - path.parent.id.type === 'Identifier' && parentPath.parent.type === 'VariableDeclaration' && grandParentPath.parent.type === 'Program'; @@ -237,6 +316,7 @@ function parseInlineableAlias( declarationPath: parentPath, moduleName, requireFnName, + identifierName: identifier.name, }; } @@ -247,6 +327,7 @@ function parseInlineableMemberAlias( declarationPath: NodePath, moduleName: string, requireFnName: string, + identifierName: string, } { const module = getInlineableModule(path, state); if (module == null) { @@ -279,6 +360,8 @@ function parseInlineableMemberAlias( return null; } + const identifier = variableDeclarator.id; + if ( grandParentPath.parent.type !== 'VariableDeclaration' || grandParentPath.parentPath?.parent.type !== 'Program' || @@ -296,6 +379,7 @@ function parseInlineableMemberAlias( declarationPath: grandParentPath, moduleName, requireFnName, + identifierName: identifier.name, }; } @@ -363,3 +447,52 @@ function getNearestLocFromPath(path: NodePath<>): ?BabelSourceLocation { } return current?.node.loc; } + +// check if a node is a branch +function isBranch(t: Types, node: BabelNode) { + return ( + t.isIfStatement(node) || + t.isLogicalExpression(node) || + t.isConditionalExpression(node) || + t.isSwitchStatement(node) || + t.isSwitchCase(node) || + t.isForStatement(node) || + t.isForInStatement(node) || + t.isForOfStatement(node) || + t.isWhileStatement(node) + ); +} + +function isDirectlyEnclosedByBlock(t: Types, path: NodePath) { + let curPath: ?NodePath = path; + while (curPath) { + if (isBranch(t, curPath.node)) { + return false; + } + if (t.isBlockStatement(curPath.node)) { + return true; + } + curPath = curPath.parentPath; + } + return true; +} + +// insert statement to the beginning of the scope block +function addStmtToBlock( + block: BabelNodeProgram, + stmt: BabelNodeStatement, + idx: number, +): boolean { + const scopeBody = block.body; + if (Array.isArray(scopeBody)) { + // if the code is inside global scope + scopeBody.splice(idx, 0, stmt); + return true; + } else if (scopeBody && Array.isArray(scopeBody.body)) { + // if the code is inside function scope + scopeBody.body.splice(idx, 0, stmt); + return true; + } else { + return false; + } +} diff --git a/packages/metro-transform-worker/src/index.js b/packages/metro-transform-worker/src/index.js index 903b65e64b..db6f691411 100644 --- a/packages/metro-transform-worker/src/index.js +++ b/packages/metro-transform-worker/src/index.js @@ -96,6 +96,8 @@ export type JsTransformerConfig = $ReadOnly<{ unstable_compactOutput: boolean, /** Enable `require.context` statements which can be used to import multiple files in a directory. */ unstable_allowRequireContext: boolean, + /** With inlineRequires, enable a module-scope memo var and inline as (v || v=require('foo')) */ + unstable_memoizeInlineRequires?: boolean, /** Whether to rename scoped `require` functions to `_$$_REQUIRE`, usually an extraneous operation when serializing to iife (default). */ unstable_renameRequire?: boolean, }>; @@ -114,6 +116,7 @@ export type JsTransformOptions = $ReadOnly<{ platform: ?string, type: Type, unstable_disableES6Transforms?: boolean, + unstable_memoizeInlineRequires?: boolean, unstable_transformProfile: TransformProfile, }>; @@ -298,6 +301,7 @@ async function transformJS( { ...babelPluginOpts, ignoredRequires: options.nonInlinedRequires, + memoizeCalls: options.unstable_memoizeInlineRequires, }, ]); } diff --git a/packages/metro-transform-worker/types/index.d.ts b/packages/metro-transform-worker/types/index.d.ts index 125f0caecf..f7db52fc14 100644 --- a/packages/metro-transform-worker/types/index.d.ts +++ b/packages/metro-transform-worker/types/index.d.ts @@ -64,6 +64,8 @@ export type JsTransformerConfig = Readonly<{ unstable_compactOutput: boolean; /** Enable `require.context` statements which can be used to import multiple files in a directory. */ unstable_allowRequireContext: boolean; + /** With inlineRequires, enable a module-scope memo var and inline as (v || v=require('foo')) */ + unstable_memoizeInlineRequires?: boolean; /** Whether to rename scoped `require` functions to `_$$_REQUIRE`, usually an extraneous operation when serializing to iife (default). */ unstable_renameRequire?: boolean; }>;