Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(worker): support dynamic worker option fields #19010

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
155 changes: 155 additions & 0 deletions packages/vite/src/node/__tests__/plugins/workerImportMetaUrl.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
import path from 'node:path'
import { describe, expect, test } from 'vitest'
import { parseAst } from 'rollup/parseAst'
import { workerImportMetaUrlPlugin } from '../../plugins/workerImportMetaUrl'
import { resolveConfig } from '../../config'
import { PartialEnvironment } from '../../baseEnvironment'

async function createWorkerImportMetaUrlPluginTransform() {
const root = path.join(import.meta.dirname, 'fixtures/worker')
const config = await resolveConfig({ configFile: false, root }, 'serve')
const instance = workerImportMetaUrlPlugin(config)
const environment = new PartialEnvironment('client', config)

return async (code: string) => {
// @ts-expect-error transform should exist
const result = await instance.transform.call(
{ environment, parse: parseAst },
code,
path.join(root, 'foo.ts'),
)
return result?.code || result
}
}

describe('workerImportMetaUrlPlugin', async () => {
const transform = await createWorkerImportMetaUrlPluginTransform()

test('without worker options', async () => {
expect(
await transform('new Worker(new URL("./worker.js", import.meta.url))'),
).toMatchInlineSnapshot(

Check failure on line 31 in packages/vite/src/node/__tests__/plugins/workerImportMetaUrl.spec.ts

View workflow job for this annotation

GitHub Actions / Build&Test: node-22, windows-latest

packages/vite/src/node/__tests__/plugins/workerImportMetaUrl.spec.ts > workerImportMetaUrlPlugin > without worker options

Error: Snapshot `workerImportMetaUrlPlugin > without worker options 1` mismatched Expected: ""new Worker(new URL(/* @vite-ignore */ "/worker.js?worker_file&type=classic", import.meta.url))"" Received: ""new Worker(new URL(/* @vite-ignore */ "/@fs/D:/a/vite/vite/packages/vite/src/node/__tests__/plugins/fixtures/worker/worker.js?worker_file&type=classic", import.meta.url))"" ❯ packages/vite/src/node/__tests__/plugins/workerImportMetaUrl.spec.ts:31:7
`"new Worker(new URL(/* @vite-ignore */ "/worker.js?worker_file&type=classic", import.meta.url))"`,
hi-ogawa marked this conversation as resolved.
Show resolved Hide resolved
)
})

test('with shared worker', async () => {
expect(
await transform(
'new SharedWorker(new URL("./worker.js", import.meta.url))',
),
).toMatchInlineSnapshot(

Check failure on line 41 in packages/vite/src/node/__tests__/plugins/workerImportMetaUrl.spec.ts

View workflow job for this annotation

GitHub Actions / Build&Test: node-22, windows-latest

packages/vite/src/node/__tests__/plugins/workerImportMetaUrl.spec.ts > workerImportMetaUrlPlugin > with shared worker

Error: Snapshot `workerImportMetaUrlPlugin > with shared worker 1` mismatched Expected: ""new SharedWorker(new URL(/* @vite-ignore */ "/worker.js?worker_file&type=classic", import.meta.url))"" Received: ""new SharedWorker(new URL(/* @vite-ignore */ "/@fs/D:/a/vite/vite/packages/vite/src/node/__tests__/plugins/fixtures/worker/worker.js?worker_file&type=classic", import.meta.url))"" ❯ packages/vite/src/node/__tests__/plugins/workerImportMetaUrl.spec.ts:41:7
`"new SharedWorker(new URL(/* @vite-ignore */ "/worker.js?worker_file&type=classic", import.meta.url))"`,
)
})

test('with static worker options', async () => {
expect(
await transform(
'new Worker(new URL("./worker.js", import.meta.url), { type: "module", name: "worker1" })',
),
).toMatchInlineSnapshot(

Check failure on line 51 in packages/vite/src/node/__tests__/plugins/workerImportMetaUrl.spec.ts

View workflow job for this annotation

GitHub Actions / Build&Test: node-22, windows-latest

packages/vite/src/node/__tests__/plugins/workerImportMetaUrl.spec.ts > workerImportMetaUrlPlugin > with static worker options

Error: Snapshot `workerImportMetaUrlPlugin > with static worker options 1` mismatched Expected: ""new Worker(new URL(/* @vite-ignore */ "/worker.js?worker_file&type=module", import.meta.url), { type: "module", name: "worker1" })"" Received: ""new Worker(new URL(/* @vite-ignore */ "/@fs/D:/a/vite/vite/packages/vite/src/node/__tests__/plugins/fixtures/worker/worker.js?worker_file&type=module", import.meta.url), { type: "module", name: "worker1" })"" ❯ packages/vite/src/node/__tests__/plugins/workerImportMetaUrl.spec.ts:51:7
`"new Worker(new URL(/* @vite-ignore */ "/worker.js?worker_file&type=module", import.meta.url), { type: "module", name: "worker1" })"`,
)
})

test('with dynamic name field in worker options', async () => {
expect(
await transform(
'const id = 1; new Worker(new URL("./worker.js", import.meta.url), { name: "worker" + id })',
),
).toMatchInlineSnapshot(

Check failure on line 61 in packages/vite/src/node/__tests__/plugins/workerImportMetaUrl.spec.ts

View workflow job for this annotation

GitHub Actions / Build&Test: node-22, windows-latest

packages/vite/src/node/__tests__/plugins/workerImportMetaUrl.spec.ts > workerImportMetaUrlPlugin > with dynamic name field in worker options

Error: Snapshot `workerImportMetaUrlPlugin > with dynamic name field in worker options 1` mismatched Expected: ""const id = 1; new Worker(new URL(/* @vite-ignore */ "/worker.js?worker_file&type=classic", import.meta.url), { name: "worker" + id })"" Received: ""const id = 1; new Worker(new URL(/* @vite-ignore */ "/@fs/D:/a/vite/vite/packages/vite/src/node/__tests__/plugins/fixtures/worker/worker.js?worker_file&type=classic", import.meta.url), { name: "worker" + id })"" ❯ packages/vite/src/node/__tests__/plugins/workerImportMetaUrl.spec.ts:61:7
`"const id = 1; new Worker(new URL(/* @vite-ignore */ "/worker.js?worker_file&type=classic", import.meta.url), { name: "worker" + id })"`,
)
})

test('with dynamic name field and static type in worker options', async () => {
expect(
await transform(
'const id = 1; new Worker(new URL("./worker.js", import.meta.url), { name: "worker" + id, type: "module" })',
),
).toMatchInlineSnapshot(

Check failure on line 71 in packages/vite/src/node/__tests__/plugins/workerImportMetaUrl.spec.ts

View workflow job for this annotation

GitHub Actions / Build&Test: node-22, windows-latest

packages/vite/src/node/__tests__/plugins/workerImportMetaUrl.spec.ts > workerImportMetaUrlPlugin > with dynamic name field and static type in worker options

Error: Snapshot `workerImportMetaUrlPlugin > with dynamic name field and static type in worker options 1` mismatched Expected: ""const id = 1; new Worker(new URL(/* @vite-ignore */ "/worker.js?worker_file&type=module", import.meta.url), { name: "worker" + id, type: "module" })"" Received: ""const id = 1; new Worker(new URL(/* @vite-ignore */ "/@fs/D:/a/vite/vite/packages/vite/src/node/__tests__/plugins/fixtures/worker/worker.js?worker_file&type=module", import.meta.url), { name: "worker" + id, type: "module" })"" ❯ packages/vite/src/node/__tests__/plugins/workerImportMetaUrl.spec.ts:71:7
`"const id = 1; new Worker(new URL(/* @vite-ignore */ "/worker.js?worker_file&type=module", import.meta.url), { name: "worker" + id, type: "module" })"`,
)
})

test('with parenthesis inside of worker options', async () => {
expect(
await transform(
'const worker = new Worker(new URL("./worker.js", import.meta.url), { name: genName(), type: "module"})',
),
).toMatchInlineSnapshot(

Check failure on line 81 in packages/vite/src/node/__tests__/plugins/workerImportMetaUrl.spec.ts

View workflow job for this annotation

GitHub Actions / Build&Test: node-22, windows-latest

packages/vite/src/node/__tests__/plugins/workerImportMetaUrl.spec.ts > workerImportMetaUrlPlugin > with parenthesis inside of worker options

Error: Snapshot `workerImportMetaUrlPlugin > with parenthesis inside of worker options 1` mismatched Expected: ""const worker = new Worker(new URL(/* @vite-ignore */ "/worker.js?worker_file&type=module", import.meta.url), { name: genName(), type: "module"})"" Received: ""const worker = new Worker(new URL(/* @vite-ignore */ "/@fs/D:/a/vite/vite/packages/vite/src/node/__tests__/plugins/fixtures/worker/worker.js?worker_file&type=module", import.meta.url), { name: genName(), type: "module"})"" ❯ packages/vite/src/node/__tests__/plugins/workerImportMetaUrl.spec.ts:81:7
`"const worker = new Worker(new URL(/* @vite-ignore */ "/worker.js?worker_file&type=module", import.meta.url), { name: genName(), type: "module"})"`,
)
})

test('with multi-line code and worker options', async () => {
expect(
await transform(`
const worker = new Worker(new URL("./worker.js", import.meta.url), {
name: genName(),
type: "module",
},
)

worker.addEventListener('message', (ev) => text('.simple-worker-url', JSON.stringify(ev.data)))
`),
).toMatchInlineSnapshot(`"

Check failure on line 97 in packages/vite/src/node/__tests__/plugins/workerImportMetaUrl.spec.ts

View workflow job for this annotation

GitHub Actions / Build&Test: node-22, windows-latest

packages/vite/src/node/__tests__/plugins/workerImportMetaUrl.spec.ts > workerImportMetaUrlPlugin > with multi-line code and worker options

Error: Snapshot `workerImportMetaUrlPlugin > with multi-line code and worker options 1` mismatched - Expected + Received @@ -1,7 +1,7 @@ " - const worker = new Worker(new URL(/* @vite-ignore */ "/worker.js?worker_file&type=module", import.meta.url), { + const worker = new Worker(new URL(/* @vite-ignore */ "/@fs/D:/a/vite/vite/packages/vite/src/node/__tests__/plugins/fixtures/worker/worker.js?worker_file&type=module", import.meta.url), { name: genName(), type: "module", }, ) ❯ packages/vite/src/node/__tests__/plugins/workerImportMetaUrl.spec.ts:97:7
const worker = new Worker(new URL(/* @vite-ignore */ "/worker.js?worker_file&type=module", import.meta.url), {
name: genName(),
type: "module",
},
)

worker.addEventListener('message', (ev) => text('.simple-worker-url', JSON.stringify(ev.data)))
"`)
})

test('throws an error when non-static worker options are provided', async () => {
await expect(
transform(
'new Worker(new URL("./worker.js", import.meta.url), myWorkerOptions)',
),
).rejects.toThrow(
'Vite is unable to parse the worker options as the value is not static. To ignore this error, please use /* @vite-ignore */ in the worker options.',
)
})

test('throws an error when worker options are not an object', async () => {
await expect(
transform(
'new Worker(new URL("./worker.js", import.meta.url), "notAnObject")',
),
).rejects.toThrow('Expected worker options to be an object, got string')
})

test('throws an error when non-literal type field in worker options', async () => {
await expect(
transform(
'const type = "module"; new Worker(new URL("./worker.js", import.meta.url), { type })',
),
).rejects.toThrow(
'Expected worker options type property to be a literal value.',
)
})

test('throws an error when spread operator used without the type field', async () => {
await expect(
transform(
'const options = { name: "worker1" }; new Worker(new URL("./worker.js", import.meta.url), { ...options })',
),
).rejects.toThrow(
'Expected object spread to be used before the definition of the type property. Vite needs a static value for the type property to correctly infer it.',
)
})

test('throws an error when spread operator used after definition of type field', async () => {
await expect(
transform(
'const options = { name: "worker1" }; new Worker(new URL("./worker.js", import.meta.url), { type: "module", ...options })',
),
).rejects.toThrow(
'Expected object spread to be used before the definition of the type property. Vite needs a static value for the type property to correctly infer it.',
)
})
})
89 changes: 82 additions & 7 deletions packages/vite/src/node/plugins/workerImportMetaUrl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,87 @@ function err(e: string, pos: number) {
return error
}

