feat: add unstable_renameRequire to allow disabling the require ren…

…ame transform (#1230)

Summary:
Renaming `require` to `_$$_REQUIRE` is an extraneous babel traversal that apparently is used internally at Meta for additional dependency extraction. Since the modules are scoped as iife functions, and there's no special convention to preserve native `require` syntax, this transform does not appear to add any value to standard projects. Per advice from robhogan, I've added a flag to disable this transform, which we can enable by default in Expo CLI.

Saves ~165ms when transforming the production react renderer module.

Pull Request resolved: #1230

Reviewed By: huntie

Differential Revision: D54559949

Pulled By: robhogan

fbshipit-source-id: 8155c5fef8e8febd5ae65a4434be28a98b76a312

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_renameRequire": true,
     "unstable_workerThreads": false,
     "workerPath": "metro/src/DeltaBundler/Worker",
   },
@@ -333,6 +334,7 @@ Object {
     "unstable_dependencyMapReservedName": null,
     "unstable_disableModuleWrapping": false,
     "unstable_disableNormalizePseudoGlobals": false,
+    "unstable_renameRequire": true,
     "unstable_workerThreads": false,
     "workerPath": "metro/src/DeltaBundler/Worker",
   },
@@ -514,6 +516,7 @@ Object {
     "unstable_dependencyMapReservedName": null,
     "unstable_disableModuleWrapping": false,
     "unstable_disableNormalizePseudoGlobals": false,
+    "unstable_renameRequire": true,
     "unstable_workerThreads": false,
     "workerPath": "metro/src/DeltaBundler/Worker",
   },
@@ -695,6 +698,7 @@ Object {
     "unstable_dependencyMapReservedName": null,
     "unstable_disableModuleWrapping": false,
     "unstable_disableNormalizePseudoGlobals": false,
+    "unstable_renameRequire": true,
     "unstable_workerThreads": false,
     "workerPath": "metro/src/DeltaBundler/Worker",
   },

packages/metro-config/src/defaults/index.js

@@ -135,6 +135,7 @@ const getDefaultValues = (projectRoot: ?string): ConfigT => ({
     unstable_dependencyMapReservedName: null,
     unstable_disableModuleWrapping: false,
     unstable_disableNormalizePseudoGlobals: false,
+    unstable_renameRequire: true,
     unstable_compactOutput: false,
     unstable_workerThreads: false,
   },

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,
+  /** Whether to rename scoped `require` functions to `_$$_REQUIRE`, usually an extraneous operation when serializing to iife (default). */
+  unstable_renameRequire?: boolean,
 }>;
 
 export type {CustomTransformOptions} from 'metro-babel-transformer';
@@ -387,6 +389,10 @@ async function transformJS(
         importAll,
         dependencyMapName,
         config.globalPrefix,
+        // TODO: This config is optional to allow its introduction in a minor
+        // release. It should be made non-optional in ConfigT or removed in
+        // future.
+        config.unstable_renameRequire === false,
       ));
     }
   }

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;
+  /** Whether to rename scoped `require` functions to `_$$_REQUIRE`, usually an extraneous operation when serializing to iife (default). */
+  unstable_renameRequire?: boolean;
 }>;
 
 export {CustomTransformOptions} from 'metro-babel-transformer';

packages/metro/src/ModuleGraph/worker/JsFileWrapping.js

@@ -32,6 +32,7 @@ function wrapModule(
   importAllName: string,
   dependencyMapName: string,
   globalPrefix: string,
+  skipRequireRename: boolean,
 ): {
   ast: BabelNodeFile,
   requireName: string,
@@ -45,7 +46,9 @@ function wrapModule(
   const def = t.callExpression(t.identifier(`${globalPrefix}__d`), [factory]);
   const ast = t.file(t.program([t.expressionStatement(def)]));
 
-  const requireName = renameRequires(ast);
+  // `require` doesn't need to be scoped when Metro serializes to iife because the local function
+  // `require` will be used instead of the global one.
+  const requireName = skipRequireRename ? 'require' : renameRequires(ast);
 
   return {ast, requireName};
 }

packages/metro/src/ModuleGraph/worker/__tests__/JsFileWrapping-test.js

@@ -41,6 +41,7 @@ it('wraps a module correctly', () => {
     '_$$_IMPORT_ALL',
     dependencyMapName,
     defaultGlobalPrefix,
+    false,
   );
 
   expect(requireName).toBe(BABEL_RENAMED);
@@ -58,6 +59,42 @@ it('wraps a module correctly', () => {
   );
 });
 
+it('wraps a module without renaming require statements', () => {
+  const dependencyMapName = '_dependencyMapName';
+  const skipRequireRename = true;
+  const originalAst = astFromCode(`
+    const dynamicRequire = require;
+    const a = require('b/lib/a');
+    exports.do = () => require("do");
+    if (!something) {
+      require("setup/something");
+    }
+    require.blah('do');
+  `);
+  const {ast, requireName} = JsFileWrapping.wrapModule(
+    originalAst,
+    '_$$_IMPORT_DEFAULT',
+    '_$$_IMPORT_ALL',
+    dependencyMapName,
+    defaultGlobalPrefix,
+    skipRequireRename,
+  );
+
+  expect(requireName).toBe('require');
+  expect(codeFromAst(ast)).toEqual(
+    comparableCode(`
+      __d(function (global, require, _$$_IMPORT_DEFAULT, _$$_IMPORT_ALL, module, exports, _dependencyMapName) {
+        const dynamicRequire = require;
+        const a = require('b/lib/a');
+        exports.do = () => require("do");
+        if (!something) {
+          require("setup/something");
+        }
+        require.blah('do');
+      });`),
+  );
+});
+
 it('wraps a module correctly with global prefix', () => {
   const dependencyMapName = '_dependencyMapName';
 
@@ -71,6 +108,7 @@ it('wraps a module correctly with global prefix', () => {
     '_$$_IMPORT_ALL',
     dependencyMapName,
     globalPrefix,
+    false,
   );
 
   expect(requireName).toBe(BABEL_RENAMED);
@@ -104,6 +142,7 @@ describe('safe renaming of require', () => {
           '_$$_IMPORT_ALL',
           dependencyMapName,
           defaultGlobalPrefix,
+          false,
         );
 
         expect(requireName).toBe(BABEL_RENAMED);
@@ -141,6 +180,7 @@ describe('safe renaming of require', () => {
           '_$$_IMPORT_ALL',
           dependencyMapName,
           defaultGlobalPrefix,
+          false,
         );
 
         expect(requireName).toBe(BABEL_RENAMED2);
@@ -181,6 +221,7 @@ describe('safe renaming of require', () => {
           '_$$_IMPORT_ALL',
           dependencyMapName,
           defaultGlobalPrefix,
+          false,
         );
 
         expect(requireName).toBe(BABEL_RENAMED2);