fix(worker): prevent inject esm in classic workers (#14918)

Author: bluwy

.prettierignore

@@ -8,5 +8,4 @@ pnpm-workspace.yaml
 playground/tsconfig-json-load-error/has-error/tsconfig.json
 playground/html/invalid.html
 playground/html/valid.html
-playground/worker/classic-worker.js
 playground/external/public/[email protected]

packages/vite/src/client/client.ts

@@ -642,7 +642,7 @@ export function injectQuery(url: string, queryToInject: string): string {
   }
 
   // can't use pathname from URL since it may be relative like ../
-  const pathname = url.replace(/#.*$/, '').replace(/\?.*$/, '')
+  const pathname = url.replace(/[?#].*$/s, '')
   const { search, hash } = new URL(url, 'http://vitejs.dev')
 
   return `${pathname}?${queryToInject}${search ? `&` + search.slice(1) : ''}${

packages/vite/src/node/plugins/importAnalysis.ts

@@ -59,6 +59,7 @@ import { throwOutdatedRequest } from './optimizedDeps'
 import { isCSSRequest, isDirectCSSRequest } from './css'
 import { browserExternalId } from './resolve'
 import { serializeDefine } from './define'
+import { WORKER_FILE_ID } from './worker'
 
 const debug = createDebugger('vite:import-analysis')
 
@@ -685,12 +686,17 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
       const acceptedUrls = mergeAcceptedUrls(orderedAcceptedUrls)
       const acceptedExports = mergeAcceptedUrls(orderedAcceptedExports)
 
-      if (hasEnv) {
+      // While we always expect to work with ESM, a classic worker is the only
+      // case where it's not ESM and we need to avoid injecting ESM-specific code
+      const isClassicWorker =
+        importer.includes(WORKER_FILE_ID) && importer.includes('type=classic')
+
+      if (hasEnv && !isClassicWorker) {
         // inject import.meta.env
         str().prepend(getEnv(ssr))
       }
 
-      if (hasHMR && !ssr) {
+      if (hasHMR && !ssr && !isClassicWorker) {
         debugHmr?.(
           `${
             isSelfAccepting
@@ -712,9 +718,13 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
       }
 
       if (needQueryInjectHelper) {
-        str().prepend(
-          `import { injectQuery as __vite__injectQuery } from "${clientPublicPath}";`,
-        )
+        if (isClassicWorker) {
+          str().append('\n' + __vite__injectQuery.toString())
+        } else {
+          str().prepend(
+            `import { injectQuery as __vite__injectQuery } from "${clientPublicPath}";`,
+          )
+        }
       }
 
       // normalize and rewrite accepted urls
@@ -1009,3 +1019,19 @@ export function transformCjsImport(
     return lines.join('; ')
   }
 }
+
+// Copied from `client/client.ts`. Only needed so we can inline inject this function for classic workers.
+function __vite__injectQuery(url: string, queryToInject: string): string {
+  // skip urls that won't be handled by vite
+  if (url[0] !== '.' && url[0] !== '/') {
+    return url
+  }
+
+  // can't use pathname from URL since it may be relative like ../
+  const pathname = url.replace(/[?#].*$/s, '')
+  const { search, hash } = new URL(url, 'http://vitejs.dev')
+
+  return `${pathname}?${queryToInject}${search ? `&` + search.slice(1) : ''}${
+    hash || ''
+  }`
+}

playground/worker/__tests__/es/es-worker.spec.ts

@@ -173,6 +173,10 @@ test('classic worker', async () => {
     'A classic',
     true,
   )
+  await untilUpdated(
+    () => page.textContent('.classic-worker-import'),
+    '[success] classic-esm',
+  )
   await untilUpdated(
     () => page.textContent('.classic-shared-worker'),
     'A classic',

playground/worker/__tests__/iife/iife-worker.spec.ts

@@ -136,6 +136,10 @@ test('module worker', async () => {
 
 test('classic worker', async () => {
   await untilUpdated(() => page.textContent('.classic-worker'), 'A classic')
+  await untilUpdated(
+    () => page.textContent('.classic-worker-import'),
+    '[success] classic-esm',
+  )
   await untilUpdated(
     () => page.textContent('.classic-shared-worker'),
     'A classic',

playground/worker/__tests__/relative-base/relative-base-worker.spec.ts

@@ -122,6 +122,10 @@ test.runIf(isBuild)('classic worker', async () => {
     'A classic',
     true,
   )
+  await untilUpdated(
+    () => page.textContent('.classic-worker-import'),
+    '[success] classic-esm',
+  )
   await untilUpdated(
     () => page.textContent('.classic-shared-worker'),
     'A classic',

playground/worker/classic-esm.js

@@ -0,0 +1 @@
+export const msg = '[success] classic-esm'

playground/worker/classic-worker.js

@@ -3,9 +3,29 @@ if (base.endsWith('.js') || base === `/worker-entries`) base = '' // for dev
 
 importScripts(`${base}/classic.js`)
 
-self.addEventListener('message', () => {
-  self.postMessage(self.constant)
+self.addEventListener('message', async (e) => {
+  switch (e.data) {
+    case 'ping': {
+      self.postMessage({
+        message: e.data,
+        result: self.constant,
+      })
+      break
+    }
+    case 'test-import': {
+      // Vite may inject imports to handle this dynamic import, make sure
+      // it still works in classic workers.
+      // NOTE: this test only works in dev.
+      const importPath = `${base}/classic-esm.js`
+      const { msg } = await import(/* @vite-ignore */ importPath)
+      self.postMessage({
+        message: e.data,
+        result: msg,
+      })
+      break
+    }
+  }
 })
 
 // for sourcemap
-console.log("classic-worker.js")
+console.log('classic-worker.js')

playground/worker/index.html

@@ -125,6 +125,7 @@ <h2 class="format-iife">format iife:</h2>
   <span class="classname">.classic-worker</span>
 </p>
 <code class="classic-worker"></code>
+<code class="classic-worker-import"></code>
 
 <p>
   new SharedWorker(new URL('./classic-shared-worker.js', import.meta.url), {

playground/worker/worker/main-classic.js

@@ -12,9 +12,19 @@ let classicWorker = new Worker(
 classicWorker = new Worker(new URL('../classic-worker.js', import.meta.url))
 
 classicWorker.addEventListener('message', ({ data }) => {
-  text('.classic-worker', JSON.stringify(data))
+  switch (data.message) {
+    case 'ping': {
+      text('.classic-worker', data.result)
+      break
+    }
+    case 'test-import': {
+      text('.classic-worker-import', data.result)
+      break
+    }
+  }
 })
 classicWorker.postMessage('ping')
+classicWorker.postMessage('test-import')
 
 // prettier-ignore
 // test trailing comma