function parseWorkerOptions(
function findClosingParen(input: string, fromIndex: number) {
let count = 1

for (let i = fromIndex + 1; i < input.length; i++) {
if (input[i] === '(') count++
if (input[i] === ')') count--
if (count === 0) return i
}

return -1
}

function extractWorkerTypeFromAst(
astNode: any,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
astNode: any,
astNode: Expression,

optsStartIndex: number,
): 'classic' | 'module' | undefined {
if (astNode.type !== 'ObjectExpression') {
return
}

let lastSpreadElementIndex = -1
let typeProperty = null
let typePropertyIndex = -1

for (let i = 0; i < astNode.properties.length; i++) {
const property = astNode.properties[i]

if (property.type === 'SpreadElement') {
lastSpreadElementIndex = i
continue
}

if (property.type === 'Property' && property.key?.name === 'type') {
Copy link
Member

@sapphi-red sapphi-red Dec 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (property.type === 'Property' && property.key?.name === 'type') {
if (
property.type === 'Property' &&
((property.key.type === 'Identifier' && property.key.name === 'type') ||
(property.key.type === 'Literal' && property.key.value === 'type'))
) {

I think we can add support for { "type": "module" }. It would be nice to have a test for this one.

typeProperty = property
typePropertyIndex = i
}
}

if (typePropertyIndex === -1 && lastSpreadElementIndex === -1) {
// No type property or spread element in use. Assume safe usage and default to classic
return 'classic'
}

if (typePropertyIndex < lastSpreadElementIndex) {
throw err(
'Expected object spread to be used before the definition of the type property. ' +
'Vite needs a static value for the type property to correctly infer it.',
optsStartIndex,
)
}

if (typeProperty?.value?.type !== 'Literal') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (typeProperty?.value?.type !== 'Literal') {
if (typeProperty?.value.type !== 'Literal') {

I guess this ? is not necessary.

throw err(
'Expected worker options type property to be a literal value.',
optsStartIndex,
)
}

// Silently default to classic type like the getWorkerType method
return typeProperty.value.value === 'module' ? 'module' : 'classic'
}

async function parseWorkerOptions(
rawOpts: string,
optsStartIndex: number,
): WorkerOptions {
): Promise<WorkerOptions> {
let opts: WorkerOptions = {}
try {
opts = evalValue<WorkerOptions>(rawOpts)
} catch {
const { parseAstAsync } = await import('rollup/parseAst')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We statically import rollup/parseAst in other places so let's do it here as well.

const optsNode = ((await parseAstAsync(`(${rawOpts})`)).body[0] as any)
.expression
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const optsNode = ((await parseAstAsync(`(${rawOpts})`)).body[0] as any)
.expression
const optsNode = (
(await parseAstAsync(`(${rawOpts})`))
.body[0] as RollupAstNode<ExpressionStatement>
).expression

Let's add some types to make it safer.


const type = extractWorkerTypeFromAst(optsNode, optsStartIndex)
if (type) {
return { type }
}

throw err(
'Vite is unable to parse the worker options as the value is not static.' +
'Vite is unable to parse the worker options as the value is not static. ' +
'To ignore this error, please use /* @vite-ignore */ in the worker options.',
optsStartIndex,
)
Expand All @@ -54,12 +125,16 @@ function parseWorkerOptions(
return opts
}

function getWorkerType(raw: string, clean: string, i: number): WorkerType {
async function getWorkerType(
raw: string,
clean: string,
i: number,
): Promise<WorkerType> {
const commaIndex = clean.indexOf(',', i)
if (commaIndex === -1) {
return 'classic'
}
const endIndex = clean.indexOf(')', i)
const endIndex = findClosingParen(clean, i)

// case: ') ... ,' mean no worker options params
if (commaIndex > endIndex) {
Expand All @@ -82,7 +157,7 @@ function getWorkerType(raw: string, clean: string, i: number): WorkerType {
return 'classic'
}

const workerOpts = parseWorkerOptions(workerOptString, commaIndex + 1)
const workerOpts = await parseWorkerOptions(workerOptString, commaIndex + 1)
if (
workerOpts.type &&
(workerOpts.type === 'module' || workerOpts.type === 'classic')
Expand Down Expand Up @@ -152,7 +227,7 @@ export function workerImportMetaUrlPlugin(config: ResolvedConfig): Plugin {
}

s ||= new MagicString(code)
const workerType = getWorkerType(code, cleanString, endIndex)
const workerType = await getWorkerType(code, cleanString, endIndex)
const url = rawUrl.slice(1, -1)
let file: string | undefined
if (url[0] === '.') {
Expand Down
1 change: 0 additions & 1 deletion playground/worker/worker/main-module.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,6 @@ const genWorkerName = () => 'module'
const w2 = new SharedWorker(
new URL('../url-shared-worker.js', import.meta.url),
{
/* @vite-ignore */
jamsinclair marked this conversation as resolved.
Show resolved Hide resolved
name: genWorkerName(),
type: 'module',
},
Expand Down
Loading