diff --git a/.changeset/eleven-goats-confess.md b/.changeset/eleven-goats-confess.md new file mode 100644 index 000000000..a65c2c783 --- /dev/null +++ b/.changeset/eleven-goats-confess.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Prevent locking up when encountering invalid CSS diff --git a/packages/astro/package.json b/packages/astro/package.json index 5428882c4..56349a1ad 100644 --- a/packages/astro/package.json +++ b/packages/astro/package.json @@ -88,13 +88,14 @@ "dev": "astro-scripts dev --prebuild \"src/runtime/server/astro-island.ts\" --prebuild \"src/runtime/client/{idle,load,media,only,visible}.ts\" \"src/**/*.ts\"", "postbuild": "astro-scripts copy \"src/**/*.astro\"", "benchmark": "node test/benchmark/dev.bench.js && node test/benchmark/build.bench.js", - "test": "mocha --exit --timeout 20000 --ignore **/lit-element.test.js && mocha --timeout 20000 **/lit-element.test.js", + "test:unit": "mocha --exit --timeout 2000 ./test/units/**/*.test.js", + "test": "pnpm run test:unit && mocha --exit --timeout 20000 --ignore **/lit-element.test.js && mocha --timeout 20000 **/lit-element.test.js", "test:match": "mocha --timeout 20000 -g", "test:e2e": "playwright test", "test:e2e:match": "playwright test -g" }, "dependencies": { - "@astrojs/compiler": "^0.23.5", + "@astrojs/compiler": "^0.24.0", "@astrojs/language-server": "^0.23.0", "@astrojs/markdown-remark": "^1.1.1", "@astrojs/telemetry": "^1.0.0", diff --git a/packages/astro/src/vite-plugin-astro/compile.ts b/packages/astro/src/core/compile/compile.ts similarity index 53% rename from packages/astro/src/vite-plugin-astro/compile.ts rename to packages/astro/src/core/compile/compile.ts index 368e310fc..d1c5c2f4b 100644 --- a/packages/astro/src/vite-plugin-astro/compile.ts +++ b/packages/astro/src/core/compile/compile.ts @@ -1,14 +1,12 @@ import type { TransformResult } from '@astrojs/compiler'; -import type { PluginContext, SourceMapInput } from 'rollup'; -import type { ViteDevServer } from 'vite'; -import type { AstroConfig } from '../@types/astro'; -import type { TransformStyleWithVite } from './styles'; +import type { AstroConfig } from '../../@types/astro'; +import type { TransformStyle } from './types'; import { transform } from '@astrojs/compiler'; -import { fileURLToPath } from 'url'; -import { AstroErrorCodes } from '../core/errors.js'; -import { prependForwardSlash } from '../core/path.js'; -import { viteID } from '../core/util.js'; +import { AstroErrorCodes } from '../errors.js'; +import { prependForwardSlash } from '../path.js'; +import { viteID, AggregateError } from '../util.js'; +import { createStylePreprocessor } from './style.js'; type CompilationCache = Map; type CompileResult = TransformResult & { @@ -23,20 +21,7 @@ export interface CompileProps { filename: string; moduleId: string; source: string; - ssr: boolean; - transformStyleWithVite: TransformStyleWithVite; - viteDevServer?: ViteDevServer; - pluginContext: PluginContext; -} - -function getNormalizedID(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; - } + transformStyle: TransformStyle; } async function compile({ @@ -44,19 +29,10 @@ async function compile({ filename, moduleId, source, - ssr, - transformStyleWithVite, - viteDevServer, - pluginContext, + transformStyle, }: CompileProps): Promise { - const normalizedID = getNormalizedID(filename); let cssDeps = new Set(); - let cssTransformError: Error | undefined; - - // handleHotUpdate doesn't have `addWatchFile` used by transformStyleWithVite. - if (!pluginContext.addWatchFile) { - pluginContext.addWatchFile = () => {}; - } + let cssTransformErrors: Error[] = []; // Transform from `.astro` to valid `.ts` // use `sourcemap: "both"` so that sourcemap is included in the code @@ -69,44 +45,11 @@ async function compile({ sourcefile: filename, sourcemap: 'both', internalURL: `/@fs${prependForwardSlash( - viteID(new URL('../runtime/server/index.js', import.meta.url)) + viteID(new URL('../../runtime/server/index.js', import.meta.url)) )}`, // TODO: baseline flag experimentalStaticExtraction: true, - preprocessStyle: async (value: string, attrs: Record) => { - const lang = `.${attrs?.lang || 'css'}`.toLowerCase(); - - try { - const result = await transformStyleWithVite.call(pluginContext, { - id: normalizedID, - source: value, - lang, - ssr, - viteDevServer, - }); - - if (!result) return null as any; // TODO: add type in compiler to fix "any" - - for (const dep of result.deps) { - cssDeps.add(dep); - } - - let map: SourceMapInput | undefined; - if (result.map) { - if (typeof result.map === 'string') { - map = result.map; - } else if (result.map.mappings) { - map = result.map.toString(); - } - } - - return { code: result.code, map }; - } catch (err) { - // save error to throw in plugin context - cssTransformError = err as any; - return null; - } - }, + preprocessStyle: createStylePreprocessor(transformStyle, cssDeps, cssTransformErrors), }) .catch((err) => { // throw compiler errors here if encountered @@ -114,13 +57,21 @@ async function compile({ throw err; }) .then((result) => { - // throw CSS transform errors here if encountered - if (cssTransformError) { - (cssTransformError as any).code = - (cssTransformError as any).code || AstroErrorCodes.UnknownCompilerCSSError; - throw cssTransformError; + switch(cssTransformErrors.length) { + case 0: return result; + case 1: { + let error = cssTransformErrors[0]; + if(!(error as any).code) { + (error as any).code = AstroErrorCodes.UnknownCompilerCSSError; + } + throw cssTransformErrors[0]; + } + default: { + const aggregateError = new AggregateError(cssTransformErrors); + (aggregateError as any).code = AstroErrorCodes.UnknownCompilerCSSError; + throw aggregateError; + } } - return result; }); const compileResult: CompileResult = Object.create(transformResult, { diff --git a/packages/astro/src/core/compile/index.ts b/packages/astro/src/core/compile/index.ts new file mode 100644 index 000000000..7d3539c2e --- /dev/null +++ b/packages/astro/src/core/compile/index.ts @@ -0,0 +1,13 @@ +export type { + CompileProps +} from './compile'; +export type { + TransformStyle +} from './types'; + +export { + cachedCompilation, + invalidateCompilation, + isCached, + getCachedSource +} from './compile.js'; diff --git a/packages/astro/src/core/compile/style.ts b/packages/astro/src/core/compile/style.ts new file mode 100644 index 000000000..fde1e60bd --- /dev/null +++ b/packages/astro/src/core/compile/style.ts @@ -0,0 +1,39 @@ +import type { TransformOptions } from '@astrojs/compiler'; +import type { TransformStyle } from './types'; +import type { SourceMapInput } from 'rollup'; + +type PreprocessStyle = TransformOptions['preprocessStyle']; + +export function createStylePreprocessor(transformStyle: TransformStyle, cssDeps: Set, errors: Error[]): PreprocessStyle { + const preprocessStyle: PreprocessStyle = async (value: string, attrs: Record) => { + const lang = `.${attrs?.lang || 'css'}`.toLowerCase(); + + try { + const result = await transformStyle(value, lang); + + if (!result) return null as any; // TODO: add type in compiler to fix "any" + + for (const dep of result.deps) { + cssDeps.add(dep); + } + + let map: SourceMapInput | undefined; + if (result.map) { + if (typeof result.map === 'string') { + map = result.map; + } else if (result.map.mappings) { + map = result.map.toString(); + } + } + + return { code: result.code, map }; + } catch (err) { + errors.push(err as unknown as Error); + return { + error: err + '' + }; + } + }; + + return preprocessStyle; +}; diff --git a/packages/astro/src/core/compile/types.ts b/packages/astro/src/core/compile/types.ts new file mode 100644 index 000000000..0e7f79fb2 --- /dev/null +++ b/packages/astro/src/core/compile/types.ts @@ -0,0 +1,9 @@ +import type { SourceMap } from 'rollup'; + +export type TransformStyleResult = null | { + code: string; + map: SourceMap | null; + deps: Set; +} + +export type TransformStyle = (source: string, lang: string) => TransformStyleResult | Promise; diff --git a/packages/astro/src/core/util.ts b/packages/astro/src/core/util.ts index 3c74b5580..3dc5a698e 100644 --- a/packages/astro/src/core/util.ts +++ b/packages/astro/src/core/util.ts @@ -226,3 +226,11 @@ export async function resolveIdToUrl(viteServer: ViteDevServer, id: string) { } return VALID_ID_PREFIX + result.id; } + +export const AggregateError = typeof globalThis.AggregateError !== 'undefined' ? globalThis.AggregateError : class extends Error { + errors: Array = []; + constructor( errors: Iterable, message?: string | undefined) { + super(message); + this.errors = Array.from(errors); + } +} diff --git a/packages/astro/src/vite-plugin-astro/hmr.ts b/packages/astro/src/vite-plugin-astro/hmr.ts index 6ee123e07..9c729715d 100644 --- a/packages/astro/src/vite-plugin-astro/hmr.ts +++ b/packages/astro/src/vite-plugin-astro/hmr.ts @@ -4,7 +4,7 @@ import type { AstroConfig } from '../@types/astro'; import type { LogOptions } from '../core/logger/core.js'; import { info } from '../core/logger/core.js'; import * as msg from '../core/messages.js'; -import { cachedCompilation, invalidateCompilation, isCached } from './compile.js'; +import { cachedCompilation, invalidateCompilation, isCached } from '../core/compile/index.js'; import { isAstroScript } from './query.js'; const PKG_PREFIX = new URL('../../', import.meta.url); diff --git a/packages/astro/src/vite-plugin-astro/index.ts b/packages/astro/src/vite-plugin-astro/index.ts index ccfba7af7..f8b98a6a4 100644 --- a/packages/astro/src/vite-plugin-astro/index.ts +++ b/packages/astro/src/vite-plugin-astro/index.ts @@ -3,6 +3,7 @@ import type * as vite from 'vite'; import type { AstroConfig } from '../@types/astro'; import type { LogOptions } from '../core/logger/core.js'; import type { PluginMetadata as AstroPluginMetadata } from './types'; +import type { ViteStyleTransformer } from '../vite-style-transform'; import ancestor from 'common-ancestor-path'; import esbuild from 'esbuild'; @@ -10,10 +11,14 @@ import slash from 'slash'; import { fileURLToPath } from 'url'; import { isRelativePath, startsWithForwardSlash } from '../core/path.js'; import { getFileInfo } from '../vite-plugin-utils/index.js'; -import { cachedCompilation, CompileProps, getCachedSource } from './compile.js'; +import { + cachedCompilation, + CompileProps, + getCachedSource +} from '../core/compile/index.js'; import { handleHotUpdate } from './hmr.js'; import { parseAstroRequest, ParsedRequestResult } from './query.js'; -import { createTransformStyleWithViteFn, TransformStyleWithVite } from './styles.js'; +import { createViteStyleTransformer, createTransformStyles } from '../vite-style-transform/index.js'; const FRONTMATTER_PARSE_REGEXP = /^\-\-\-(.*)^\-\-\-/ms; interface AstroPluginOptions { @@ -38,7 +43,7 @@ export default function astro({ config, logging }: AstroPluginOptions): vite.Plu } let resolvedConfig: vite.ResolvedConfig; - let transformStyleWithVite: TransformStyleWithVite; + let styleTransformer: ViteStyleTransformer; let viteDevServer: vite.ViteDevServer | undefined; // Variables for determining if an id starts with /src... @@ -60,10 +65,11 @@ export default function astro({ config, logging }: AstroPluginOptions): vite.Plu enforce: 'pre', // run transforms before other plugins can configResolved(_resolvedConfig) { resolvedConfig = _resolvedConfig; - transformStyleWithVite = createTransformStyleWithViteFn(_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) { @@ -117,10 +123,7 @@ export default function astro({ config, logging }: AstroPluginOptions): vite.Plu filename, moduleId: id, source, - ssr: Boolean(opts?.ssr), - transformStyleWithVite, - viteDevServer, - pluginContext: this, + transformStyle: createTransformStyles(styleTransformer, filename, Boolean(opts?.ssr), this), }; switch (query.type) { @@ -216,10 +219,7 @@ export default function astro({ config, logging }: AstroPluginOptions): vite.Plu filename, moduleId: id, source, - ssr: Boolean(opts?.ssr), - transformStyleWithVite, - viteDevServer, - pluginContext: this, + transformStyle: createTransformStyles(styleTransformer, filename, Boolean(opts?.ssr), this), }; try { @@ -352,10 +352,7 @@ ${source} filename: context.file, moduleId: context.file, source: await context.read(), - ssr: true, - transformStyleWithVite, - viteDevServer, - pluginContext: this, + transformStyle: createTransformStyles(styleTransformer, context.file, true, this), }; const compile = () => cachedCompilation(compileProps); return handleHotUpdate.call(this, context, { diff --git a/packages/astro/src/vite-plugin-markdown-legacy/index.ts b/packages/astro/src/vite-plugin-markdown-legacy/index.ts index 1bfb8546b..cd26a7ba6 100644 --- a/packages/astro/src/vite-plugin-markdown-legacy/index.ts +++ b/packages/astro/src/vite-plugin-markdown-legacy/index.ts @@ -9,11 +9,8 @@ import type { AstroConfig } from '../@types/astro'; import { pagesVirtualModuleId } from '../core/app/index.js'; import { collectErrorMetadata } from '../core/errors.js'; import type { LogOptions } from '../core/logger/core.js'; -import { cachedCompilation, CompileProps } from '../vite-plugin-astro/compile.js'; -import { - createTransformStyleWithViteFn, - TransformStyleWithVite, -} from '../vite-plugin-astro/styles.js'; +import { cachedCompilation, CompileProps } from '../core/compile/index.js'; +import { ViteStyleTransformer, createViteStyleTransformer, createTransformStyles } from '../vite-style-transform/index.js'; import type { PluginMetadata as AstroPluginMetadata } from '../vite-plugin-astro/types'; import { getFileInfo } from '../vite-plugin-utils/index.js'; @@ -64,14 +61,17 @@ export default function markdown({ config, logging }: AstroPluginOptions): Plugi return false; } - let transformStyleWithVite: TransformStyleWithVite; + let styleTransformer: ViteStyleTransformer; let viteDevServer: ViteDevServer | undefined; return { name: 'astro:markdown', enforce: 'pre', configResolved(_resolvedConfig) { - transformStyleWithVite = createTransformStyleWithViteFn(_resolvedConfig); + styleTransformer = createViteStyleTransformer(_resolvedConfig); + }, + configureServer(server) { + styleTransformer.viteDevServer = server; }, async resolveId(id, importer, options) { // Resolve any .md files with the `?content` cache buster. This should only come from @@ -208,10 +208,7 @@ ${setup}`.trim(); filename, moduleId: id, source: astroResult, - ssr: Boolean(opts?.ssr), - transformStyleWithVite, - viteDevServer, - pluginContext: this, + 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 new file mode 100644 index 000000000..f3635cba7 --- /dev/null +++ b/packages/astro/src/vite-style-transform/index.ts @@ -0,0 +1,7 @@ +export type { + ViteStyleTransformer +} from './style-transform'; +export { + createViteStyleTransformer, + createTransformStyles +} 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 new file mode 100644 index 000000000..aebec2440 --- /dev/null +++ b/packages/astro/src/vite-style-transform/style-transform.ts @@ -0,0 +1,49 @@ +import type { PluginContext } from 'rollup'; +import type { TransformStyle } from '../core/compile/index'; +import { TransformStyleWithVite, createTransformStyleWithViteFn } from './transform-with-vite.js'; +import { fileURLToPath } from 'url'; + +import type * as vite from 'vite'; + +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 { + // handleHotUpdate doesn't have `addWatchFile` used by transformStyleWithVite. + // TODO, refactor, why is this happening *here* ? + if (!pluginContext.addWatchFile) { + pluginContext.addWatchFile = () => {}; + } + + const normalizedID = getNormalizedIDForPostCSS(filename); + + return async function(styleSource, lang) { + const result = await viteStyleTransformer.transformStyleWithVite.call(pluginContext, { + id: normalizedID, + source: styleSource, + lang, + ssr, + viteDevServer: viteStyleTransformer.viteDevServer, + }); + + return result; + }; +} diff --git a/packages/astro/src/vite-plugin-astro/styles.ts b/packages/astro/src/vite-style-transform/transform-with-vite.ts similarity index 100% rename from packages/astro/src/vite-plugin-astro/styles.ts rename to packages/astro/src/vite-style-transform/transform-with-vite.ts diff --git a/packages/astro/test/units/compile/invalid-css.test.js b/packages/astro/test/units/compile/invalid-css.test.js new file mode 100644 index 000000000..72309c445 --- /dev/null +++ b/packages/astro/test/units/compile/invalid-css.test.js @@ -0,0 +1,43 @@ + +import { expect } from 'chai'; +import { cachedCompilation } from '../../../dist/core/compile/index.js'; +import { AggregateError } from '../../../dist/core/util.js'; + +describe('astro/src/core/compile', () => { + describe('Invalid CSS', () => { + it('throws an aggregate error with the errors', async () => { + let error; + try { + let r = await cachedCompilation({ + config: /** @type {any} */({ + root: '/' + }), + filename: '/src/pages/index.astro', + moduleId: '/src/pages/index.astro', + source: ` + --- + --- + + + `, + 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'); + }); + }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index e67cd1546..c3cca8ddd 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -338,7 +338,7 @@ importers: packages/astro: specifiers: - '@astrojs/compiler': ^0.23.5 + '@astrojs/compiler': ^0.24.0 '@astrojs/language-server': ^0.23.0 '@astrojs/markdown-remark': ^1.1.1 '@astrojs/telemetry': ^1.0.0 @@ -421,7 +421,7 @@ importers: yargs-parser: ^21.0.1 zod: ^3.17.3 dependencies: - '@astrojs/compiler': 0.23.5 + '@astrojs/compiler': 0.24.0 '@astrojs/language-server': 0.23.3 '@astrojs/markdown-remark': link:../markdown/remark '@astrojs/telemetry': link:../telemetry @@ -3232,6 +3232,10 @@ packages: resolution: {integrity: sha512-vBMPy9ok4iLapSyCCT1qsZ9dK7LkVFl9mObtLEmWiec9myGHS9h2kQY2xzPeFNJiWXUf9O6tSyQpQTy5As/p3g==} dev: false + /@astrojs/compiler/0.24.0: + resolution: {integrity: sha512-xZ81C/oMfExdF18I1Tyd2BKKzBqO+qYYctSy4iCwH4UWSo/4Y8A8MAzV1hG67uuE7hFRourSl6H5KUbhyChv/A==} + dev: false + /@astrojs/language-server/0.23.3: resolution: {integrity: sha512-ROoMKo37NZ76pE/A2xHfjDlgfsNnFmkhL4+Wifs0L855n73SUCbnXz7ZaQktIGAq2Te2TpSjAawiOx0q9L5qeg==} hasBin: true