Refactor Astro plugin and compile flow (#5506)

.changeset/lazy-walls-sneeze.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Dedupe Astro package when resolving

.changeset/seven-seahorses-talk.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Refactor Astro compile flow

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<string, CompileResult>;
+
+const configCache = new WeakMap<AstroConfig, CompilationCache>();
+
+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<CompileResult> {
+	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;
+}

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 } from '../util.js';
-import { resolvePath, viteID } from '../util.js';
 import { createStylePreprocessor } from './style.js';
 
-type CompilationCache = Map<string, CompileResult>;
-type CompileResult = TransformResult & {
-	cssDeps: Set<string>;
-	source: string;
-};
-
-const configCache = new WeakMap<AstroConfig, CompilationCache>();
-
 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<string>;
+	source: string;
+}
+
+export async function compile({
 	astroConfig,
 	viteConfig,
 	filename,
 	source,
 }: CompileProps): Promise<CompileResult> {
-	let cssDeps = new Set<string>();
+	const cssDeps = new Set<string>();
-	let cssTransformErrors: AstroError[] = [];
+	const cssTransformErrors: AstroError[] = [];
+	let transformResult: TransformResult;
 
-	// Transform from `.astro` to valid `.ts`
+	try {
-	// use `sourcemap: "both"` so that sourcemap is included in the code
+		// Transform from `.astro` to valid `.ts`
-	// result passed to esbuild, but also available in the catch handler.
+		// use `sourcemap: "both"` so that sourcemap is included in the code
-	const transformResult = await transform(source, {
+		// result passed to esbuild, but also available in the catch handler.
-		pathname: filename,
+		transformResult = await transform(source, {
-		projectRoot: astroConfig.root.toString(),
+			pathname: filename,
-		site: astroConfig.site?.toString(),
+			projectRoot: astroConfig.root.toString(),
-		sourcefile: filename,
+			site: astroConfig.site?.toString(),
-		sourcemap: 'both',
+			sourcefile: filename,
-		internalURL: `/@fs${prependForwardSlash(
+			sourcemap: 'both',
-			viteID(new URL('../../runtime/server/index.js', import.meta.url))
+			internalURL: 'astro/server/index.js',
-		)}`,
+			// TODO: baseline flag
-		// TODO: baseline flag
+			experimentalStaticExtraction: true,
-		experimentalStaticExtraction: true,
+			preprocessStyle: createStylePreprocessor({
-		preprocessStyle: createStylePreprocessor({
+				filename,
-			filename,
+				viteConfig,
-			viteConfig,
+				cssDeps,
-			cssDeps,
+				cssTransformErrors,
-			cssTransformErrors,
+			}),
-		}),
+			async resolvePath(specifier) {
-		async resolvePath(specifier) {
+				return resolvePath(specifier, filename);
-			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,
-				});
-			}
-
-			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,
-					});
-				}
-			}
 		});
+	} 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, {
+	handleCompileResultErrors(transformResult, cssTransformErrors);
-		cssDeps: {
-			value: cssDeps,
-		},
-		source: {
-			value: source,
-		},
-	});
 
-	return compileResult;
+	return {
+		...transformResult,
+		cssDeps,
+		source,
+	};
 }
 
-export function isCached(config: AstroConfig, filename: string) {
+function handleCompileResultErrors(result: TransformResult, cssTransformErrors: AstroError[]) {
-	return configCache.has(config) && configCache.get(config)!.has(filename);
+	const compilerError = result.diagnostics.find((diag) => diag.severity === 1);
-}
 
-export function getCachedSource(config: AstroConfig, filename: string): string | null {
+	if (compilerError) {
-	if (!isCached(config, filename)) return null;
+		throw new CompilerError({
-	let src = configCache.get(config)!.get(filename);
+			code: compilerError.code,
-	if (!src) return null;
+			message: compilerError.text,
-	return src.source;
+			location: {
-}
+				line: compilerError.location.line,
+				column: compilerError.location.column,
+				file: compilerError.location.file,
+			},
+			hint: compilerError.hint,
+		});
+	}
 
-export function invalidateCompilation(config: AstroConfig, filename: string) {
+	switch (cssTransformErrors.length) {
-	if (configCache.has(config)) {
+		case 0:
-		const cache = configCache.get(config)!;
+			break;
-		cache.delete(filename);
+		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,
+			});
+		}
 	}
 }
-
-export async function cachedCompilation(props: CompileProps): Promise<CompileResult> {
-	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;
-}

packages/astro/src/core/compile/index.ts

@@ -1,3 +1,8 @@
-export type { CompileProps } from './compile';
+export {
-export { cachedCompilation, getCachedSource, invalidateCompilation, isCached } from './compile.js';
+	cachedCompilation,
+	getCachedCompileResult,
+	invalidateCompilation,
+	isCached,
+} from './cache.js';
+export type { CompileProps, CompileResult } from './compile';
 export type { TransformStyle } from './types';

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: [

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<CompileResult, 'map'> {
+	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<FullCompileResult> {
+	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<never> {
+	// 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;
+}

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, the main Astro module should already be cached
-			// For CSS / hoisted scripts we need to load the source ourselves.
+			const filename = normalizeFilename(parsedId.filename, config);
-			// It should be in the compilation cache at this point.
+			const compileResult = getCachedCompileResult(config, filename);
-			let raw = await this.resolve(filename, undefined);
+			if (!compileResult) {
-			if (!raw) {
 				return null;
 			}
-
-			let source = getCachedSource(config, raw.id);
-			if (!source) {
-				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 code = compileResult.css[query.index];
-					const csses = transformResult.css;
+					if (!code) {
-					const code = csses[query.index];
+						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 hoistedScript = compileResult.scripts[query.index];
-					const scripts = transformResult.scripts;
-					const hoistedScript = 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 cachedFullCompilation({
-				const transformResult = await cachedCompilation(compileProps);
+				compileProps,
-				const { fileId: file, fileUrl: url } = getFileInfo(id, config);
+				rawId: id,
+				logging,
+			});
 
-				for (const dep of transformResult.cssDeps) {
+			for (const dep of transformResult.cssDeps) {
-					this.addWatchFile(dep);
+				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++;
-					}
-				}
-
-				// Prefer live reload to HMR in `.astro` files
-				if (!resolvedConfig.isProduction) {
-					SUFFIX += `\nif (import.meta.hot) { import.meta.hot.decline() }`;
-				}
-
-				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',
-						},
-					},
-				};
-			} 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;
 			}
+
+			const astroMetadata: AstroPluginMetadata['astro'] = {
+				clientOnlyComponents: transformResult.clientOnlyComponents,
+				hydratedComponents: transformResult.hydratedComponents,
+				scripts: transformResult.scripts,
+			};
+
+			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',
+					},
+				},
+			};
 		},
 		async handleHotUpdate(context) {
 			if (context.server.config.isProduction) return;

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);
 }