From ea6cbd06a2580527786707ec735079ff9abd0ec0 Mon Sep 17 00:00:00 2001 From: Bjorn Lu Date: Fri, 19 Jan 2024 16:44:17 +0800 Subject: [PATCH] Refactor style-only HMR cache and performance (#9712) Co-authored-by: Nate Moore --- .changeset/eight-turtles-itch.md | 5 + packages/astro/src/core/compile/cache.ts | 42 ------ packages/astro/src/core/compile/index.ts | 7 +- .../astro/src/vite-plugin-astro/compile.ts | 22 ++- packages/astro/src/vite-plugin-astro/hmr.ts | 88 ++++++------ packages/astro/src/vite-plugin-astro/index.ts | 131 ++++++++---------- packages/astro/src/vite-plugin-astro/types.ts | 12 +- .../test/units/compile/invalid-css.test.js | 4 +- .../units/vite-plugin-astro/compile.test.js | 9 +- 9 files changed, 145 insertions(+), 175 deletions(-) create mode 100644 .changeset/eight-turtles-itch.md delete mode 100644 packages/astro/src/core/compile/cache.ts diff --git a/.changeset/eight-turtles-itch.md b/.changeset/eight-turtles-itch.md new file mode 100644 index 0000000000..dbac472b9a --- /dev/null +++ b/.changeset/eight-turtles-itch.md @@ -0,0 +1,5 @@ +--- +"astro": patch +--- + +Improves HMR behavior for style-only changes in `.astro` files diff --git a/packages/astro/src/core/compile/cache.ts b/packages/astro/src/core/compile/cache.ts deleted file mode 100644 index cdd9ddd558..0000000000 --- a/packages/astro/src/core/compile/cache.ts +++ /dev/null @@ -1,42 +0,0 @@ -import type { AstroConfig } from '../../@types/astro.js'; -import { compile, type CompileProps, type CompileResult } from './compile.js'; - -type CompilationCache = Map; - -const configCache = new WeakMap(); - -export function isCached(config: AstroConfig, filename: string) { - return configCache.has(config) && configCache.get(config)!.has(filename); -} - -export function getCachedCompileResult( - config: AstroConfig, - filename: string -): CompileResult | null { - if (!isCached(config, filename)) return null; - return configCache.get(config)!.get(filename)!; -} - -export function invalidateCompilation(config: AstroConfig, filename: string) { - if (configCache.has(config)) { - const cache = configCache.get(config)!; - cache.delete(filename); - } -} - -export async function cachedCompilation(props: CompileProps): Promise { - const { astroConfig, filename } = props; - let cache: CompilationCache; - if (!configCache.has(astroConfig)) { - cache = new Map(); - configCache.set(astroConfig, cache); - } else { - cache = configCache.get(astroConfig)!; - } - if (cache.has(filename)) { - return cache.get(filename)!; - } - const compileResult = await compile(props); - cache.set(filename, compileResult); - return compileResult; -} diff --git a/packages/astro/src/core/compile/index.ts b/packages/astro/src/core/compile/index.ts index 4a2094de75..0e61c584ac 100644 --- a/packages/astro/src/core/compile/index.ts +++ b/packages/astro/src/core/compile/index.ts @@ -1,8 +1,3 @@ -export { - cachedCompilation, - getCachedCompileResult, - invalidateCompilation, - isCached, -} from './cache.js'; +export { compile } from './compile.js'; export type { CompileProps, CompileResult } from './compile.js'; export type { TransformStyle } from './types.js'; diff --git a/packages/astro/src/vite-plugin-astro/compile.ts b/packages/astro/src/vite-plugin-astro/compile.ts index 85c2d97fc3..15fc9ba73b 100644 --- a/packages/astro/src/vite-plugin-astro/compile.ts +++ b/packages/astro/src/vite-plugin-astro/compile.ts @@ -1,15 +1,17 @@ import { transformWithEsbuild, type ESBuildTransformResult } from 'vite'; import type { AstroConfig } from '../@types/astro.js'; -import { cachedCompilation, type CompileProps, type CompileResult } from '../core/compile/index.js'; +import { compile, type CompileProps, type CompileResult } from '../core/compile/index.js'; import type { Logger } from '../core/logger/core.js'; import { getFileInfo } from '../vite-plugin-utils/index.js'; +import type { CompileMetadata } from './types.js'; -interface CachedFullCompilation { +interface CompileAstroOption { compileProps: CompileProps; + astroFileToCompileMetadata: Map; logger: Logger; } -interface FullCompileResult extends Omit { +export interface CompileAstroResult extends Omit { map: ESBuildTransformResult['map']; } @@ -23,15 +25,16 @@ interface EnhanceCompilerErrorOptions { const FRONTMATTER_PARSE_REGEXP = /^\-\-\-(.*)^\-\-\-/ms; -export async function cachedFullCompilation({ +export async function compileAstro({ compileProps, + astroFileToCompileMetadata, logger, -}: CachedFullCompilation): Promise { +}: CompileAstroOption): Promise { let transformResult: CompileResult; let esbuildResult: ESBuildTransformResult; try { - transformResult = await cachedCompilation(compileProps); + transformResult = await compile(compileProps); // Compile all TypeScript to JavaScript. // Also, catches invalid JS/TS in the compiled output before returning. esbuildResult = await transformWithEsbuild(transformResult.code, compileProps.filename, { @@ -76,6 +79,13 @@ export async function cachedFullCompilation({ } } + // Attach compile metadata to map for use by virtual modules + astroFileToCompileMetadata.set(compileProps.filename, { + originalCode: compileProps.source, + css: transformResult.css, + scripts: transformResult.scripts, + }); + return { ...transformResult, code: esbuildResult.code + SUFFIX, diff --git a/packages/astro/src/vite-plugin-astro/hmr.ts b/packages/astro/src/vite-plugin-astro/hmr.ts index b7e1dc48b7..949fc1d6cb 100644 --- a/packages/astro/src/vite-plugin-astro/hmr.ts +++ b/packages/astro/src/vite-plugin-astro/hmr.ts @@ -1,42 +1,32 @@ import path from 'node:path'; import { appendForwardSlash } from '@astrojs/internal-helpers/path'; import type { HmrContext } from 'vite'; -import type { AstroConfig } from '../@types/astro.js'; -import type { cachedCompilation } from '../core/compile/index.js'; -import { invalidateCompilation, isCached, type CompileResult } from '../core/compile/index.js'; import type { Logger } from '../core/logger/core.js'; +import type { CompileAstroResult } from './compile.js'; +import type { CompileMetadata } from './types.js'; export interface HandleHotUpdateOptions { - config: AstroConfig; logger: Logger; + compile: (code: string, filename: string) => Promise; astroFileToCssAstroDeps: Map>; - - compile: () => ReturnType; - source: string; + astroFileToCompileMetadata: Map; } export async function handleHotUpdate( ctx: HmrContext, - { config, logger, astroFileToCssAstroDeps, compile, source }: HandleHotUpdateOptions + { logger, compile, astroFileToCssAstroDeps, astroFileToCompileMetadata }: HandleHotUpdateOptions ) { - let isStyleOnlyChange = false; - if (ctx.file.endsWith('.astro') && isCached(config, ctx.file)) { - // Get the compiled result from the cache - const oldResult = await compile(); - // Skip HMR if source isn't changed - if (oldResult.source === source) return []; - // Invalidate to get fresh, uncached result to compare it to - invalidateCompilation(config, ctx.file); - const newResult = await compile(); - if (isStyleOnlyChanged(oldResult, newResult)) { - isStyleOnlyChange = true; - } - } else { - invalidateCompilation(config, ctx.file); - } - - if (isStyleOnlyChange) { + const oldCode = astroFileToCompileMetadata.get(ctx.file)?.originalCode; + const newCode = await ctx.read(); + // If only the style code has changed, e.g. editing the `color`, then we can directly invalidate + // the Astro CSS virtual modules only. The main Astro module's JS result will be the same and doesn't + // need to be invalidated. + if (oldCode && isStyleOnlyChanged(oldCode, newCode)) { logger.debug('watch', 'style-only change'); + // Re-compile the main Astro component (even though we know its JS result will be the same) + // so that `astroFileToCompileMetadata` gets a fresh set of compile metadata to be used + // by the virtual modules later in the `load()` hook. + await compile(newCode, ctx.file); // Only return the Astro styles that have changed! return ctx.modules.filter((mod) => mod.id?.includes('astro&type=style')); } @@ -68,25 +58,39 @@ export async function handleHotUpdate( } } -function isStyleOnlyChanged(oldResult: CompileResult, newResult: CompileResult) { - return ( - normalizeCode(oldResult.code) === normalizeCode(newResult.code) && - // If style tags are added/removed, we need to regenerate the main Astro file - // so that its CSS imports are also added/removed - oldResult.css.length === newResult.css.length && - !isArrayEqual(oldResult.css, newResult.css) - ); -} +const frontmatterRE = /^\-\-\-.*?^\-\-\-/ms; +const scriptRE = /.*?<\/script>/gs; +const styleRE = /.*?<\/style>/gs; -const astroStyleImportRE = /import\s*"[^"]+astro&type=style[^"]+";/g; -const sourceMappingUrlRE = /\/\/# sourceMappingURL=[^ ]+$/gm; +function isStyleOnlyChanged(oldCode: string, newCode: string) { + if (oldCode === newCode) return false; -/** - * Remove style-related code and sourcemap from the final astro output so they - * can be compared between non-style code - */ -function normalizeCode(code: string) { - return code.replace(astroStyleImportRE, '').replace(sourceMappingUrlRE, '').trim(); + // Before we can regex-capture style tags, we remove the frontmatter and scripts + // first as they could contain false-positive style tag matches. At the same time, + // we can also compare if they have changed and early out. + + // Strip off and compare frontmatter + let oldFrontmatter = ''; + let newFrontmatter = ''; + oldCode = oldCode.replace(frontmatterRE, (m) => ((oldFrontmatter = m), '')); + newCode = newCode.replace(frontmatterRE, (m) => ((newFrontmatter = m), '')); + if (oldFrontmatter !== newFrontmatter) return false; + + // Strip off and compare scripts + const oldScripts: string[] = []; + const newScripts: string[] = []; + oldCode = oldCode.replace(scriptRE, (m) => (oldScripts.push(m), '')); + newCode = newCode.replace(scriptRE, (m) => (newScripts.push(m), '')); + if (!isArrayEqual(oldScripts, newScripts)) return false; + + // Finally, we can compare styles + const oldStyles: string[] = []; + const newStyles: string[] = []; + oldCode.match(styleRE)?.forEach((m) => oldStyles.push(m)); + newCode.match(styleRE)?.forEach((m) => newStyles.push(m)); + // The length must also be the same for style only change. If style tags are added/removed, + // we need to regenerate the main Astro file so that its CSS imports are also added/removed + return oldStyles.length === newStyles.length && !isArrayEqual(oldStyles, newStyles); } function isArrayEqual(a: any[], b: any[]) { diff --git a/packages/astro/src/vite-plugin-astro/index.ts b/packages/astro/src/vite-plugin-astro/index.ts index 4a50091468..9cb36e4935 100644 --- a/packages/astro/src/vite-plugin-astro/index.ts +++ b/packages/astro/src/vite-plugin-astro/index.ts @@ -1,19 +1,12 @@ import type { SourceDescription } from 'rollup'; import type * as vite from 'vite'; -import type { AstroSettings } from '../@types/astro.js'; +import type { AstroConfig, AstroSettings } from '../@types/astro.js'; import type { Logger } from '../core/logger/core.js'; -import type { PluginMetadata as AstroPluginMetadata } from './types.js'; +import type { PluginMetadata as AstroPluginMetadata, CompileMetadata } from './types.js'; import { normalizePath } from 'vite'; -import { - cachedCompilation, - getCachedCompileResult, - type CompileProps, - invalidateCompilation, -} from '../core/compile/index.js'; -import { isRelativePath } from '../core/path.js'; import { normalizeFilename } from '../vite-plugin-utils/index.js'; -import { cachedFullCompilation } from './compile.js'; +import { compileAstro, type CompileAstroResult } from './compile.js'; import { handleHotUpdate } from './hmr.js'; import { parseAstroRequest } from './query.js'; export { getAstroMetadata } from './metadata.js'; @@ -24,17 +17,22 @@ interface AstroPluginOptions { logger: Logger; } +const astroFileToCompileMetadataWeakMap = new WeakMap>(); + /** Transform .astro files for Vite */ export default function astro({ settings, logger }: AstroPluginOptions): vite.Plugin[] { const { config } = settings; - let resolvedConfig: vite.ResolvedConfig; let server: vite.ViteDevServer | undefined; + let compile: (code: string, filename: string) => Promise; // Tailwind styles could register Astro files as dependencies of other Astro files, // causing circular imports which trips Vite's HMR. This set is passed to `handleHotUpdate` // to force a page reload when these dependency files are updated // NOTE: We need to initialize a map here and in `buildStart` because our unit tests don't // call `buildStart` (test bug) let astroFileToCssAstroDeps = new Map>(); + // Each Astro file has its own compile metadata so that its scripts and styles virtual module + // can retrieve their code from here. + let astroFileToCompileMetadata = new Map(); // Variables for determining if an id starts with /src... const srcRootWeb = config.srcDir.pathname.slice(config.root.pathname.length - 1); @@ -43,14 +41,41 @@ export default function astro({ settings, logger }: AstroPluginOptions): vite.Pl const prePlugin: vite.Plugin = { name: 'astro:build', enforce: 'pre', // run transforms before other plugins can - configResolved(_resolvedConfig) { - resolvedConfig = _resolvedConfig; + configResolved(viteConfig) { + // Initialize `compile` function to simplify usage later + compile = (code, filename) => { + return compileAstro({ + compileProps: { + astroConfig: config, + viteConfig, + preferences: settings.preferences, + filename, + source: code, + }, + astroFileToCompileMetadata, + logger, + }); + }; }, configureServer(_server) { server = _server; + // Make sure deleted files are removed from the compile metadata to save memory + server.watcher.on('unlink', (filename) => { + astroFileToCompileMetadata.delete(filename); + }); }, buildStart() { astroFileToCssAstroDeps = new Map(); + astroFileToCompileMetadata = new Map(); + + // Share the `astroFileToCompileMetadata` across the same Astro config as Astro performs + // multiple builds and its hoisted scripts analyzer requires the compile metadata from + // previous builds. Ideally this should not be needed when we refactor hoisted scripts analysis. + if (astroFileToCompileMetadataWeakMap.has(config)) { + astroFileToCompileMetadata = astroFileToCompileMetadataWeakMap.get(config)!; + } else { + astroFileToCompileMetadataWeakMap.set(config, astroFileToCompileMetadata); + } }, async load(id, opts) { const parsedId = parseAstroRequest(id); @@ -58,20 +83,18 @@ export default function astro({ settings, logger }: AstroPluginOptions): vite.Pl if (!query.astro) { return null; } - // For CSS / hoisted scripts, the main Astro module should already be cached - const filename = normalizePath(normalizeFilename(parsedId.filename, config.root)); - let compileResult = getCachedCompileResult(config, filename); - if (!compileResult) { - // In dev, HMR could cause this compile result to be empty, try to load it first - if (server) { - await server.transformRequest('/@fs' + filename); - compileResult = getCachedCompileResult(config, filename); - } - // If there's really no compilation result, error - if (!compileResult) { - throw new Error('No cached compile result found for ' + id); - } + // Astro scripts and styles virtual module code comes from the main Astro compilation + // through the metadata from `astroFileToCompileMetadata`. It should always exist as Astro + // modules are compiled first, then its virtual modules. If the virtual modules are somehow + // compiled first, throw an error and we should investigate it. + const filename = normalizePath(normalizeFilename(parsedId.filename, config.root)); + const compileMetadata = astroFileToCompileMetadata.get(filename); + if (!compileMetadata) { + throw new Error( + `No cached compile metadata found for "${id}". The main Astro module "${filename}" should have ` + + `compiled and filled the metadata first, before its virtual modules can be requested.` + ); } switch (query.type) { @@ -80,19 +103,12 @@ export default function astro({ settings, logger }: AstroPluginOptions): vite.Pl throw new Error(`Requests for Astro CSS must include an index.`); } - const code = compileResult.css[query.index]; + const code = compileMetadata.css[query.index]; if (!code) { throw new Error(`No Astro CSS at index ${query.index}`); } - return { - code, - meta: { - vite: { - isSelfAccepting: true, - }, - }, - }; + return { code }; } case 'script': { if (typeof query.index === 'undefined') { @@ -105,7 +121,7 @@ export default function astro({ settings, logger }: AstroPluginOptions): vite.Pl }; } - const hoistedScript = compileResult.scripts[query.index]; + const hoistedScript = compileMetadata.scripts[query.index]; if (!hoistedScript) { throw new Error(`No hoisted script at index ${query.index}`); } @@ -154,24 +170,9 @@ export default function astro({ settings, logger }: AstroPluginOptions): vite.Pl if (!id.endsWith('.astro') || parsedId.query.astro) { return; } - // if we still get a relative path here, vite couldn't resolve the import - if (isRelativePath(parsedId.filename)) { - return; - } - const compileProps: CompileProps = { - astroConfig: config, - viteConfig: resolvedConfig, - preferences: settings.preferences, - filename: normalizePath(parsedId.filename), - source, - }; - - // We invalidate and then compile again as we know Vite will only call this `transform` - // when its cache is invalidated. - // TODO: Do the compilation directly and remove our cache so we rely on Vite only. - invalidateCompilation(config, compileProps.filename); - const transformResult = await cachedFullCompilation({ compileProps, logger }); + const filename = normalizePath(parsedId.filename); + const transformResult = await compile(source, filename); // Register dependencies of this module const astroDeps = new Set(); @@ -190,7 +191,7 @@ export default function astro({ settings, logger }: AstroPluginOptions): vite.Pl // Here we manually invalidate the virtual modules ourselves when we're compiling the Astro module. // When that bug is resolved, we can add the dependencies to the virtual module directly and remove this. if (server) { - const mods = server.moduleGraph.getModulesByFile(compileProps.filename); + const mods = server.moduleGraph.getModulesByFile(filename); if (mods) { const seen = new Set(mods); for (const mod of mods) { @@ -223,24 +224,14 @@ export default function astro({ settings, logger }: AstroPluginOptions): vite.Pl }, }; }, - async handleHotUpdate(context) { - if (context.server.config.isProduction) return; - const filename = context.file; - const source = await context.read(); - const compile = () => - cachedCompilation({ - astroConfig: config, - viteConfig: resolvedConfig, - preferences: settings.preferences, - filename, - source, - }); - return handleHotUpdate(context, { - config, + async handleHotUpdate(ctx) { + if (!ctx.file.endsWith('.astro')) return; + + return handleHotUpdate(ctx, { logger, - astroFileToCssAstroDeps, compile, - source, + astroFileToCssAstroDeps, + astroFileToCompileMetadata, }); }, }; diff --git a/packages/astro/src/vite-plugin-astro/types.ts b/packages/astro/src/vite-plugin-astro/types.ts index 49bbd5feaf..589a0b0b03 100644 --- a/packages/astro/src/vite-plugin-astro/types.ts +++ b/packages/astro/src/vite-plugin-astro/types.ts @@ -1,5 +1,6 @@ -import type { TransformResult } from '@astrojs/compiler'; +import type { HoistedScript, TransformResult } from '@astrojs/compiler'; import type { PropagationHint } from '../@types/astro.js'; +import type { CompileAstroResult } from './compile.js'; export interface PageOptions { prerender?: boolean; @@ -15,3 +16,12 @@ export interface PluginMetadata { pageOptions: PageOptions; }; } + +export interface CompileMetadata { + /** Used for HMR to compare code changes */ + originalCode: string; + /** For Astro CSS virtual module */ + css: string[]; + /** For Astro hoisted scripts virtual module */ + scripts: HoistedScript[]; +} diff --git a/packages/astro/test/units/compile/invalid-css.test.js b/packages/astro/test/units/compile/invalid-css.test.js index 7a03034374..794e335b42 100644 --- a/packages/astro/test/units/compile/invalid-css.test.js +++ b/packages/astro/test/units/compile/invalid-css.test.js @@ -1,6 +1,6 @@ import { resolveConfig } from 'vite'; import { expect } from 'chai'; -import { cachedCompilation } from '../../../dist/core/compile/index.js'; +import { compile } from '../../../dist/core/compile/index.js'; import { AggregateError } from '../../../dist/core/errors/index.js'; import { pathToFileURL } from 'node:url'; @@ -9,7 +9,7 @@ describe('astro/src/core/compile', () => { it('throws an aggregate error with the errors', async () => { let error; try { - await cachedCompilation({ + await compile({ astroConfig: { root: pathToFileURL('/'), experimental: {}, diff --git a/packages/astro/test/units/vite-plugin-astro/compile.test.js b/packages/astro/test/units/vite-plugin-astro/compile.test.js index 4427d3acd0..c37506cfbb 100644 --- a/packages/astro/test/units/vite-plugin-astro/compile.test.js +++ b/packages/astro/test/units/vite-plugin-astro/compile.test.js @@ -1,6 +1,6 @@ import { expect } from 'chai'; import { resolveConfig } from 'vite'; -import { cachedFullCompilation } from '../../../dist/vite-plugin-astro/compile.js'; +import { compileAstro } from '../../../dist/vite-plugin-astro/compile.js'; import { init, parse } from 'es-module-lexer'; import { pathToFileURL } from 'node:url'; @@ -11,17 +11,14 @@ const viteConfig = await resolveConfig({ configFile: false }, 'serve'); * @param {string} id */ async function compile(source, id) { - return await cachedFullCompilation({ + return await compileAstro({ compileProps: { astroConfig: { root: pathToFileURL('/'), base: '/', experimental: {} }, viteConfig, filename: id, source, }, - logging: { - level: 'info', - }, - rawId: id, + astroFileToCompileMetadata: new Map(), }); }