Allow blocklisting of target modules from inline-requires memoisation

Summary: Add a new transform options `unstable_nonMemozedInlineRequires` to complement `unstable_memoizeInlineRequires`. If inline requires and memoization are enabled, this list prevents the named modules (import specifiers) from being memoized in a module-scoped `var`, while allowing them to be inlined. Facebook In particular, allow `sx` to be *inlined* but not *memoised*, because extraction of sound resources from a bundle works by static analysis of `sx` imports, and that (currently) doesn't support the memoised form: https://www.internalfb.com/code/fbsource/[9f890d033f51]/xplat/js/tools/metro-buck-transform-worker/src/analyzers/extractSoundResources.js?lines=51 Reviewed By: GijsWeterings Differential Revision: D62637023 fbshipit-source-id: 9351e5a9a358d8bd869f23a226929e8cff3f41cf
facebook · Sep 13, 2024 · 5dbb344 · 5dbb344
1 parent 8cacee1
commit 5dbb344
Show file tree

Hide file tree

Showing 7 changed files with 70 additions and 20 deletions.
diff --git a/packages/metro-config/src/configTypes.flow.js b/packages/metro-config/src/configTypes.flow.js
@@ -38,6 +38,7 @@ export type ExtraTransformOptions = $ReadOnly<{
     nonInlinedRequires?: $ReadOnlyArray<string>,
     unstable_disableES6Transforms?: boolean,
     unstable_memoizeInlineRequires?: boolean,
+    unstable_nonMemoizedInlineRequires?: $ReadOnlyArray<string>,
   }>,
   ...
 }>;

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
@@ -11,6 +11,7 @@
 'use strict';
 
 import type {PluginOptions, State} from '../inline-requires-plugin';
+import type {Plugins} from '@babel/core';
 import type {PluginTesterOptions} from 'babel-plugin-tester';
 
 const inlineRequiresPlugin = require('../inline-requires-plugin');
@@ -257,25 +258,21 @@ describe.each([true, false])('memoizeCalls=%s:', memoizeCalls => {
 describe('inline-requires', () => {
   const transform = (
     source: $ReadOnlyArray<string>,
-    options?: $ReadOnly<{...}>,
+    plugins?: Plugins = [[inlineRequiresPlugin, {}]],
   ) =>
     babel.transformSync(source.join('\n'), {
       ast: true,
       compact: true,
-      plugins: [
-        // $FlowFixMe[untyped-import] @babel/plugin-transform-modules-commonjs
-        [require('@babel/plugin-transform-modules-commonjs'), {strict: false}],
-        [inlineRequiresPlugin, options],
-      ],
+      plugins,
     });
 
   const compare = (
     input: $ReadOnlyArray<string>,
     output: $ReadOnlyArray<string>,
-    options?: $ReadOnly<{...}>,
+    plugins?: Plugins = [[inlineRequiresPlugin, {}]],
   ) => {
-    expect(transform(input, options).code).toBe(
-      transform(output, options).code,
+    expect(transform(input, plugins).code).toBe(
+      transform(output, plugins).code,
     );
   };
 
@@ -287,6 +284,11 @@ describe('inline-requires', () => {
         'function _interopRequireDefault(e) { return e && e.__esModule ? e : { default: e }; }',
         'console.log(_foo.default);',
       ],
+      [
+        // $FlowFixMe[untyped-import] @babel/plugin-transform-modules-commonjs
+        [require('@babel/plugin-transform-modules-commonjs'), {strict: false}],
+        [inlineRequiresPlugin, {}],
+      ],
     );
   });
 
@@ -305,6 +307,11 @@ describe('inline-requires', () => {
         '  }',
         '};',
       ],
+      [
+        // $FlowFixMe[untyped-import] @babel/plugin-transform-modules-commonjs
+        [require('@babel/plugin-transform-modules-commonjs'), {strict: false}],
+        [inlineRequiresPlugin, {}],
+      ],
     );
   });
 
@@ -333,4 +340,36 @@ describe('inline-requires', () => {
     ]).ast;
     validateOutputAst(nullthrows(ast));
   });
+
+  it('respects nonMemoizedModules', function () {
+    expect(
+      transform(
+        [
+          'const foo = require("foo");',
+          'const noMemo = require("noMemo");',
+          'module.exports = function() {',
+          '  foo();',
+          '  noMemo();',
+          '};',
+        ],
+        [
+          [
+            inlineRequiresPlugin,
+            {
+              memoizeCalls: true,
+              nonMemoizedModules: ['noMemo'],
+            },
+          ],
+        ],
+      ).code.replace('"use strict";', ''),
+    ).toEqual(
+      [
+        'var foo;',
+        'module.exports=function(){',
+        '(foo||(foo=require("foo")))();',
+        'require("noMemo")();',
+        '};',
+      ].join(''),
+    );
+  });
 });
diff --git a/packages/metro-transform-plugins/src/inline-requires-plugin.js b/packages/metro-transform-plugins/src/inline-requires-plugin.js
@@ -19,6 +19,7 @@ type Types = Babel['types'];
 export type PluginOptions = $ReadOnly<{
   ignoredRequires?: $ReadOnlyArray<string>,
   inlineableCalls?: $ReadOnlyArray<string>,
+  nonMemoizedModules?: $ReadOnlyArray<string>,
   memoizeCalls?: boolean,
 }>;
 
