fix!: worker.plugins is a function (#14685)

Co-authored-by: jamsinclair <[email protected]>
Co-authored-by: Bjorn Lu <[email protected]>

Author: 3 people

docs/config/worker-options.md

@@ -11,9 +11,10 @@ Output format for worker bundle.
 
 ## worker.plugins
 
-- **Type:** [`(Plugin | Plugin[])[]`](./shared-options#plugins)
+- **Type:** [`() => (Plugin | Plugin[])[]`](./shared-options#plugins)
 
-Vite plugins that apply to worker bundle. Note that [config.plugins](./shared-options#plugins) only applies to workers in dev, it should be configured here instead for build.
+Vite plugins that apply to the worker bundles. Note that [config.plugins](./shared-options#plugins) only applies to workers in dev, it should be configured here instead for build.
+The function should return new plugin instances as they are used in parallel rollup worker builds. As such, modifying `config.worker` options in the `config` hook will be ignored.
 
 ## worker.rollupOptions
 

docs/guide/migration.md

@@ -34,6 +34,10 @@ See the [troubleshooting guide](/guide/troubleshooting.html#vite-cjs-node-api-de
 
 ## General Changes
 
+### `worker.plugins` is now a function
+
+In Vite 4, `worker.plugins` accepted an array of plugins (`(Plugin | Plugin[])[]`). From Vite 5, it needs to be configured as a function that returns an array of plugins (`() => (Plugin | Plugin[])[]`). This change is required so parallel worker builds run more consistently and predictably.
+
 ### Allow path containing `.` to fallback to index.html
 
 In Vite 4, accessing a path containing `.` did not fallback to index.html even if `appType` is set to `'SPA'` (default).

packages/vite/src/node/config.ts

@@ -259,9 +259,11 @@ export interface UserConfig {
      */
     format?: 'es' | 'iife'
     /**
-     * Vite plugins that apply to worker bundle
+     * Vite plugins that apply to worker bundle. The plugins retured by this function
+     * should be new instances every time it is called, because they are used for each
+     * rollup worker bundling process.
      */
-    plugins?: PluginOption[]
+    plugins?: () => PluginOption[]
     /**
      * Rollup options to build worker bundle
      */
@@ -316,9 +318,9 @@ export interface LegacyOptions {
    */
 }
 
-export interface ResolvedWorkerOptions extends PluginHookUtils {
+export interface ResolvedWorkerOptions {
   format: 'es' | 'iife'
-  plugins: Plugin[]
+  plugins: () => Promise<Plugin[]>
   rollupOptions: RollupOptions
 }
 
@@ -440,12 +442,6 @@ export async function resolveConfig(
       return p.apply === command
     }
   }
-  // Some plugins that aren't intended to work in the bundling of workers (doing post-processing at build time for example).
-  // And Plugins may also have cached that could be corrupted by being used in these extra rollup calls.
-  // So we need to separate the worker plugin from the plugin that vite needs to run.
-  const rawWorkerUserPlugins = (
-    (await asyncFlatten(config.worker?.plugins || [])) as Plugin[]
-  ).filter(filterPlugin)
 
   // resolve plugins
   const rawUserPlugins = (
@@ -659,27 +655,74 @@ export async function resolveConfig(
 
   const BASE_URL = resolvedBase
 
-  // resolve worker
-  let workerConfig = mergeConfig({}, config)
-  const [workerPrePlugins, workerNormalPlugins, workerPostPlugins] =
-    sortUserPlugins(rawWorkerUserPlugins)
+  let resolved: ResolvedConfig
+
+  let createUserWorkerPlugins = config.worker?.plugins
+  if (Array.isArray(createUserWorkerPlugins)) {
+    // @ts-expect-error backward compatibility
+    createUserWorkerPlugins = () => config.worker?.plugins
+
+    logger.warn(
+      colors.yellow(
+        `worker.plugins is now a function that returns an array of plugins. ` +
+          `Please update your Vite config accordingly.\n`,
+      ),
+    )
+  }
+
+  const createWorkerPlugins = async function () {
+    // Some plugins that aren't intended to work in the bundling of workers (doing post-processing at build time for example).
+    // And Plugins may also have cached that could be corrupted by being used in these extra rollup calls.
+    // So we need to separate the worker plugin from the plugin that vite needs to run.
+    const rawWorkerUserPlugins = (
+      (await asyncFlatten(createUserWorkerPlugins?.() || [])) as Plugin[]
+    ).filter(filterPlugin)
+
+    // resolve worker
+    let workerConfig = mergeConfig({}, config)
+    const [workerPrePlugins, workerNormalPlugins, workerPostPlugins] =
+      sortUserPlugins(rawWorkerUserPlugins)
+
+    // run config hooks
+    const workerUserPlugins = [
+      ...workerPrePlugins,
+      ...workerNormalPlugins,
+      ...workerPostPlugins,
+    ]
+    workerConfig = await runConfigHook(
+      workerConfig,
+      workerUserPlugins,
+      configEnv,
+    )
+
+    const workerResolved: ResolvedConfig = {
+      ...workerConfig,
+      ...resolved,
+      isWorker: true,
+      mainConfig: resolved,
+    }
+    const resolvedWorkerPlugins = await resolvePlugins(
+      workerResolved,
+      workerPrePlugins,
+      workerNormalPlugins,
+      workerPostPlugins,
+    )
+
+    // run configResolved hooks
+    createPluginHookUtils(resolvedWorkerPlugins)
+      .getSortedPluginHooks('configResolved')
+      .map((hook) => hook(workerResolved))
+
+    return resolvedWorkerPlugins
+  }
 
-  // run config hooks
-  const workerUserPlugins = [
-    ...workerPrePlugins,
-    ...workerNormalPlugins,
-    ...workerPostPlugins,
-  ]
-  workerConfig = await runConfigHook(workerConfig, workerUserPlugins, configEnv)
   const resolvedWorkerOptions: ResolvedWorkerOptions = {
-    format: workerConfig.worker?.format || 'iife',
-    plugins: [],
-    rollupOptions: workerConfig.worker?.rollupOptions || {},
-    getSortedPlugins: undefined!,
-    getSortedPluginHooks: undefined!,
+    format: config.worker?.format || 'iife',
+    plugins: createWorkerPlugins,
+    rollupOptions: config.worker?.rollupOptions || {},
   }
 
-  const resolvedConfig: ResolvedConfig = {
+  resolved = {
     configFile: configFile ? normalizePath(configFile) : undefined,
     configFileDependencies: configFileDependencies.map((name) =>
       normalizePath(path.resolve(name)),
@@ -741,11 +784,10 @@ export async function resolveConfig(
     getSortedPlugins: undefined!,
     getSortedPluginHooks: undefined!,
   }
-  const resolved: ResolvedConfig = {
+  resolved = {
     ...config,
-    ...resolvedConfig,
+    ...resolved,
   }
-
   ;(resolved.plugins as Plugin[]) = await resolvePlugins(
     resolved,
     prePlugins,
@@ -754,32 +796,12 @@ export async function resolveConfig(
   )
   Object.assign(resolved, createPluginHookUtils(resolved.plugins))
 
-  const workerResolved: ResolvedConfig = {
-    ...workerConfig,
-    ...resolvedConfig,
-    isWorker: true,
-    mainConfig: resolved,
-  }
-  resolvedConfig.worker.plugins = await resolvePlugins(
-    workerResolved,
-    workerPrePlugins,
-    workerNormalPlugins,
-    workerPostPlugins,
-  )
-  Object.assign(
-    resolvedConfig.worker,
-    createPluginHookUtils(resolvedConfig.worker.plugins),
-  )
-
   // call configResolved hooks
-  await Promise.all([
-    ...resolved
+  await Promise.all(
+    resolved
       .getSortedPluginHooks('configResolved')
       .map((hook) => hook(resolved)),
-    ...resolvedConfig.worker
-      .getSortedPluginHooks('configResolved')
-      .map((hook) => hook(workerResolved)),
-  ])
+  )
 
   // validate config
 
@@ -804,7 +826,7 @@ export async function resolveConfig(
     plugins: resolved.plugins.map((p) => p.name),
     worker: {
       ...resolved.worker,
-      plugins: resolved.worker.plugins.map((p) => p.name),
+      plugins: `() => plugins`,
     },
   })
 

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

@@ -41,30 +41,7 @@ function saveEmitWorkerAsset(
   workerMap.assets.set(fileName, asset)
 }
 
-// Ensure that only one rollup build is called at the same time to avoid
-// leaking state in plugins between worker builds.
-// TODO: Review if we can parallelize the bundling of workers.
-const workerConfigSemaphore = new WeakMap<
-  ResolvedConfig,
-  Promise<OutputChunk>
->()
-export async function bundleWorkerEntry(
-  config: ResolvedConfig,
-  id: string,
-  query: Record<string, string> | null,
-): Promise<OutputChunk> {
-  const processing = workerConfigSemaphore.get(config)
-  if (processing) {
-    await processing
-    return bundleWorkerEntry(config, id, query)
-  }
-  const promise = serialBundleWorkerEntry(config, id, query)
-  workerConfigSemaphore.set(config, promise)
-  promise.then(() => workerConfigSemaphore.delete(config))
-  return promise
-}
-
-async function serialBundleWorkerEntry(
+async function bundleWorkerEntry(
   config: ResolvedConfig,
   id: string,
   query: Record<string, string> | null,
@@ -75,7 +52,7 @@ async function serialBundleWorkerEntry(
   const bundle = await rollup({
     ...rollupOptions,
     input: cleanUrl(id),
-    plugins,
+    plugins: await plugins(),
     onwarn(warning, warn) {
       onRollupWarning(warning, warn, config)
     },

packages/vite/src/node/utils.ts

@@ -1012,6 +1012,16 @@ export function removeComments(raw: string): string {
   return raw.replace(multilineCommentsRE, '').replace(singlelineCommentsRE, '')
 }
 
+function backwardCompatibleWorkerPlugins(plugins: any) {
+  if (Array.isArray(plugins)) {
+    return plugins
+  }
+  if (typeof plugins === 'function') {
+    return plugins()
+  }
+  return []
+}
+
 function mergeConfigRecursively(
   defaults: Record<string, any>,
   overrides: Record<string, any>,
@@ -1045,6 +1055,12 @@ function mergeConfigRecursively(
     ) {
       merged[key] = true
       continue
+    } else if (key === 'plugins' && rootPath === 'worker') {
+      merged[key] = () => [
+        ...backwardCompatibleWorkerPlugins(existing),
+        ...backwardCompatibleWorkerPlugins(value),
+      ]
+      continue
     }
 
     if (Array.isArray(existing) || Array.isArray(value)) {

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

@@ -80,12 +80,30 @@ test('worker emitted and import.meta.url in nested worker (serve)', async () =>
   )
 })
 
+test('deeply nested workers', async () => {
+  await untilUpdated(
+    async () => page.textContent('.deeply-nested-worker'),
+    /Hello\sfrom\sroot.*\/es\/.+deeply-nested-worker\.js/,
+    true,
+  )
+  await untilUpdated(
+    async () => page.textContent('.deeply-nested-second-worker'),
+    /Hello\sfrom\ssecond.*\/es\/.+second-worker\.js/,
+    true,
+  )
+  await untilUpdated(
+    async () => page.textContent('.deeply-nested-third-worker'),
+    /Hello\sfrom\sthird.*\/es\/.+third-worker\.js/,
+    true,
+  )
+})
+
 describe.runIf(isBuild)('build', () => {
   // assert correct files
   test('inlined code generation', async () => {
     const assetsDir = path.resolve(testDir, 'dist/es/assets')
     const files = fs.readdirSync(assetsDir)
-    expect(files.length).toBe(28)
+    expect(files.length).toBe(32)
     const index = files.find((f) => f.includes('main-module'))
     const content = fs.readFileSync(path.resolve(assetsDir, index), 'utf-8')
     const worker = files.find((f) => f.includes('my-worker'))

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

@@ -74,10 +74,10 @@ describe.runIf(isBuild)('build', () => {
   test('inlined code generation', async () => {
     const assetsDir = path.resolve(testDir, 'dist/iife/assets')
     const files = fs.readdirSync(assetsDir)
-    expect(files.length).toBe(16)
+    expect(files.length).toBe(20)
     const index = files.find((f) => f.includes('main-module'))
     const content = fs.readFileSync(path.resolve(assetsDir, index), 'utf-8')
-    const worker = files.find((f) => f.includes('my-worker'))
+    const worker = files.find((f) => f.includes('worker_entry-my-worker'))
     const workerContent = fs.readFileSync(
       path.resolve(assetsDir, worker),
       'utf-8',

playground/worker/__tests__/sourcemap-hidden/sourcemap-hidden-worker.spec.ts

@@ -10,7 +10,7 @@ describe.runIf(isBuild)('build', () => {
 
     const files = fs.readdirSync(assetsDir)
     // should have 2 worker chunk
-    expect(files.length).toBe(32)
+    expect(files.length).toBe(40)
     const index = files.find((f) => f.includes('main-module'))
     const content = fs.readFileSync(path.resolve(assetsDir, index), 'utf-8')
     const indexSourcemap = getSourceMapUrl(content)

playground/worker/__tests__/sourcemap-inline/sourcemap-inline-worker.spec.ts

@@ -10,7 +10,7 @@ describe.runIf(isBuild)('build', () => {
 
     const files = fs.readdirSync(assetsDir)
     // should have 2 worker chunk
-    expect(files.length).toBe(16)
+    expect(files.length).toBe(20)
     const index = files.find((f) => f.includes('main-module'))
     const content = fs.readFileSync(path.resolve(assetsDir, index), 'utf-8')
     const indexSourcemap = getSourceMapUrl(content)

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

@@ -9,7 +9,7 @@ describe.runIf(isBuild)('build', () => {
     const assetsDir = path.resolve(testDir, 'dist/iife-sourcemap/assets')
     const files = fs.readdirSync(assetsDir)
     // should have 2 worker chunk
-    expect(files.length).toBe(32)
+    expect(files.length).toBe(40)
     const index = files.find((f) => f.includes('main-module'))
     const content = fs.readFileSync(path.resolve(assetsDir, index), 'utf-8')
     const indexSourcemap = getSourceMapUrl(content)

playground/worker/deeply-nested-second-worker.js

@@ -0,0 +1,19 @@
+self.postMessage({
+  type: 'deeplyNestedSecondWorker',
+  data: [
+    'Hello from second level nested worker',
+    import.meta.env.BASE_URL,
+    self.location.url,
+    import.meta.url,
+  ].join(' '),
+})
+
+const deeplyNestedThirdWorker = new Worker(
+  new URL('deeply-nested-third-worker.js', import.meta.url),
+  { type: 'module' },
+)
+deeplyNestedThirdWorker.addEventListener('message', (ev) => {
+  self.postMessage(ev.data)
+})
+
+console.log('deeply-nested-second-worker.js')

playground/worker/deeply-nested-third-worker.js

@@ -0,0 +1,11 @@
+self.postMessage({
+  type: 'deeplyNestedThirdWorker',
+  data: [
+    'Hello from third level nested worker',
+    import.meta.env.BASE_URL,
+    self.location.url,
+    import.meta.url,
+  ].join(' '),
+})
+
+console.log('deeply-nested-third-worker.js')