Skip to content

Commit

Permalink
fix(worker): prevent inject esm in classic workers (#14918)
Browse files Browse the repository at this point in the history
  • Loading branch information
bluwy committed Nov 9, 2023
1 parent e49ef02 commit 2687dbb
Show file tree
Hide file tree
Showing 10 changed files with 80 additions and 11 deletions.
1 change: 0 additions & 1 deletion .prettierignore
Original file line number Diff line number Diff line change
Expand Up @@ -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/slash@3.0.0.js
2 changes: 1 addition & 1 deletion packages/vite/src/client/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, '')

This comment has been minimized.

Copy link
@MajedMuhammad

MajedMuhammad Jan 31, 2024

Contributor

regex s modifier is problematic for older browsers!

const { search, hash } = new URL(url, 'http://vitejs.dev')

return `${pathname}?${queryToInject}${search ? `&` + search.slice(1) : ''}${
Expand Down
36 changes: 31 additions & 5 deletions packages/vite/src/node/plugins/importAnalysis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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 || ''
}`
}
4 changes: 4 additions & 0 deletions playground/worker/__tests__/es/es-worker.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
4 changes: 4 additions & 0 deletions playground/worker/__tests__/iife/iife-worker.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
1 change: 1 addition & 0 deletions playground/worker/classic-esm.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const msg = '[success] classic-esm'
26 changes: 23 additions & 3 deletions playground/worker/classic-worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
1 change: 1 addition & 0 deletions playground/worker/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -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), {
Expand Down
12 changes: 11 additions & 1 deletion playground/worker/worker/main-classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 2687dbb

Please sign in to comment.