@@ -65,20 +66,16 @@ module.exports = ({types: t, traverse}: Babel): PluginObj<State> => ({
       exit(path: NodePath<Program>, state: State): void {
         const ignoredRequires = new Set<string>();
         const inlineableCalls = new Set(['require']);
+        const nonMemoizedModules = new Set<string>();
         let memoizeCalls = false;
         const opts = state.opts;
 
         if (opts != null) {
-          if (opts.ignoredRequires != null) {
-            for (const name of opts.ignoredRequires) {
-              ignoredRequires.add(name);
-            }
-          }
-          if (opts.inlineableCalls != null) {
-            for (const name of opts.inlineableCalls) {
-              inlineableCalls.add(name);
-            }
-          }
+          opts.ignoredRequires?.forEach(name => ignoredRequires.add(name));
+          opts.inlineableCalls?.forEach(name => inlineableCalls.add(name));
+          opts.nonMemoizedModules?.forEach(name =>
+            nonMemoizedModules.add(name),
+          );
           memoizeCalls = opts.memoizeCalls ?? false;
         }
 
@@ -138,7 +135,10 @@ module.exports = ({types: t, traverse}: Babel): PluginObj<State> => ({
                 memoizeCalls &&
                 // Don't add a var init statement if there are no references to
                 // the lvalue of the require assignment.
-                binding.referencePaths.length > 0
+                binding.referencePaths.length > 0 &&
+                // Some modules should never be memoized even though they
+                // may be inlined.
+                !nonMemoizedModules.has(moduleName)
               ) {
                 // create var init statement
                 const varInitStmt = t.variableDeclaration('var', [

diff --git a/packages/metro-transform-worker/src/index.js b/packages/metro-transform-worker/src/index.js
@@ -98,6 +98,8 @@ export type JsTransformerConfig = $ReadOnly<{
   unstable_allowRequireContext: boolean,
   /** With inlineRequires, enable a module-scope memo var and inline as (v || v=require('foo')) */
   unstable_memoizeInlineRequires?: boolean,
+  /** With inlineRequires, do not memoize these module specifiers */
+  unstable_nonMemoizedInlineRequires?: $ReadOnlyArray<string>,
   /** Whether to rename scoped `require` functions to `_$$_REQUIRE`, usually an extraneous operation when serializing to iife (default). */
   unstable_renameRequire?: boolean,
 }>;
@@ -117,6 +119,7 @@ export type JsTransformOptions = $ReadOnly<{
   type: Type,
   unstable_disableES6Transforms?: boolean,
   unstable_memoizeInlineRequires?: boolean,
+  unstable_nonMemoizedInlineRequires?: $ReadOnlyArray<string>,
   unstable_transformProfile: TransformProfile,
 }>;
 
@@ -304,6 +307,7 @@ async function transformJS(
         memoizeCalls:
           options.customTransformOptions?.unstable_memoizeInlineRequires ??
           options.unstable_memoizeInlineRequires,
+        nonMemoizedModules: options.unstable_nonMemoizedInlineRequires,
       },
     ]);
   }

diff --git a/packages/metro-transform-worker/types/index.d.ts b/packages/metro-transform-worker/types/index.d.ts
@@ -66,6 +66,8 @@ export type JsTransformerConfig = Readonly<{
   unstable_allowRequireContext: boolean;
   /** With inlineRequires, enable a module-scope memo var and inline as (v || v=require('foo')) */
   unstable_memoizeInlineRequires?: boolean;
+  /** With inlineRequires, do not memoize these module specifiers */
+  unstable_nonMemoizedInlineRequires?: ReadonlyArray<string>;
   /** Whether to rename scoped `require` functions to `_$$_REQUIRE`, usually an extraneous operation when serializing to iife (default). */
   unstable_renameRequire?: boolean;
 }>;

diff --git a/packages/metro/src/DeltaBundler/Transformer.js b/packages/metro/src/DeltaBundler/Transformer.js
@@ -88,6 +88,7 @@ class Transformer {
       unstable_disableES6Transforms,
       unstable_transformProfile,
       unstable_memoizeInlineRequires,
+      unstable_nonMemoizedInlineRequires,
       ...extra
     } = transformerOptions;
 
@@ -123,6 +124,7 @@ class Transformer {
       type,
       unstable_disableES6Transforms,
       unstable_memoizeInlineRequires,
+      unstable_nonMemoizedInlineRequires,
       unstable_transformProfile,
     ]);
 

diff --git a/packages/metro/src/lib/transformHelpers.js b/packages/metro/src/lib/transformHelpers.js
@@ -118,6 +118,8 @@ async function calcTransformerOptions(
       transform?.unstable_disableES6Transforms || false,
     unstable_memoizeInlineRequires:
       transform?.unstable_memoizeInlineRequires || false,
+    unstable_nonMemoizedInlineRequires:
+      transform?.unstable_nonMemoizedInlineRequires || [],
     nonInlinedRequires:
       transform?.nonInlinedRequires || baseIgnoredInlineRequires,
     type: 'module',