From 02a7266e3c32c196fe733a5d3480f9e308cb62ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joaqu=C3=ADn=20S=C3=A1nchez?= Date: Mon, 27 Feb 2023 22:26:11 +0100 Subject: [PATCH] fix: don't mix Vite plugins when spawning temporary Vite server (#6368) * fix: don't mix Vite plugins when spawning temporary Vite server * chore: include command to `createVite` options * chore: use `command` and exclude `preview` * chore: add test * fix(test): remove command check from apply fn * chore: add hint about filtering vite plugins and command * chore: apply suggestion Co-authored-by: Ben Holmes --------- Co-authored-by: Nate Moore Co-authored-by: Ben Holmes --- .changeset/green-wombats-pay.md | 5 ++ packages/astro/src/core/build/index.ts | 2 +- packages/astro/src/core/create-vite.ts | 37 ++++++++- packages/astro/src/core/dev/container.ts | 2 +- .../test/static-build-vite-plugins.test.js | 76 +++++++++++++++++++ packages/webapi/mod.d.ts | 24 +++--- 6 files changed, 130 insertions(+), 16 deletions(-) create mode 100644 .changeset/green-wombats-pay.md create mode 100644 packages/astro/test/static-build-vite-plugins.test.js diff --git a/.changeset/green-wombats-pay.md b/.changeset/green-wombats-pay.md new file mode 100644 index 000000000000..031a85e959f2 --- /dev/null +++ b/.changeset/green-wombats-pay.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fix regression that caused some stateful Vite plugins to assume they were running in `dev` mode during the `build` and vice versa. diff --git a/packages/astro/src/core/build/index.ts b/packages/astro/src/core/build/index.ts index 638d78d105b9..206bed6e417e 100644 --- a/packages/astro/src/core/build/index.ts +++ b/packages/astro/src/core/build/index.ts @@ -77,7 +77,7 @@ class AstroBuilder { middlewareMode: true, }, }, - { settings: this.settings, logging, mode: 'build' } + { settings: this.settings, logging, mode: 'build', command: 'build' } ); await runHookConfigDone({ settings: this.settings, logging }); diff --git a/packages/astro/src/core/create-vite.ts b/packages/astro/src/core/create-vite.ts index c25668af4a83..478a7b5fd31d 100644 --- a/packages/astro/src/core/create-vite.ts +++ b/packages/astro/src/core/create-vite.ts @@ -30,6 +30,8 @@ interface CreateViteOptions { settings: AstroSettings; logging: LogOptions; mode: 'dev' | 'build' | string; + // will be undefined when using `sync` + command?: 'dev' | 'build'; fs?: typeof nodeFs; } @@ -48,7 +50,7 @@ const ALWAYS_NOEXTERNAL = [ /** Return a common starting point for all Vite actions */ export async function createVite( commandConfig: vite.InlineConfig, - { settings, logging, mode, fs = nodeFs }: CreateViteOptions + { settings, logging, mode, command, fs = nodeFs }: CreateViteOptions ): Promise { const astroPkgsConfig = await crawlFrameworkPkgs({ root: fileURLToPath(settings.config.root), @@ -170,7 +172,38 @@ export async function createVite( // 3. integration-provided vite config, via the `config:setup` hook // 4. command vite config, passed as the argument to this function let result = commonConfig; - result = vite.mergeConfig(result, settings.config.vite || {}); + // PR #6238 Calls user integration `astro:config:setup` hooks when running `astro sync`. + // Without proper filtering, user integrations may run twice unexpectedly: + // - with `command` set to `build/dev` (src/core/build/index.ts L72) + // - and again in the `sync` module to generate `Content Collections` (src/core/sync/index.ts L36) + // We need to check if the command is `build` or `dev` before merging the user-provided vite config. + // We also need to filter out the plugins that are not meant to be applied to the current command: + // - If the command is `build`, we filter out the plugins that are meant to be applied for `serve`. + // - If the command is `dev`, we filter out the plugins that are meant to be applied for `build`. + if (command) { + let plugins = settings.config.vite?.plugins; + if (plugins) { + const { plugins: _, ...rest } = settings.config.vite + const applyToFilter = command === 'build' ? 'serve' : 'build' + const applyArgs = [{...settings.config.vite, mode}, { command, mode }] + // @ts-expect-error ignore TS2589: Type instantiation is excessively deep and possibly infinite. + plugins = plugins.flat(Infinity).filter((p) => { + if (!p || p?.apply === applyToFilter) { + return false; + } + + if (typeof p.apply === 'function') { + return p.apply(applyArgs[0], applyArgs[1]) + } + + return true; + }); + + result = vite.mergeConfig(result, { ...rest, plugins }); + } else { + result = vite.mergeConfig(result, settings.config.vite || {}); + } + } result = vite.mergeConfig(result, commandConfig); if (result.plugins) { sortPlugins(result.plugins); diff --git a/packages/astro/src/core/dev/container.ts b/packages/astro/src/core/dev/container.ts index 5b2b04835758..22c489b2e5da 100644 --- a/packages/astro/src/core/dev/container.ts +++ b/packages/astro/src/core/dev/container.ts @@ -92,7 +92,7 @@ export async function createContainer(params: CreateContainerParams = {}): Promi : 'undefined', }, }, - { settings, logging, mode: 'dev', fs } + { settings, logging, mode: 'dev', command: 'dev', fs } ); await runHookConfigDone({ settings, logging }); const viteServer = await vite.createServer(viteConfig); diff --git a/packages/astro/test/static-build-vite-plugins.test.js b/packages/astro/test/static-build-vite-plugins.test.js new file mode 100644 index 000000000000..10fe44957b33 --- /dev/null +++ b/packages/astro/test/static-build-vite-plugins.test.js @@ -0,0 +1,76 @@ +import { expect } from 'chai'; +import { loadFixture } from './test-utils.js'; + +describe('Static build: vite plugins included when required', () => { + /** @type {Map} */ + const pluginsCalled = new Map(); + /** @type {Map} */ + const expectedPluginResult = new Map([ + ['prepare-no-apply-plugin', true], + ['prepare-serve-plugin', false], + ['prepare-apply-fn-plugin', true], + ['prepare-dont-apply-fn-plugin', false], + ['prepare-build-plugin', true], + ]); + before(async () => { + /** @type {import('./test-utils').Fixture} */ + const fixture = await loadFixture({ + root: './fixtures/astro pages/', + integrations: [ + { + name: '@astrojs/prepare-vite-plugins', + hooks: { + 'astro:config:setup': ({ updateConfig }) => { + pluginsCalled.set('prepare-no-apply-plugin', false); + pluginsCalled.set('prepare-serve-plugin', false); + pluginsCalled.set('prepare-apply-fn-plugin', false); + pluginsCalled.set('prepare-dont-apply-fn-plugin', false); + pluginsCalled.set('prepare-build-plugin', false); + updateConfig({ + vite: { + plugins: [{ + name: 'prepare-no-apply-plugin', + configResolved: () => { + pluginsCalled.set('prepare-no-apply-plugin', true); + } + }, { + name: 'prepare-serve-plugin', + apply: 'serve', + configResolved: () => { + pluginsCalled.set('prepare-serve-plugin', true); + } + }, { + name: 'prepare-apply-fn-plugin', + apply: (_, { command }) => command === 'build', + configResolved: () => { + pluginsCalled.set('prepare-apply-fn-plugin', true); + } + }, { + name: 'prepare-dont-apply-fn-plugin', + apply: (_, { command }) => command === 'serve', + configResolved: () => { + pluginsCalled.set('prepare-dont-apply-fn-plugin', true); + } + }, { + name: 'prepare-build-plugin', + apply: 'build', + configResolved: () => { + pluginsCalled.set('prepare-build-plugin', true); + } + }] + } + }) + }, + }, + }, + ], + }); + await fixture.build(); + }); + it('Vite Plugins are included/excluded properly', async () => { + expect(pluginsCalled.size).to.equal(expectedPluginResult.size, 'Not all plugins were initialized'); + Array.from(expectedPluginResult.entries()).forEach(([plugin, called]) => + expect(pluginsCalled.get(plugin)).to.equal(called, `${plugin} was ${called ? 'not' : ''} called`) + ); + }); +}); diff --git a/packages/webapi/mod.d.ts b/packages/webapi/mod.d.ts index ecc90236488d..00be1f2746d3 100644 --- a/packages/webapi/mod.d.ts +++ b/packages/webapi/mod.d.ts @@ -1,13 +1,13 @@ // organize-imports-ignore -export { pathToPosix } from './lib/utils'; -export { alert, ByteLengthQueuingStrategy, cancelAnimationFrame, cancelIdleCallback, CanvasRenderingContext2D, CharacterData, clearTimeout, Comment, CountQueuingStrategy, CSSStyleSheet, CustomElementRegistry, CustomEvent, Document, DocumentFragment, DOMException, Element, Event, EventTarget, fetch, File, FormData, Headers, HTMLBodyElement, HTMLCanvasElement, HTMLDivElement, HTMLDocument, HTMLElement, HTMLHeadElement, HTMLHtmlElement, HTMLImageElement, HTMLSpanElement, HTMLStyleElement, HTMLTemplateElement, HTMLUnknownElement, Image, ImageData, IntersectionObserver, MediaQueryList, MutationObserver, Node, NodeFilter, NodeIterator, OffscreenCanvas, ReadableByteStreamController, ReadableStream, ReadableStreamBYOBReader, ReadableStreamBYOBRequest, ReadableStreamDefaultController, ReadableStreamDefaultReader, Request, requestAnimationFrame, requestIdleCallback, ResizeObserver, Response, setTimeout, ShadowRoot, structuredClone, StyleSheet, Text, TransformStream, TreeWalker, URLPattern, Window, WritableStream, WritableStreamDefaultController, WritableStreamDefaultWriter, } from './mod.js'; -export declare const polyfill: { - (target: any, options?: PolyfillOptions): any; - internals(target: any, name: string): any; -}; -interface PolyfillOptions { - exclude?: string | string[]; - override?: Record; -} \ No newline at end of file +export { pathToPosix } from './lib/utils'; +export { alert, ByteLengthQueuingStrategy, cancelAnimationFrame, cancelIdleCallback, CanvasRenderingContext2D, CharacterData, clearTimeout, Comment, CountQueuingStrategy, CSSStyleSheet, CustomElementRegistry, CustomEvent, Document, DocumentFragment, DOMException, Element, Event, EventTarget, fetch, File, FormData, Headers, HTMLBodyElement, HTMLCanvasElement, HTMLDivElement, HTMLDocument, HTMLElement, HTMLHeadElement, HTMLHtmlElement, HTMLImageElement, HTMLSpanElement, HTMLStyleElement, HTMLTemplateElement, HTMLUnknownElement, Image, ImageData, IntersectionObserver, MediaQueryList, MutationObserver, Node, NodeFilter, NodeIterator, OffscreenCanvas, ReadableByteStreamController, ReadableStream, ReadableStreamBYOBReader, ReadableStreamBYOBRequest, ReadableStreamDefaultController, ReadableStreamDefaultReader, Request, requestAnimationFrame, requestIdleCallback, ResizeObserver, Response, setTimeout, ShadowRoot, structuredClone, StyleSheet, Text, TransformStream, TreeWalker, URLPattern, Window, WritableStream, WritableStreamDefaultController, WritableStreamDefaultWriter, } from './mod.js'; +export declare const polyfill: { + (target: any, options?: PolyfillOptions): any; + internals(target: any, name: string): any; +}; +interface PolyfillOptions { + exclude?: string | string[]; + override?: Record; +} \ No newline at end of file