fix: use string manipulation instead of regex to inject esbuild helpe…

…rs (#14094)
vitejs · Oct 3, 2023 · 128ad8f · 128ad8f
1 parent b1b816a
commit 128ad8f
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 17 deletions.
diff --git a/packages/vite/src/node/plugins/esbuild.ts b/packages/vite/src/node/plugins/esbuild.ts
@@ -28,10 +28,9 @@ import { searchForWorkspaceRoot } from '../server/searchRoot'
 
 const debug = createDebugger('vite:esbuild')
 
-const INJECT_HELPERS_IIFE_RE =
-  /^(.*?)((?:const|var)\s+\S+\s*=\s*function\s*\([^)]*\)\s*\{\s*"use strict";)/s
-const INJECT_HELPERS_UMD_RE =
-  /^(.*?)(\(function\([^)]*\)\s*\{.+?amd.+?function\([^)]*\)\s*\{\s*"use strict";)/s
+// IIFE content looks like `var MyLib = function() {`. Spaces are removed when minified
+const IIFE_BEGIN_RE =
+  /(const|var)\s+\S+\s*=\s*function\(\)\s*\{.*"use strict";/s
 
 const validExtensionRE = /\.\w+$/
 const jsxExtensionsRE = /\.(?:j|t)sx\b/
@@ -333,22 +332,30 @@ export const buildEsbuildPlugin = (config: ResolvedConfig): Plugin => {
       if (config.build.lib) {
         // #7188, esbuild adds helpers out of the UMD and IIFE wrappers, and the
         // names are minified potentially causing collision with other globals.
-        // We use a regex to inject the helpers inside the wrappers.
+        // We inject the helpers inside the wrappers.
+        // e.g. turn:
+        //    <esbuild helpers> (function(){ /*actual content/* })()
+        // into:
+        //    (function(){ <esbuild helpers> /*actual content/* })()
+        // Not using regex because it's too hard to rule out performance issues like #8738 #8099 #10900 #14065
+        // Instead, using plain string index manipulation (indexOf, slice) which is simple and performant
         // We don't need to create a MagicString here because both the helpers and
         // the headers don't modify the sourcemap
-        const injectHelpers =
-          opts.format === 'umd'
-            ? INJECT_HELPERS_UMD_RE
-            : opts.format === 'iife'
-            ? INJECT_HELPERS_IIFE_RE
-            : undefined
-        if (injectHelpers) {
-          res.code = res.code.replace(
-            injectHelpers,
-            (_, helpers, header) => header + helpers,
-          )
+        const esbuildCode = res.code
+        const contentIndex =
+          opts.format === 'iife'
+            ? esbuildCode.match(IIFE_BEGIN_RE)?.index || 0
+            : opts.format === 'umd'
+            ? esbuildCode.indexOf(`(function(`) // same for minified or not
+            : 0
+        if (contentIndex > 0) {
+          const esbuildHelpers = esbuildCode.slice(0, contentIndex)
+          res.code = esbuildCode
+            .slice(contentIndex)
+            .replace(`"use strict";`, `"use strict";` + esbuildHelpers)
         }
       }
+
       return res
     },
   }

diff --git a/playground/lib/__tests__/lib.spec.ts b/playground/lib/__tests__/lib.spec.ts
@@ -33,7 +33,9 @@ describe.runIf(isBuild)('build', () => {
       'dist/nominify/my-lib-custom-filename.iife.js',
     )
     // esbuild helpers are injected inside of the IIFE wrapper
-    expect(code).toMatch(/^var MyLib=function\(\)\{"use strict";/)
+    // esbuild has a bug that injects some statements before `"use strict"`: https://github.com/evanw/esbuild/issues/3322
+    // remove the `.*?` part once it's fixed
+    expect(code).toMatch(/^var MyLib=function\(\)\{.*?"use strict";/)
     expect(noMinifyCode).toMatch(
       /^var MyLib\s*=\s*function\(\)\s*\{.*?"use strict";/s,
     )

diff --git a/playground/lib/src/main.js b/playground/lib/src/main.js
@@ -10,3 +10,6 @@ export default function myLib(sel) {
   // make sure umd helper has been moved to the right position
   console.log(`amd function(){ "use strict"; }`)
 }
+
+// For triggering unhandled global esbuild helpers in previous regex-based implementation for injection
+;(function () {})()?.foo
diff --git a/playground/lib/vite.config.js b/playground/lib/vite.config.js
@@ -7,6 +7,7 @@ export default defineConfig({
     supported: {
       // Force esbuild inject helpers to test regex
       'object-rest-spread': false,
+      'optional-chain': false,
     },
   },
   build: {