From f536a34e53121104f87f2ad117a539daf2b7d5ee Mon Sep 17 00:00:00 2001 From: Bjorn Lu Date: Mon, 5 Dec 2022 21:34:49 +0800 Subject: [PATCH] Refactor Astro plugin and compile flow (#5506) --- .changeset/lazy-walls-sneeze.md | 5 + .changeset/seven-seahorses-talk.md | 5 + packages/astro/src/core/compile/cache.ts | 42 ++++ packages/astro/src/core/compile/compile.ts | 202 +++++++----------- packages/astro/src/core/compile/index.ts | 9 +- packages/astro/src/core/create-vite.ts | 2 + .../astro/src/vite-plugin-astro/compile.ts | 147 +++++++++++++ packages/astro/src/vite-plugin-astro/index.ts | 177 ++++----------- packages/astro/src/vite-plugin-utils/index.ts | 12 +- 9 files changed, 335 insertions(+), 266 deletions(-) create mode 100644 .changeset/lazy-walls-sneeze.md create mode 100644 .changeset/seven-seahorses-talk.md create mode 100644 packages/astro/src/core/compile/cache.ts create mode 100644 packages/astro/src/vite-plugin-astro/compile.ts diff --git a/.changeset/lazy-walls-sneeze.md b/.changeset/lazy-walls-sneeze.md new file mode 100644 index 000000000000..117c163cc3ea --- /dev/null +++ b/.changeset/lazy-walls-sneeze.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Dedupe Astro package when resolving diff --git a/.changeset/seven-seahorses-talk.md b/.changeset/seven-seahorses-talk.md new file mode 100644 index 000000000000..fc130f8b07f3 --- /dev/null +++ b/.changeset/seven-seahorses-talk.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Refactor Astro compile flow diff --git a/packages/astro/src/core/compile/cache.ts b/packages/astro/src/core/compile/cache.ts new file mode 100644 index 000000000000..01de66af6f08 --- /dev/null +++ b/packages/astro/src/core/compile/cache.ts @@ -0,0 +1,42 @@ +import type { AstroConfig } from '../../@types/astro'; +import { compile, CompileProps, 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/compile.ts b/packages/astro/src/core/compile/compile.ts index a305ef5eca64..9936fc5e36e9 100644 --- a/packages/astro/src/core/compile/compile.ts +++ b/packages/astro/src/core/compile/compile.ts @@ -5,18 +5,9 @@ import type { AstroConfig } from '../../@types/astro'; import { transform } from '@astrojs/compiler'; import { AggregateError, AstroError, CompilerError } from '../errors/errors.js'; import { AstroErrorData } from '../errors/index.js'; -import { prependForwardSlash } from '../path.js'; -import { resolvePath, viteID } from '../util.js'; +import { resolvePath } from '../util.js'; import { createStylePreprocessor } from './style.js'; -type CompilationCache = Map; -type CompileResult = TransformResult & { - cssDeps: Set; - source: string; -}; - -const configCache = new WeakMap(); - export interface CompileProps { astroConfig: AstroConfig; viteConfig: ResolvedConfig; @@ -24,131 +15,98 @@ export interface CompileProps { source: string; } -async function compile({ +export interface CompileResult extends TransformResult { + cssDeps: Set; + source: string; +} + +export async function compile({ astroConfig, viteConfig, filename, source, }: CompileProps): Promise { - let cssDeps = new Set(); - let cssTransformErrors: AstroError[] = []; - - // Transform from `.astro` to valid `.ts` - // use `sourcemap: "both"` so that sourcemap is included in the code - // result passed to esbuild, but also available in the catch handler. - const transformResult = await transform(source, { - pathname: filename, - projectRoot: astroConfig.root.toString(), - site: astroConfig.site?.toString(), - sourcefile: filename, - sourcemap: 'both', - internalURL: `/@fs${prependForwardSlash( - viteID(new URL('../../runtime/server/index.js', import.meta.url)) - )}`, - // TODO: baseline flag - experimentalStaticExtraction: true, - preprocessStyle: createStylePreprocessor({ - filename, - viteConfig, - cssDeps, - cssTransformErrors, - }), - async resolvePath(specifier) { - return resolvePath(specifier, filename); - }, - }) - .catch((err: Error) => { - // The compiler should be able to handle errors by itself, however - // for the rare cases where it can't let's directly throw here with as much info as possible - throw new CompilerError({ - ...AstroErrorData.UnknownCompilerError, - message: err.message ?? 'Unknown compiler error', - stack: err.stack, - location: { - file: filename, - }, - }); - }) - .then((result) => { - const compilerError = result.diagnostics.find((diag) => diag.severity === 1); - - if (compilerError) { - throw new CompilerError({ - code: compilerError.code, - message: compilerError.text, - location: { - line: compilerError.location.line, - column: compilerError.location.column, - file: compilerError.location.file, - }, - hint: compilerError.hint, - }); - } + const cssDeps = new Set(); + const cssTransformErrors: AstroError[] = []; + let transformResult: TransformResult; - switch (cssTransformErrors.length) { - case 0: - return result; - case 1: { - let error = cssTransformErrors[0]; - if (!error.errorCode) { - error.errorCode = AstroErrorData.UnknownCSSError.code; - } - - throw cssTransformErrors[0]; - } - default: { - throw new AggregateError({ - ...cssTransformErrors[0], - code: cssTransformErrors[0].errorCode, - errors: cssTransformErrors, - }); - } - } + try { + // Transform from `.astro` to valid `.ts` + // use `sourcemap: "both"` so that sourcemap is included in the code + // result passed to esbuild, but also available in the catch handler. + transformResult = await transform(source, { + pathname: filename, + projectRoot: astroConfig.root.toString(), + site: astroConfig.site?.toString(), + sourcefile: filename, + sourcemap: 'both', + internalURL: 'astro/server/index.js', + // TODO: baseline flag + experimentalStaticExtraction: true, + preprocessStyle: createStylePreprocessor({ + filename, + viteConfig, + cssDeps, + cssTransformErrors, + }), + async resolvePath(specifier) { + return resolvePath(specifier, filename); + }, }); + } catch (err: any) { + // The compiler should be able to handle errors by itself, however + // for the rare cases where it can't let's directly throw here with as much info as possible + throw new CompilerError({ + ...AstroErrorData.UnknownCompilerError, + message: err.message ?? 'Unknown compiler error', + stack: err.stack, + location: { + file: filename, + }, + }); + } - const compileResult: CompileResult = Object.create(transformResult, { - cssDeps: { - value: cssDeps, - }, - source: { - value: source, - }, - }); - - return compileResult; -} + handleCompileResultErrors(transformResult, cssTransformErrors); -export function isCached(config: AstroConfig, filename: string) { - return configCache.has(config) && configCache.get(config)!.has(filename); + return { + ...transformResult, + cssDeps, + source, + }; } -export function getCachedSource(config: AstroConfig, filename: string): string | null { - if (!isCached(config, filename)) return null; - let src = configCache.get(config)!.get(filename); - if (!src) return null; - return src.source; -} +function handleCompileResultErrors(result: TransformResult, cssTransformErrors: AstroError[]) { + const compilerError = result.diagnostics.find((diag) => diag.severity === 1); -export function invalidateCompilation(config: AstroConfig, filename: string) { - if (configCache.has(config)) { - const cache = configCache.get(config)!; - cache.delete(filename); + if (compilerError) { + throw new CompilerError({ + code: compilerError.code, + message: compilerError.text, + location: { + line: compilerError.location.line, + column: compilerError.location.column, + file: compilerError.location.file, + }, + hint: compilerError.hint, + }); } -} -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)!; + switch (cssTransformErrors.length) { + case 0: + break; + case 1: { + const error = cssTransformErrors[0]; + if (!error.errorCode) { + error.errorCode = AstroErrorData.UnknownCSSError.code; + } + throw cssTransformErrors[0]; + } + default: { + throw new AggregateError({ + ...cssTransformErrors[0], + code: cssTransformErrors[0].errorCode, + errors: cssTransformErrors, + }); + } } - 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 1ee6af06a069..6851c7c5ecbc 100644 --- a/packages/astro/src/core/compile/index.ts +++ b/packages/astro/src/core/compile/index.ts @@ -1,3 +1,8 @@ -export type { CompileProps } from './compile'; -export { cachedCompilation, getCachedSource, invalidateCompilation, isCached } from './compile.js'; +export { + cachedCompilation, + getCachedCompileResult, + invalidateCompilation, + isCached, +} from './cache.js'; +export type { CompileProps, CompileResult } from './compile'; export type { TransformStyle } from './types'; diff --git a/packages/astro/src/core/create-vite.ts b/packages/astro/src/core/create-vite.ts index 0b075e21a0bb..4339563c46ae 100644 --- a/packages/astro/src/core/create-vite.ts +++ b/packages/astro/src/core/create-vite.ts @@ -151,6 +151,8 @@ export async function createVite( }, ], conditions: ['astro'], + // Astro imports in third-party packages should use the same version as root + dedupe: ['astro'], }, ssr: { noExternal: [ diff --git a/packages/astro/src/vite-plugin-astro/compile.ts b/packages/astro/src/vite-plugin-astro/compile.ts new file mode 100644 index 000000000000..f9e581e841fd --- /dev/null +++ b/packages/astro/src/vite-plugin-astro/compile.ts @@ -0,0 +1,147 @@ +import { fileURLToPath } from 'url'; +import { ESBuildTransformResult, transformWithEsbuild } from 'vite'; +import { AstroConfig } from '../@types/astro'; +import { cachedCompilation, CompileProps, CompileResult } from '../core/compile/index.js'; +import { LogOptions } from '../core/logger/core.js'; +import { getFileInfo } from '../vite-plugin-utils/index.js'; + +interface CachedFullCompilation { + compileProps: CompileProps; + rawId: string; + logging: LogOptions; +} + +interface FullCompileResult extends Omit { + map: ESBuildTransformResult['map']; +} + +interface EnhanceCompilerErrorOptions { + err: Error; + id: string; + source: string; + config: AstroConfig; + logging: LogOptions; +} + +const FRONTMATTER_PARSE_REGEXP = /^\-\-\-(.*)^\-\-\-/ms; + +export async function cachedFullCompilation({ + compileProps, + rawId, + logging, +}: CachedFullCompilation): Promise { + let transformResult: CompileResult; + let esbuildResult: ESBuildTransformResult; + + try { + transformResult = await cachedCompilation(compileProps); + // Compile all TypeScript to JavaScript. + // Also, catches invalid JS/TS in the compiled output before returning. + esbuildResult = await transformWithEsbuild(transformResult.code, rawId, { + loader: 'ts', + target: 'esnext', + sourcemap: 'external', + }); + } catch (err: any) { + await enhanceCompileError({ + err, + id: rawId, + source: compileProps.source, + config: compileProps.astroConfig, + logging: logging, + }); + throw err; + } + + const { fileId: file, fileUrl: url } = getFileInfo(rawId, compileProps.astroConfig); + + let SUFFIX = ''; + SUFFIX += `\nconst $$file = ${JSON.stringify(file)};\nconst $$url = ${JSON.stringify( + url + )};export { $$file as file, $$url as url };\n`; + + // Add HMR handling in dev mode. + if (!compileProps.viteConfig.isProduction) { + let i = 0; + while (i < transformResult.scripts.length) { + SUFFIX += `import "${rawId}?astro&type=script&index=${i}&lang.ts";`; + i++; + } + } + + // Prefer live reload to HMR in `.astro` files + if (!compileProps.viteConfig.isProduction) { + SUFFIX += `\nif (import.meta.hot) { import.meta.hot.decline() }`; + } + + return { + ...transformResult, + code: esbuildResult.code + SUFFIX, + map: esbuildResult.map, + }; +} + +async function enhanceCompileError({ + err, + id, + source, + config, + logging, +}: EnhanceCompilerErrorOptions): Promise { + // Verify frontmatter: a common reason that this plugin fails is that + // the user provided invalid JS/TS in the component frontmatter. + // If the frontmatter is invalid, the `err` object may be a compiler + // panic or some other vague/confusing compiled error message. + // + // Before throwing, it is better to verify the frontmatter here, and + // let esbuild throw a more specific exception if the code is invalid. + // If frontmatter is valid or cannot be parsed, then continue. + const scannedFrontmatter = FRONTMATTER_PARSE_REGEXP.exec(source); + if (scannedFrontmatter) { + try { + await transformWithEsbuild(scannedFrontmatter[1], id, { + loader: 'ts', + target: 'esnext', + sourcemap: false, + }); + } catch (frontmatterErr: any) { + // Improve the error by replacing the phrase "unexpected end of file" + // with "unexpected end of frontmatter" in the esbuild error message. + if (frontmatterErr && frontmatterErr.message) { + frontmatterErr.message = frontmatterErr.message.replace( + 'end of file', + 'end of frontmatter' + ); + } + throw frontmatterErr; + } + } + + // improve compiler errors + if (err.stack && err.stack.includes('wasm-function')) { + const search = new URLSearchParams({ + labels: 'compiler', + title: '🐛 BUG: `@astrojs/compiler` panic', + template: '---01-bug-report.yml', + 'bug-description': `\`@astrojs/compiler\` encountered an unrecoverable error when compiling the following file. + +**${id.replace(fileURLToPath(config.root), '')}** +\`\`\`astro +${source} +\`\`\``, + }); + (err as any).url = `https://github.com/withastro/astro/issues/new?${search.toString()}`; + err.message = `Error: Uh oh, the Astro compiler encountered an unrecoverable error! + + Please open + a GitHub issue using the link below: + ${(err as any).url}`; + + if (logging.level !== 'debug') { + // TODO: remove stack replacement when compiler throws better errors + err.stack = ` at ${id}`; + } + } + + throw err; +} diff --git a/packages/astro/src/vite-plugin-astro/index.ts b/packages/astro/src/vite-plugin-astro/index.ts index 59aea73a0bc9..f5c5e34f533e 100644 --- a/packages/astro/src/vite-plugin-astro/index.ts +++ b/packages/astro/src/vite-plugin-astro/index.ts @@ -4,10 +4,9 @@ import type { AstroSettings } from '../@types/astro'; import type { LogOptions } from '../core/logger/core.js'; import type { PluginMetadata as AstroPluginMetadata } from './types'; -import esbuild from 'esbuild'; import slash from 'slash'; import { fileURLToPath } from 'url'; -import { cachedCompilation, CompileProps, getCachedSource } from '../core/compile/index.js'; +import { cachedCompilation, CompileProps, getCachedCompileResult } from '../core/compile/index.js'; import { isRelativePath, prependForwardSlash, @@ -15,11 +14,11 @@ import { startsWithForwardSlash, } from '../core/path.js'; import { viteID } from '../core/util.js'; -import { getFileInfo, normalizeFilename } from '../vite-plugin-utils/index.js'; +import { normalizeFilename } from '../vite-plugin-utils/index.js'; import { handleHotUpdate } from './hmr.js'; import { parseAstroRequest, ParsedRequestResult } from './query.js'; +import { cachedFullCompilation } from './compile.js'; -const FRONTMATTER_PARSE_REGEXP = /^\-\-\-(.*)^\-\-\-/ms; interface AstroPluginOptions { settings: AstroSettings; logging: LogOptions; @@ -103,35 +102,22 @@ export default function astro({ settings, logging }: AstroPluginOptions): vite.P if (!query.astro) { return null; } - let filename = parsedId.filename; - // For CSS / hoisted scripts we need to load the source ourselves. - // It should be in the compilation cache at this point. - let raw = await this.resolve(filename, undefined); - if (!raw) { - return null; - } - - let source = getCachedSource(config, raw.id); - if (!source) { + // For CSS / hoisted scripts, the main Astro module should already be cached + const filename = normalizeFilename(parsedId.filename, config); + const compileResult = getCachedCompileResult(config, filename); + if (!compileResult) { return null; } - - const compileProps: CompileProps = { - astroConfig: config, - viteConfig: resolvedConfig, - filename, - source, - }; - switch (query.type) { case 'style': { if (typeof query.index === 'undefined') { throw new Error(`Requests for Astro CSS must include an index.`); } - const transformResult = await cachedCompilation(compileProps); - const csses = transformResult.css; - const code = csses[query.index]; + const code = compileResult.css[query.index]; + if (!code) { + throw new Error(`No Astro CSS at index ${query.index}`); + } return { code, @@ -153,10 +139,7 @@ export default function astro({ settings, logging }: AstroPluginOptions): vite.P }; } - const transformResult = await cachedCompilation(compileProps); - const scripts = transformResult.scripts; - const hoistedScript = scripts[query.index]; - + const hoistedScript = compileResult.scripts[query.index]; if (!hoistedScript) { throw new Error(`No hoisted script at index ${query.index}`); } @@ -171,7 +154,7 @@ export default function astro({ settings, logging }: AstroPluginOptions): vite.P } } - let result: SourceDescription & { meta: any } = { + const result: SourceDescription = { code: '', meta: { vite: { @@ -182,7 +165,7 @@ export default function astro({ settings, logging }: AstroPluginOptions): vite.P switch (hoistedScript.type) { case 'inline': { - let { code, map } = hoistedScript; + const { code, map } = hoistedScript; result.code = appendSourceMap(code, map); break; } @@ -218,118 +201,34 @@ export default function astro({ settings, logging }: AstroPluginOptions): vite.P source, }; - try { - const transformResult = await cachedCompilation(compileProps); - const { fileId: file, fileUrl: url } = getFileInfo(id, config); - - for (const dep of transformResult.cssDeps) { - this.addWatchFile(dep); - } - - // Compile all TypeScript to JavaScript. - // Also, catches invalid JS/TS in the compiled output before returning. - const { code, map } = await esbuild.transform(transformResult.code, { - loader: 'ts', - sourcemap: 'external', - sourcefile: id, - // Pass relevant Vite options, if needed: - define: config.vite?.define, - }); - - let SUFFIX = ''; - SUFFIX += `\nconst $$file = ${JSON.stringify(file)};\nconst $$url = ${JSON.stringify( - url - )};export { $$file as file, $$url as url };\n`; - // Add HMR handling in dev mode. - if (!resolvedConfig.isProduction) { - let i = 0; - while (i < transformResult.scripts.length) { - SUFFIX += `import "${id}?astro&type=script&index=${i}&lang.ts";`; - i++; - } - } + const transformResult = await cachedFullCompilation({ + compileProps, + rawId: id, + logging, + }); - // Prefer live reload to HMR in `.astro` files - if (!resolvedConfig.isProduction) { - SUFFIX += `\nif (import.meta.hot) { import.meta.hot.decline() }`; - } + for (const dep of transformResult.cssDeps) { + this.addWatchFile(dep); + } - const astroMetadata: AstroPluginMetadata['astro'] = { - clientOnlyComponents: transformResult.clientOnlyComponents, - hydratedComponents: transformResult.hydratedComponents, - scripts: transformResult.scripts, - }; + const astroMetadata: AstroPluginMetadata['astro'] = { + clientOnlyComponents: transformResult.clientOnlyComponents, + hydratedComponents: transformResult.hydratedComponents, + scripts: transformResult.scripts, + }; - return { - code: `${code}${SUFFIX}`, - map, - meta: { - astro: astroMetadata, - vite: { - // Setting this vite metadata to `ts` causes Vite to resolve .js - // extensions to .ts files. - lang: 'ts', - }, + return { + code: transformResult.code, + map: transformResult.map, + meta: { + astro: astroMetadata, + vite: { + // Setting this vite metadata to `ts` causes Vite to resolve .js + // extensions to .ts files. + lang: 'ts', }, - }; - } catch (err: any) { - // Verify frontmatter: a common reason that this plugin fails is that - // the user provided invalid JS/TS in the component frontmatter. - // If the frontmatter is invalid, the `err` object may be a compiler - // panic or some other vague/confusing compiled error message. - // - // Before throwing, it is better to verify the frontmatter here, and - // let esbuild throw a more specific exception if the code is invalid. - // If frontmatter is valid or cannot be parsed, then continue. - const scannedFrontmatter = FRONTMATTER_PARSE_REGEXP.exec(source); - if (scannedFrontmatter) { - try { - await esbuild.transform(scannedFrontmatter[1], { - loader: 'ts', - sourcemap: false, - sourcefile: id, - }); - } catch (frontmatterErr: any) { - // Improve the error by replacing the phrase "unexpected end of file" - // with "unexpected end of frontmatter" in the esbuild error message. - if (frontmatterErr && frontmatterErr.message) { - frontmatterErr.message = frontmatterErr.message.replace( - 'end of file', - 'end of frontmatter' - ); - } - throw frontmatterErr; - } - } - - // improve compiler errors - if (err.stack && err.stack.includes('wasm-function')) { - const search = new URLSearchParams({ - labels: 'compiler', - title: '🐛 BUG: `@astrojs/compiler` panic', - template: '---01-bug-report.yml', - 'bug-description': `\`@astrojs/compiler\` encountered an unrecoverable error when compiling the following file. - -**${id.replace(fileURLToPath(config.root), '')}** -\`\`\`astro -${source} -\`\`\``, - }); - err.url = `https://github.com/withastro/astro/issues/new?${search.toString()}`; - err.message = `Error: Uh oh, the Astro compiler encountered an unrecoverable error! - - Please open - a GitHub issue using the link below: - ${err.url}`; - - if (logging.level !== 'debug') { - // TODO: remove stack replacement when compiler throws better errors - err.stack = ` at ${id}`; - } - } - - throw err; - } + }, + }; }, async handleHotUpdate(context) { if (context.server.config.isProduction) return; diff --git a/packages/astro/src/vite-plugin-utils/index.ts b/packages/astro/src/vite-plugin-utils/index.ts index bea28877e92b..82de4c097893 100644 --- a/packages/astro/src/vite-plugin-utils/index.ts +++ b/packages/astro/src/vite-plugin-utils/index.ts @@ -1,7 +1,13 @@ import ancestor from 'common-ancestor-path'; +import path from 'path'; +import { fileURLToPath } from 'url'; import { Data } from 'vfile'; import type { AstroConfig, MarkdownAstroData } from '../@types/astro'; -import { appendExtension, appendForwardSlash } from '../core/path.js'; +import { + appendExtension, + appendForwardSlash, + removeLeadingForwardSlashWindows, +} from '../core/path.js'; export function getFileInfo(id: string, config: AstroConfig) { const sitePathname = appendForwardSlash( @@ -56,7 +62,7 @@ export function safelyGetAstroData(vfileData: Data): MarkdownAstroData { * - /@fs/home/user/project/src/pages/index.astro * - /src/pages/index.astro * - * as absolute file paths. + * as absolute file paths with forward slashes. */ export function normalizeFilename(filename: string, config: AstroConfig) { if (filename.startsWith('/@fs')) { @@ -64,5 +70,5 @@ export function normalizeFilename(filename: string, config: AstroConfig) { } else if (filename.startsWith('/') && !ancestor(filename, config.root.pathname)) { filename = new URL('.' + filename, config.root).pathname; } - return filename; + return removeLeadingForwardSlashWindows(filename); }