diff --git a/.changeset/tall-keys-hunt.md b/.changeset/tall-keys-hunt.md new file mode 100644 index 000000000..1f5e3a6ea --- /dev/null +++ b/.changeset/tall-keys-hunt.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Refactor CSS preprocessing handling diff --git a/packages/astro/src/core/compile/compile.ts b/packages/astro/src/core/compile/compile.ts index f3fe01f0c..0b5a8f30c 100644 --- a/packages/astro/src/core/compile/compile.ts +++ b/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(); 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 { let cssDeps = new Set(); 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(), - site: config.site?.toString(), + projectRoot: astroConfig.root.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 { - 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)!; diff --git a/packages/astro/src/core/compile/style.ts b/packages/astro/src/core/compile/style.ts index 20079cede..779aa4ec9 100644 --- a/packages/astro/src/core/compile/style.ts +++ b/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 type { TransformStyle } from './types'; +import { preprocessCSS, ResolvedConfig } from 'vite'; +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( - transformStyle: TransformStyle, - cssDeps: Set, - errors: Error[] -): PreprocessStyle { - const preprocessStyle: PreprocessStyle = async (value: string, attrs: Record) => { +export function createStylePreprocessor({ + filename, + viteConfig, + cssDeps, + cssTransformErrors, +}: { + filename: string; + viteConfig: ResolvedConfig; + cssDeps: Set; + 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" - - for (const dep of result.deps) { + result.deps?.forEach((dep) => { 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) { - errors.push(err as unknown as Error); - return { - error: err + '', - }; + } catch (err: any) { + try { + err = enhanceCSSError(err, filename); + } 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, + }); } diff --git a/packages/astro/src/vite-plugin-astro/index.ts b/packages/astro/src/vite-plugin-astro/index.ts index 5c300b9e3..40f89be7c 100644 --- a/packages/astro/src/vite-plugin-astro/index.ts +++ b/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, { diff --git a/packages/astro/src/vite-plugin-markdown-legacy/index.ts b/packages/astro/src/vite-plugin-markdown-legacy/index.ts index e016d46a0..16b6e3e10 100644 --- a/packages/astro/src/vite-plugin-markdown-legacy/index.ts +++ b/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 viteDevServer: ViteDevServer | undefined; + let resolvedConfig: ResolvedConfig; 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); diff --git a/packages/astro/src/vite-style-transform/index.ts b/packages/astro/src/vite-style-transform/index.ts deleted file mode 100644 index 8e03caca6..000000000 --- a/packages/astro/src/vite-style-transform/index.ts +++ /dev/null @@ -1,2 +0,0 @@ -export type { ViteStyleTransformer } from './style-transform'; -export { createTransformStyles, createViteStyleTransformer } from './style-transform.js'; diff --git a/packages/astro/src/vite-style-transform/style-transform.ts b/packages/astro/src/vite-style-transform/style-transform.ts deleted file mode 100644 index c8dfbd3f0..000000000 --- a/packages/astro/src/vite-style-transform/style-transform.ts +++ /dev/null @@ -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; - }; -} diff --git a/packages/astro/src/vite-style-transform/transform-with-vite.ts b/packages/astro/src/vite-style-transform/transform-with-vite.ts deleted file mode 100644 index 71276dd0d..000000000 --- a/packages/astro/src/vite-style-transform/transform-with-vite.ts +++ /dev/null @@ -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; - -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; - } | 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(); - - // 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 }; - }; -} diff --git a/packages/astro/test/units/compile/invalid-css.test.js b/packages/astro/test/units/compile/invalid-css.test.js index 0ea6f77a1..1aa501070 100644 --- a/packages/astro/test/units/compile/invalid-css.test.js +++ b/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', () => { } `, - 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 ")"'); }); }); });