From 9d09dfe53980ea882d99c3fc46ed5a2de98fba0d Mon Sep 17 00:00:00 2001 From: patak Date: Thu, 19 Oct 2023 11:36:24 +0200 Subject: [PATCH] fix!: worker.plugins is a function (#14685) Co-authored-by: jamsinclair Co-authored-by: Bjorn Lu --- docs/config/worker-options.md | 5 +- docs/guide/migration.md | 4 + packages/vite/src/node/config.ts | 130 ++++++++++-------- packages/vite/src/node/plugins/worker.ts | 27 +--- packages/vite/src/node/utils.ts | 16 +++ .../worker/__tests__/es/es-worker.spec.ts | 20 ++- .../worker/__tests__/iife/iife-worker.spec.ts | 4 +- .../sourcemap-hidden-worker.spec.ts | 2 +- .../sourcemap-inline-worker.spec.ts | 2 +- .../sourcemap/sourcemap-worker.spec.ts | 2 +- .../worker/deeply-nested-second-worker.js | 19 +++ .../worker/deeply-nested-third-worker.js | 11 ++ playground/worker/deeply-nested-worker.js | 19 +++ playground/worker/index.html | 21 +++ playground/worker/vite.config-es.js | 2 +- playground/worker/vite.config-iife.js | 38 ++--- .../worker/vite.config-relative-base-iife.js | 2 +- .../worker/vite.config-relative-base.js | 2 +- playground/worker/worker-sourcemap-config.js | 2 +- .../worker/worker/main-deeply-nested.js | 18 +++ playground/worker/worker/main.js | 1 + 21 files changed, 237 insertions(+), 110 deletions(-) create mode 100644 playground/worker/deeply-nested-second-worker.js create mode 100644 playground/worker/deeply-nested-third-worker.js create mode 100644 playground/worker/deeply-nested-worker.js create mode 100644 playground/worker/worker/main-deeply-nested.js diff --git a/docs/config/worker-options.md b/docs/config/worker-options.md index 99500ae259ccfe..30db36f3eda9e8 100644 --- a/docs/config/worker-options.md +++ b/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 diff --git a/docs/guide/migration.md b/docs/guide/migration.md index 3a979b566cfd37..4ac7ba1f10bc2a 100644 --- a/docs/guide/migration.md +++ b/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). diff --git a/packages/vite/src/node/config.ts b/packages/vite/src/node/config.ts index 7b18442069c988..56d3bb8a64d43d 100644 --- a/packages/vite/src/node/config.ts +++ b/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 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`, }, }) diff --git a/packages/vite/src/node/plugins/worker.ts b/packages/vite/src/node/plugins/worker.ts index c4ff6dff78bb2e..b4f26f83355552 100644 --- a/packages/vite/src/node/plugins/worker.ts +++ b/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 ->() -export async function bundleWorkerEntry( - config: ResolvedConfig, - id: string, - query: Record | null, -): Promise { - 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 | 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) }, diff --git a/packages/vite/src/node/utils.ts b/packages/vite/src/node/utils.ts index 31d5f730ec0f74..52d0ff6a6c0fc0 100644 --- a/packages/vite/src/node/utils.ts +++ b/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, overrides: Record, @@ -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)) { diff --git a/playground/worker/__tests__/es/es-worker.spec.ts b/playground/worker/__tests__/es/es-worker.spec.ts index 2cc9e07a5d9277..8e2ecf9ea598a0 100644 --- a/playground/worker/__tests__/es/es-worker.spec.ts +++ b/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')) diff --git a/playground/worker/__tests__/iife/iife-worker.spec.ts b/playground/worker/__tests__/iife/iife-worker.spec.ts index 934e608dadeaa5..ea296defdcc942 100644 --- a/playground/worker/__tests__/iife/iife-worker.spec.ts +++ b/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', diff --git a/playground/worker/__tests__/sourcemap-hidden/sourcemap-hidden-worker.spec.ts b/playground/worker/__tests__/sourcemap-hidden/sourcemap-hidden-worker.spec.ts index 84113ee6414355..6d9a6ee55664d6 100644 --- a/playground/worker/__tests__/sourcemap-hidden/sourcemap-hidden-worker.spec.ts +++ b/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) diff --git a/playground/worker/__tests__/sourcemap-inline/sourcemap-inline-worker.spec.ts b/playground/worker/__tests__/sourcemap-inline/sourcemap-inline-worker.spec.ts index 4480d4cd5c6be0..0bfa3ca3b8b816 100644 --- a/playground/worker/__tests__/sourcemap-inline/sourcemap-inline-worker.spec.ts +++ b/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) diff --git a/playground/worker/__tests__/sourcemap/sourcemap-worker.spec.ts b/playground/worker/__tests__/sourcemap/sourcemap-worker.spec.ts index 3f89fd694d4971..a9be2c75611079 100644 --- a/playground/worker/__tests__/sourcemap/sourcemap-worker.spec.ts +++ b/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) diff --git a/playground/worker/deeply-nested-second-worker.js b/playground/worker/deeply-nested-second-worker.js new file mode 100644 index 00000000000000..43c4cd75a90e7e --- /dev/null +++ b/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') diff --git a/playground/worker/deeply-nested-third-worker.js b/playground/worker/deeply-nested-third-worker.js new file mode 100644 index 00000000000000..199dec15fbd9ed --- /dev/null +++ b/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') diff --git a/playground/worker/deeply-nested-worker.js b/playground/worker/deeply-nested-worker.js new file mode 100644 index 00000000000000..4712082b571334 --- /dev/null +++ b/playground/worker/deeply-nested-worker.js @@ -0,0 +1,19 @@ +self.postMessage({ + type: 'deeplyNestedWorker', + data: [ + 'Hello from root worker', + import.meta.env.BASE_URL, + self.location.url, + import.meta.url, + ].join(' '), +}) + +const deeplyNestedSecondWorker = new Worker( + new URL('deeply-nested-second-worker.js', import.meta.url), + { type: 'module' }, +) +deeplyNestedSecondWorker.addEventListener('message', (ev) => { + self.postMessage(ev.data) +}) + +console.log('deeply-nested-worker.js') diff --git a/playground/worker/index.html b/playground/worker/index.html index bfce46fa0d7f90..cf019b9a549020 100644 --- a/playground/worker/index.html +++ b/playground/worker/index.html @@ -145,6 +145,27 @@

format iife:

+

+ new Worker(new URL('../deeply-nested-worker.js', import.meta.url), { type: + 'module' }) + .deeply-nested-worker +

+ + +

+ new Worker(new URL('deeply-nested-second-worker.js', import.meta.url), { type: + 'module' }) + .deeply-nested-second-worker +

+ + +

+ new Worker(new URL('deeply-nested-third-worker.js', import.meta.url), { type: + 'module' }) + .deeply-nested-third-worker +

+ +

diff --git a/playground/worker/vite.config-es.js b/playground/worker/vite.config-es.js index 0049c5357fa0af..995902373d72a5 100644 --- a/playground/worker/vite.config-es.js +++ b/playground/worker/vite.config-es.js @@ -10,7 +10,7 @@ export default defineConfig({ }, worker: { format: 'es', - plugins: [workerPluginTestPlugin()], + plugins: () => [workerPluginTestPlugin()], rollupOptions: { output: { assetFileNames: 'assets/worker_asset-[name].[ext]', diff --git a/playground/worker/vite.config-iife.js b/playground/worker/vite.config-iife.js index 98757b8c00d2fd..7ac8220e25a7e4 100644 --- a/playground/worker/vite.config-iife.js +++ b/playground/worker/vite.config-iife.js @@ -10,28 +10,12 @@ export default defineConfig({ }, worker: { format: 'iife', - plugins: [ - workerPluginTestPlugin(), - { - name: 'config-test', - config() { - return { - worker: { - rollupOptions: { - output: { - entryFileNames: 'assets/worker_entry-[name].js', - }, - }, - }, - } - }, - }, - ], + plugins: () => [workerPluginTestPlugin()], rollupOptions: { output: { assetFileNames: 'assets/worker_asset-[name].[ext]', chunkFileNames: 'assets/worker_chunk-[name].js', - // should fix by config-test plugin + // should be overwritten to worker_entry-[name] by the config-test plugin entryFileNames: 'assets/worker_-[name].js', }, }, @@ -48,6 +32,22 @@ export default defineConfig({ }, }, }, - plugins: [workerPluginTestPlugin()], + plugins: [ + workerPluginTestPlugin(), + { + name: 'config-test', + config() { + return { + worker: { + rollupOptions: { + output: { + entryFileNames: 'assets/worker_entry-[name].js', + }, + }, + }, + } + }, + }, + ], cacheDir: 'node_modules/.vite-iife', }) diff --git a/playground/worker/vite.config-relative-base-iife.js b/playground/worker/vite.config-relative-base-iife.js index 6b3c7c6895f46a..3addae454039e0 100644 --- a/playground/worker/vite.config-relative-base-iife.js +++ b/playground/worker/vite.config-relative-base-iife.js @@ -10,7 +10,7 @@ export default defineConfig({ }, worker: { format: 'iife', - plugins: [workerPluginTestPlugin()], + plugins: () => [workerPluginTestPlugin()], rollupOptions: { output: { assetFileNames: 'worker-assets/worker_asset-[name]-[hash].[ext]', diff --git a/playground/worker/vite.config-relative-base.js b/playground/worker/vite.config-relative-base.js index a09aafa3b841aa..73fbaa33e32882 100644 --- a/playground/worker/vite.config-relative-base.js +++ b/playground/worker/vite.config-relative-base.js @@ -10,7 +10,7 @@ export default defineConfig({ }, worker: { format: 'es', - plugins: [workerPluginTestPlugin()], + plugins: () => [workerPluginTestPlugin()], rollupOptions: { output: { assetFileNames: 'worker-assets/worker_asset-[name]-[hash].[ext]', diff --git a/playground/worker/worker-sourcemap-config.js b/playground/worker/worker-sourcemap-config.js index 4ea0162f705119..25dd8aa83b70a0 100644 --- a/playground/worker/worker-sourcemap-config.js +++ b/playground/worker/worker-sourcemap-config.js @@ -24,7 +24,7 @@ export default (sourcemap) => { }, worker: { format: 'iife', - plugins: [workerPluginTestPlugin()], + plugins: () => [workerPluginTestPlugin()], rollupOptions: { output: { assetFileNames: 'assets/[name]-worker_asset[hash].[ext]', diff --git a/playground/worker/worker/main-deeply-nested.js b/playground/worker/worker/main-deeply-nested.js new file mode 100644 index 00000000000000..5580025d7ac2a0 --- /dev/null +++ b/playground/worker/worker/main-deeply-nested.js @@ -0,0 +1,18 @@ +const worker = new Worker( + new URL('../deeply-nested-worker.js', import.meta.url), + { type: 'module' }, +) + +function text(el, text) { + document.querySelector(el).textContent = text +} + +worker.addEventListener('message', (ev) => { + if (ev.data.type === 'deeplyNestedSecondWorker') { + text('.deeply-nested-second-worker', JSON.stringify(ev.data.data)) + } else if (ev.data.type === 'deeplyNestedThirdWorker') { + text('.deeply-nested-third-worker', JSON.stringify(ev.data.data)) + } else { + text('.deeply-nested-worker', JSON.stringify(ev.data.data)) + } +}) diff --git a/playground/worker/worker/main.js b/playground/worker/worker/main.js index ce434c38e79ce6..c710c9435adb11 100644 --- a/playground/worker/worker/main.js +++ b/playground/worker/worker/main.js @@ -2,3 +2,4 @@ import('./main-module') import('./main-classic') import('./main-url') +import('./main-deeply-nested')