Refactor CSS preprocessing handling (#5236)

.changeset/tall-keys-hunt.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Refactor CSS preprocessing handling

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

@@ -1,6 +1,6 @@
 import type { TransformResult } from '@astrojs/compiler';
+import type { ResolvedConfig } from 'vite';
 import type { AstroConfig } from '../../@types/astro';
-import type { TransformStyle } from './types';
 
 import { transform } from '@astrojs/compiler';
 import { AstroErrorCodes } from '../errors/codes.js';
@@ -18,17 +18,17 @@ type CompileResult = TransformResult & {
 const configCache = new WeakMap<AstroConfig, CompilationCache>();
 
 export interface CompileProps {
-	config: AstroConfig;
+	astroConfig: AstroConfig;
+	viteConfig: ResolvedConfig;
 	filename: string;
 	source: string;
-	transformStyle: TransformStyle;
 }
 
 async function compile({
-	config,
+	astroConfig,
+	viteConfig,
 	filename,
 	source,
-	transformStyle,
 }: CompileProps): Promise<CompileResult> {
 	let cssDeps = new Set<string>();
 	let cssTransformErrors: AstroError[] = [];
@@ -38,8 +38,8 @@ async function compile({
 	// result passed to esbuild, but also available in the catch handler.
 	const transformResult = await transform(source, {
 		pathname: filename,
-		projectRoot: config.root.toString(),
+		projectRoot: astroConfig.root.toString(),
-		site: config.site?.toString(),
+		site: astroConfig.site?.toString(),
 		sourcefile: filename,
 		sourcemap: 'both',
 		internalURL: `/@fs${prependForwardSlash(
@@ -47,7 +47,12 @@ async function compile({
 		)}`,
 		// TODO: baseline flag
 		experimentalStaticExtraction: true,
-		preprocessStyle: createStylePreprocessor(transformStyle, cssDeps, cssTransformErrors),
+		preprocessStyle: createStylePreprocessor({
+			filename,
+			viteConfig,
+			cssDeps,
+			cssTransformErrors,
+		}),
 		async resolvePath(specifier) {
 			return resolvePath(specifier, filename);
 		},
@@ -132,13 +137,13 @@ export function invalidateCompilation(config: AstroConfig, filename: string) {
 }
 
 export async function cachedCompilation(props: CompileProps): Promise<CompileResult> {
-	const { config, filename } = props;
+	const { astroConfig, filename } = props;
 	let cache: CompilationCache;
-	if (!configCache.has(config)) {
+	if (!configCache.has(astroConfig)) {
 		cache = new Map();
-		configCache.set(config, cache);
+		configCache.set(astroConfig, cache);
 	} else {
-		cache = configCache.get(config)!;
+		cache = configCache.get(astroConfig)!;
 	}
 	if (cache.has(filename)) {
 		return cache.get(filename)!;

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

@@ -1,27 +1,32 @@
+import fs from 'fs';
 import type { TransformOptions } from '@astrojs/compiler';
-import type { SourceMapInput } from 'rollup';
+import { preprocessCSS, ResolvedConfig } from 'vite';
-import type { TransformStyle } from './types';
+import { AstroErrorCodes } from '../errors/codes.js';
+import { CSSError } from '../errors/errors.js';
+import { positionAt } from '../errors/index.js';
 
-type PreprocessStyle = TransformOptions['preprocessStyle'];
+export function createStylePreprocessor({
-
+	filename,
-export function createStylePreprocessor(
+	viteConfig,
-	transformStyle: TransformStyle,
+	cssDeps,
-	cssDeps: Set<string>,
+	cssTransformErrors,
-	errors: Error[]
+}: {
-): PreprocessStyle {
+	filename: string;
-	const preprocessStyle: PreprocessStyle = async (value: string, attrs: Record<string, string>) => {
+	viteConfig: ResolvedConfig;
+	cssDeps: Set<string>;
+	cssTransformErrors: Error[];
+}): TransformOptions['preprocessStyle'] {
+	return async (content, attrs) => {
 		const lang = `.${attrs?.lang || 'css'}`.toLowerCase();
-
+		const id = `${filename}?astro&type=style&lang${lang}`;
 		try {
-			const result = await transformStyle(value, lang);
+			const result = await preprocessCSS(content, id, viteConfig);
 
-			if (!result) return null as any; // TODO: add type in compiler to fix "any"
+			result.deps?.forEach((dep) => {
-
-			for (const dep of result.deps) {
 				cssDeps.add(dep);
-			}
+			});
 
-			let map: SourceMapInput | undefined;
+			let map: string | undefined;
 			if (result.map) {
 				if (typeof result.map === 'string') {
 					map = result.map;
@@ -31,13 +36,65 @@ export function createStylePreprocessor(
 			}
 
 			return { code: result.code, map };
-		} catch (err) {
+		} catch (err: any) {
-			errors.push(err as unknown as Error);
+			try {
-			return {
+				err = enhanceCSSError(err, filename);
-				error: err + '',
+			} catch {}
-			};
+			cssTransformErrors.push(err);
+			return { error: err + '' };
 		}
 	};
-
+}
-	return preprocessStyle;
+
+function enhanceCSSError(err: any, filename: string) {
+	const fileContent = fs.readFileSync(filename).toString();
+	const styleTagBeginning = fileContent.indexOf(err.input?.source ?? err.code);
+
+	// PostCSS Syntax Error
+	if (err.name === 'CssSyntaxError') {
+		const errorLine = positionAt(styleTagBeginning, fileContent).line + (err.line ?? 0);
+
+		// Vite will handle creating the frame for us with proper line numbers, no need to create one
+
+		return new CSSError({
+			errorCode: AstroErrorCodes.CssSyntaxError,
+			message: err.reason,
+			location: {
+				file: filename,
+				line: errorLine,
+				column: err.column,
+			},
+		});
+	}
+
+	// Some CSS processor will return a line and a column, so let's try to show a pretty error
+	if (err.line && err.column) {
+		const errorLine = positionAt(styleTagBeginning, fileContent).line + (err.line ?? 0);
+
+		return new CSSError({
+			errorCode: AstroErrorCodes.CssUnknownError,
+			message: err.message,
+			location: {
+				file: filename,
+				line: errorLine,
+				column: err.column,
+			},
+			frame: err.frame,
+		});
+	}
+
+	// For other errors we'll just point to the beginning of the style tag
+	const errorPosition = positionAt(styleTagBeginning, fileContent);
+	errorPosition.line += 1;
+
+	return new CSSError({
+		errorCode: AstroErrorCodes.CssUnknownError,
+		message: err.message,
+		location: {
+			file: filename,
+			line: errorPosition.line,
+			column: 0,
+		},
+		frame: err.frame,
+	});
 }

packages/astro/src/vite-plugin-astro/index.ts

@@ -2,7 +2,6 @@ import type { PluginContext, SourceDescription } from 'rollup';
 import type * as vite from 'vite';
 import type { AstroSettings } from '../@types/astro';
 import type { LogOptions } from '../core/logger/core.js';
-import type { ViteStyleTransformer } from '../vite-style-transform';
 import type { PluginMetadata as AstroPluginMetadata } from './types';
 
 import ancestor from 'common-ancestor-path';
@@ -13,10 +12,6 @@ import { cachedCompilation, CompileProps, getCachedSource } from '../core/compil
 import { isRelativePath, prependForwardSlash, startsWithForwardSlash } from '../core/path.js';
 import { viteID } from '../core/util.js';
 import { getFileInfo } from '../vite-plugin-utils/index.js';
-import {
-	createTransformStyles,
-	createViteStyleTransformer,
-} from '../vite-style-transform/index.js';
 import { handleHotUpdate } from './hmr.js';
 import { parseAstroRequest, ParsedRequestResult } from './query.js';
 
@@ -44,8 +39,6 @@ export default function astro({ settings, logging }: AstroPluginOptions): vite.P
 	}
 
 	let resolvedConfig: vite.ResolvedConfig;
-	let styleTransformer: ViteStyleTransformer;
-	let viteDevServer: vite.ViteDevServer | undefined;
 
 	// Variables for determining if an id starts with /src...
 	const srcRootWeb = config.srcDir.pathname.slice(config.root.pathname.length - 1);
@@ -68,11 +61,6 @@ export default function astro({ settings, logging }: AstroPluginOptions): vite.P
 		enforce: 'pre', // run transforms before other plugins can
 		configResolved(_resolvedConfig) {
 			resolvedConfig = _resolvedConfig;
-			styleTransformer = createViteStyleTransformer(_resolvedConfig);
-		},
-		configureServer(server) {
-			viteDevServer = server;
-			styleTransformer.viteDevServer = server;
 		},
 		// note: don’t claim .astro files with resolveId() — it prevents Vite from transpiling the final JS (import.meta.glob, etc.)
 		async resolveId(id, from, opts) {
@@ -125,10 +113,10 @@ export default function astro({ settings, logging }: AstroPluginOptions): vite.P
 			}
 
 			const compileProps: CompileProps = {
-				config,
+				astroConfig: config,
+				viteConfig: resolvedConfig,
 				filename,
 				source,
-				transformStyle: createTransformStyles(styleTransformer, filename, Boolean(opts?.ssr), this),
 			};
 
 			switch (query.type) {
@@ -220,10 +208,10 @@ export default function astro({ settings, logging }: AstroPluginOptions): vite.P
 
 			const filename = normalizeFilename(parsedId.filename);
 			const compileProps: CompileProps = {
-				config,
+				astroConfig: config,
+				viteConfig: resolvedConfig,
 				filename,
 				source,
-				transformStyle: createTransformStyles(styleTransformer, filename, Boolean(opts?.ssr), this),
 			};
 
 			try {
@@ -342,10 +330,10 @@ ${source}
 		async handleHotUpdate(context) {
 			if (context.server.config.isProduction) return;
 			const compileProps: CompileProps = {
-				config,
+				astroConfig: config,
+				viteConfig: resolvedConfig,
 				filename: context.file,
 				source: await context.read(),
-				transformStyle: createTransformStyles(styleTransformer, context.file, true),
 			};
 			const compile = () => cachedCompilation(compileProps);
 			return handleHotUpdate(context, {

packages/astro/src/vite-plugin-markdown-legacy/index.ts

@@ -4,7 +4,7 @@ import esbuild from 'esbuild';
 import fs from 'fs';
 import matter from 'gray-matter';
 import { fileURLToPath } from 'url';
-import type { Plugin, ViteDevServer } from 'vite';
+import type { Plugin, ResolvedConfig } from 'vite';
 import type { AstroSettings } from '../@types/astro';
 import { pagesVirtualModuleId } from '../core/app/index.js';
 import { cachedCompilation, CompileProps } from '../core/compile/index.js';
@@ -13,11 +13,6 @@ import type { LogOptions } from '../core/logger/core.js';
 import { isMarkdownFile } from '../core/util.js';
 import type { PluginMetadata as AstroPluginMetadata } from '../vite-plugin-astro/types';
 import { getFileInfo } from '../vite-plugin-utils/index.js';
-import {
-	createTransformStyles,
-	createViteStyleTransformer,
-	ViteStyleTransformer,
-} from '../vite-style-transform/index.js';
 
 interface AstroPluginOptions {
 	settings: AstroSettings;
@@ -86,18 +81,11 @@ export default function markdown({ settings }: AstroPluginOptions): Plugin {
 		return false;
 	}
 
-	let styleTransformer: ViteStyleTransformer;
+	let resolvedConfig: ResolvedConfig;
-	let viteDevServer: ViteDevServer | undefined;
 
 	return {
 		name: 'astro:markdown',
 		enforce: 'pre',
-		configResolved(_resolvedConfig) {
-			styleTransformer = createViteStyleTransformer(_resolvedConfig);
-		},
-		configureServer(server) {
-			styleTransformer.viteDevServer = server;
-		},
 		async resolveId(id, importer, options) {
 			// Resolve any .md (or alternative extensions of markdown files like .markdown) files with the `?content` cache buster. This should only come from
 			// an already-resolved JS module wrapper. Needed to prevent infinite loops in Vite.
@@ -226,15 +214,10 @@ ${setup}`.trim();
 
 				// Transform from `.astro` to valid `.ts`
 				const compileProps: CompileProps = {
-					config,
+					astroConfig: config,
+					viteConfig: resolvedConfig,
 					filename,
 					source: astroResult,
-					transformStyle: createTransformStyles(
-						styleTransformer,
-						filename,
-						Boolean(opts?.ssr),
-						this
-					),
 				};
 
 				let transformResult = await cachedCompilation(compileProps);

packages/astro/src/vite-style-transform/index.ts

@@ -1,2 +0,0 @@
-export type { ViteStyleTransformer } from './style-transform';
-export { createTransformStyles, createViteStyleTransformer } from './style-transform.js';

packages/astro/src/vite-style-transform/style-transform.ts

@@ -1,106 +0,0 @@
-import type { PluginContext } from 'rollup';
-import { fileURLToPath } from 'url';
-import type { TransformStyle } from '../core/compile/index';
-import { createTransformStyleWithViteFn, TransformStyleWithVite } from './transform-with-vite.js';
-
-import { readFileSync } from 'fs';
-import type * as vite from 'vite';
-import { AstroErrorCodes } from '../core/errors/codes.js';
-import { CSSError } from '../core/errors/errors.js';
-import { positionAt } from '../core/errors/index.js';
-
-export type ViteStyleTransformer = {
-	viteDevServer?: vite.ViteDevServer;
-	transformStyleWithVite: TransformStyleWithVite;
-};
-
-export function createViteStyleTransformer(viteConfig: vite.ResolvedConfig): ViteStyleTransformer {
-	return {
-		transformStyleWithVite: createTransformStyleWithViteFn(viteConfig),
-	};
-}
-
-function getNormalizedIDForPostCSS(filename: string): string {
-	try {
-		const filenameURL = new URL(`file://${filename}`);
-		return fileURLToPath(filenameURL);
-	} catch (err) {
-		// Not a real file, so just use the provided filename as the normalized id
-		return filename;
-	}
-}
-
-export function createTransformStyles(
-	viteStyleTransformer: ViteStyleTransformer,
-	filename: string,
-	ssr: boolean,
-	pluginContext?: PluginContext
-): TransformStyle {
-	const normalizedID = getNormalizedIDForPostCSS(filename);
-
-	return async function (styleSource, lang) {
-		let result: any;
-		try {
-			result = await viteStyleTransformer.transformStyleWithVite.call(pluginContext, {
-				id: normalizedID,
-				source: styleSource,
-				lang,
-				ssr,
-				viteDevServer: viteStyleTransformer.viteDevServer,
-			});
-		} catch (err: any) {
-			const fileContent = readFileSync(filename).toString();
-			const styleTagBeginning = fileContent.indexOf(err.input?.source ?? err.code);
-
-			// PostCSS Syntax Error
-			if (err.name === 'CssSyntaxError') {
-				const errorLine = positionAt(styleTagBeginning, fileContent).line + (err.line ?? 0);
-
-				// Vite will handle creating the frame for us with proper line numbers, no need to create one
-
-				throw new CSSError({
-					errorCode: AstroErrorCodes.CssSyntaxError,
-					message: err.reason,
-					location: {
-						file: filename,
-						line: errorLine,
-						column: err.column,
-					},
-				});
-			}
-
-			// Some CSS processor will return a line and a column, so let's try to show a pretty error
-			if (err.line && err.column) {
-				const errorLine = positionAt(styleTagBeginning, fileContent).line + (err.line ?? 0);
-
-				throw new CSSError({
-					errorCode: AstroErrorCodes.CssUnknownError,
-					message: err.message,
-					location: {
-						file: filename,
-						line: errorLine,
-						column: err.column,
-					},
-					frame: err.frame,
-				});
-			}
-
-			// For other errors we'll just point to the beginning of the style tag
-			const errorPosition = positionAt(styleTagBeginning, fileContent);
-			errorPosition.line += 1;
-
-			throw new CSSError({
-				errorCode: AstroErrorCodes.CssUnknownError,
-				message: err.message,
-				location: {
-					file: filename,
-					line: errorPosition.line,
-					column: 0,
-				},
-				frame: err.frame,
-			});
-		}
-
-		return result;
-	};
-}

packages/astro/src/vite-style-transform/transform-with-vite.ts

@@ -1,72 +0,0 @@
-import type { PluginContext } from 'rollup';
-import type * as vite from 'vite';
-
-import { STYLE_EXTENSIONS } from '../core/render/util.js';
-
-export type TransformHook = (
-	code: string,
-	id: string,
-	ssr?: boolean
-) => Promise<vite.TransformResult>;
-
-interface TransformStyleWithViteOptions {
-	id: string;
-	source: string;
-	lang: string;
-	ssr?: boolean;
-	viteDevServer?: vite.ViteDevServer;
-}
-
-export interface TransformStyleWithVite {
-	(options: TransformStyleWithViteOptions): Promise<{
-		code: string;
-		map: vite.TransformResult['map'];
-		deps: Set<string>;
-	} | null>;
-}
-
-export function createTransformStyleWithViteFn(
-	viteConfig: vite.ResolvedConfig
-): TransformStyleWithVite {
-	const viteCSSPlugin = viteConfig.plugins.find(({ name }) => name === 'vite:css');
-	if (!viteCSSPlugin) throw new Error(`vite:css plugin couldn't be found`);
-	if (!viteCSSPlugin.transform) throw new Error(`vite:css has no transform() hook`);
-	const transformCss = viteCSSPlugin.transform as TransformHook;
-
-	return async function (
-		this: PluginContext,
-		{ id, source, lang, ssr, viteDevServer }: TransformStyleWithViteOptions
-	) {
-		if (!STYLE_EXTENSIONS.has(lang)) {
-			return null; // only preprocess langs supported by Vite
-		}
-
-		// Id must end with valid CSS extension for vite:css to process
-		const styleId = `${id}?astro&type=style&lang${lang}`;
-
-		viteDevServer?.moduleGraph.ensureEntryFromUrl(styleId, ssr, false);
-
-		// This function could be called in a custom Vite hook like `handleHotUpdate`
-		// which doesn't have a context
-		const ctx = this ?? { addWatchFile: () => {} };
-		const transformResult = await transformCss.call(ctx, source, styleId, ssr);
-
-		// NOTE: only `code` and `map` are returned by vite:css
-		const { code, map } = transformResult;
-		const deps = new Set<string>();
-
-		// Get deps from module created while transforming the styleId by Vite.
-		// In build, it's fine that we skip this as it's used by HMR only.
-		const mod = viteDevServer?.moduleGraph.getModuleById(styleId);
-		if (mod) {
-			// Get all @import references
-			for (const imported of mod.importedModules) {
-				if (imported.file) {
-					deps.add(imported.file);
-				}
-			}
-		}
-
-		return { code, map, deps };
-	};
-}

packages/astro/test/units/compile/invalid-css.test.js

@@ -1,3 +1,4 @@
+import { resolveConfig } from 'vite';
 import { expect } from 'chai';
 import { cachedCompilation } from '../../../dist/core/compile/index.js';
 import { AggregateError } from '../../../dist/core/errors/index.js';
@@ -8,11 +9,11 @@ describe('astro/src/core/compile', () => {
 			let error;
 			try {
 				let r = await cachedCompilation({
-					config: /** @type {any} */ ({
+					astroConfig: /** @type {any} */ ({
 						root: '/',
 					}),
+					viteConfig: await resolveConfig({ configFile: false }, 'serve'),
 					filename: '/src/pages/index.astro',
-					moduleId: '/src/pages/index.astro',
 					source: `
 	---
 	---
@@ -27,16 +28,13 @@ describe('astro/src/core/compile', () => {
 		}
 	</style>
 	`,
-					transformStyle(source, lang) {
-						throw new Error('Invalid css');
-					},
 				});
 			} catch (err) {
 				error = err;
 			}
 
 			expect(error).to.be.an.instanceOf(AggregateError);
-			expect(error.errors[0].message).to.contain('Invalid css');
+			expect(error.errors[0].message).to.contain('expected ")"');
 		});
 	});
 });