Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent locking up when encountering invalid CSS #4675

Merged
merged 9 commits into from
Sep 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/eleven-goats-confess.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Prevent locking up when encountering invalid CSS
5 changes: 3 additions & 2 deletions packages/astro/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
@@ -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<string, CompileResult>;
type CompileResult = TransformResult & {
Expand All @@ -23,40 +21,18 @@ 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({
config,
filename,
moduleId,
source,
ssr,
transformStyleWithVite,
viteDevServer,
pluginContext,
transformStyle,
}: CompileProps): Promise<CompileResult> {
const normalizedID = getNormalizedID(filename);
let cssDeps = new Set<string>();
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
Expand All @@ -69,58 +45,33 @@ 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<string, string>) => {
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
err.code = err.code || AstroErrorCodes.UnknownCompilerError;
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, {
Expand Down
13 changes: 13 additions & 0 deletions packages/astro/src/core/compile/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
export type {
CompileProps
} from './compile';
export type {
TransformStyle
} from './types';

export {
cachedCompilation,
invalidateCompilation,
isCached,
getCachedSource
} from './compile.js';
39 changes: 39 additions & 0 deletions packages/astro/src/core/compile/style.ts
Original file line number Diff line number Diff line change
@@ -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<string>, errors: Error[]): PreprocessStyle {
const preprocessStyle: PreprocessStyle = async (value: string, attrs: Record<string, string>) => {
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;
};
9 changes: 9 additions & 0 deletions packages/astro/src/core/compile/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import type { SourceMap } from 'rollup';

export type TransformStyleResult = null | {
code: string;
map: SourceMap | null;
deps: Set<string>;
}

export type TransformStyle = (source: string, lang: string) => TransformStyleResult | Promise<TransformStyleResult>;
8 changes: 8 additions & 0 deletions packages/astro/src/core/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<any> = [];
constructor( errors: Iterable<any>, message?: string | undefined) {
super(message);
this.errors = Array.from(errors);
}
}
2 changes: 1 addition & 1 deletion packages/astro/src/vite-plugin-astro/hmr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
29 changes: 13 additions & 16 deletions packages/astro/src/vite-plugin-astro/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,22 @@ 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';
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 {
Expand All @@ -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...
Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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, {
Expand Down
19 changes: 8 additions & 11 deletions packages/astro/src/vite-plugin-markdown-legacy/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
7 changes: 7 additions & 0 deletions packages/astro/src/vite-style-transform/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export type {
ViteStyleTransformer
} from './style-transform';
export {
createViteStyleTransformer,
createTransformStyles
} from './style-transform.js';
Loading