Improve performance of InlineRequires visitor

parcel-bundler · Apr 22, 2024 · 9f96b2f · 9f96b2f
1 parent a1f0ba6
commit 9f96b2f
Showing 1 changed file with 11 additions and 23 deletions.
diff --git a/packages/optimizers/inline-requires/src/RequireInliningVisitor.js b/packages/optimizers/inline-requires/src/RequireInliningVisitor.js
@@ -48,19 +48,14 @@ export class RequireInliningVisitor extends Visitor {
     // and also reset the module variable tracking data structures.
     //
     // (TODO: Support arrow functions if we modify the runtime to output arrow functions)
-    // For ease of comparison, map the arg identifiers to an array of strings.. this will skip any
-    // functions with non-identifier args (e.g. spreads etc..)
-    const args = n.params.map(param => {
-      if (param.pat.type === 'Identifier') {
-        return param.pat.value;
-      }
-      return null;
-    });
-
     if (
-      args[0] === 'require' &&
-      args[1] === 'module' &&
-      args[2] === 'exports'
+      n.params.length === 3 &&
+      n.params[0].pat.type === 'Identifier' &&
+      n.params[0].pat.value === 'require' &&
+      n.params[1].pat.type === 'Identifier' &&
+      n.params[1].pat.value === 'module' &&
+      n.params[2].pat.type === 'Identifier' &&
+      n.params[2].pat.value === 'exports'
     ) {
       // `inModuleDefinition` is either null, or the module definition node
       this.currentModuleNode = n;
@@ -82,7 +77,7 @@ export class RequireInliningVisitor extends Visitor {
     // We're looking for variable declarations that look like this:
     //
     // `var $acw62 = require("acw62");`
-    let unusedDeclIndexes = [];
+    let needsReplacement = false;
     for (let i = 0; i < n.declarations.length; i++) {
       let decl = n.declarations[i];
       const init = decl.init;
@@ -122,9 +117,6 @@ export class RequireInliningVisitor extends Visitor {
             this.publicIdToAssetSideEffects.get(assetPublicId),
           );
           if (asset.sideEffects) {
-            this.logger.verbose({
-              message: `Skipping optimisation of ${assetPublicId} (${asset.filePath}) as it declares sideEffects`,
-            });
             // eslint-disable-next-line no-continue
             continue;
           }
@@ -135,17 +127,13 @@ export class RequireInliningVisitor extends Visitor {
         // The moduleVariables set is just the used set of modules (e.g. `$acw62`)
         this.moduleVariables.add(variable);
 
-        this.logger.verbose({
-          message: `${this.bundle.name}: Found require of ${variable} for replacement`,
-        });
-
         // Replace this with a null declarator, we'll use the `init` where it's declared.
         //
         // This mutates `var $acw62 = require("acw62")` -> `var $acw62 = null`
         //
         // The variable will be unused and removed by optimisation
         decl.init = undefined;
-        unusedDeclIndexes.push(i);
+        needsReplacement = true;
       } else if (
         decl.id.type === 'Identifier' &&
         typeof decl.id.value === 'string' &&
@@ -183,10 +171,10 @@ export class RequireInliningVisitor extends Visitor {
         this.moduleVariables.add(variable);
 
         decl.init = undefined;
-        unusedDeclIndexes.push(i);
+        needsReplacement = true;
       }
     }
-    if (unusedDeclIndexes.length === 0) {
+    if (!needsReplacement) {
       return super.visitVariableDeclaration(n);
     } else {
       this.dirty = true;