From c491d1f423cc8ed7ba25d7d0dea6336ad9659a55 Mon Sep 17 00:00:00 2001 From: Drew Powers <1369770+drwpow@users.noreply.github.com> Date: Mon, 29 Nov 2021 14:44:55 -0700 Subject: [PATCH] Fix Sass WASM crashes (#2049) Partially addresses #2032 --- .changeset/honest-knives-bake.md | 5 +++ packages/astro/src/vite-plugin-astro/index.ts | 34 +++++++++++-------- .../test/fixtures/sass/src/pages/error.astro | 7 ++++ packages/astro/test/sass.test.js | 23 +++++++++++++ 4 files changed, 54 insertions(+), 15 deletions(-) create mode 100644 .changeset/honest-knives-bake.md create mode 100644 packages/astro/test/fixtures/sass/src/pages/error.astro create mode 100644 packages/astro/test/sass.test.js diff --git a/.changeset/honest-knives-bake.md b/.changeset/honest-knives-bake.md new file mode 100644 index 000000000..62292b928 --- /dev/null +++ b/.changeset/honest-knives-bake.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Bugfix: Sass compile errors cause compiler panic diff --git a/packages/astro/src/vite-plugin-astro/index.ts b/packages/astro/src/vite-plugin-astro/index.ts index ba4fb2cd9..6f6ebef78 100644 --- a/packages/astro/src/vite-plugin-astro/index.ts +++ b/packages/astro/src/vite-plugin-astro/index.ts @@ -6,7 +6,6 @@ import type { AstroConfig } from '../@types/astro'; import esbuild from 'esbuild'; import fs from 'fs'; import { fileURLToPath } from 'url'; -import os from 'os'; import { transform } from '@astrojs/compiler'; import { AstroDevServer } from '../core/dev/index.js'; import { getViteTransform, TransformHook, transformWithVite } from './styles.js'; @@ -32,13 +31,11 @@ function isSSR(options: undefined | boolean | { ssr: boolean }): boolean { /** Transform .astro files for Vite */ export default function astro({ config, devServer }: AstroPluginOptions): vite.Plugin { - let platform: NodeJS.Platform; let viteTransform: TransformHook; return { name: '@astrojs/vite-plugin-astro', enforce: 'pre', // run transforms before other plugins can configResolved(resolvedConfig) { - platform = os.platform(); // TODO: remove macOS hack viteTransform = getViteTransform(resolvedConfig); }, // note: don’t claim .astro files with resolveId() — it prevents Vite from transpiling the final JS (import.meta.globEager, etc.) @@ -52,6 +49,7 @@ export default function astro({ config, devServer }: AstroPluginOptions): vite.P const isPage = normalizedID.startsWith(fileURLToPath(config.pages)) || normalizedID.startsWith(fileURLToPath(config.layouts)); let source = await fs.promises.readFile(id, 'utf8'); let tsResult: TransformResult | undefined; + let cssTransformError: Error | undefined; try { // Transform from `.astro` to valid `.ts` @@ -65,23 +63,29 @@ export default function astro({ config, devServer }: AstroPluginOptions): vite.P internalURL: 'astro/internal', preprocessStyle: async (value: string, attrs: Record) => { const lang = `.${attrs?.lang || 'css'}`.toLowerCase(); - const result = await transformWithVite({ value, lang, id, transformHook: viteTransform, ssr: isSSR(opts) }); - if (!result) { - // TODO: compiler supports `null`, but types don't yet - return result as any; - } - let map: SourceMapInput | undefined; - if (result.map) { - if (typeof result.map === 'string') { - map = result.map; - } else if (result.map.mappings) { - map = result.map.toString(); + try { + const result = await transformWithVite({ value, lang, id, transformHook: viteTransform, ssr: isSSR(opts) }); + let map: SourceMapInput | undefined; + if (!result) return null as any; // TODO: add type in compiler to fix "any" + 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; } - return { code: result.code, map }; }, }); + // throw CSS transform errors here if encountered + if (cssTransformError) throw cssTransformError; + // Compile `.ts` to `.js` const { code, map } = await esbuild.transform(tsResult.code, { loader: 'ts', sourcemap: 'external', sourcefile: id }); diff --git a/packages/astro/test/fixtures/sass/src/pages/error.astro b/packages/astro/test/fixtures/sass/src/pages/error.astro new file mode 100644 index 000000000..dbdaf8c56 --- /dev/null +++ b/packages/astro/test/fixtures/sass/src/pages/error.astro @@ -0,0 +1,7 @@ + diff --git a/packages/astro/test/sass.test.js b/packages/astro/test/sass.test.js new file mode 100644 index 000000000..78eb1ecad --- /dev/null +++ b/packages/astro/test/sass.test.js @@ -0,0 +1,23 @@ +import { expect } from 'chai'; +import { loadFixture } from './test-utils.js'; + +// note: many Sass tests live in 0-css.test.js to test within context of a framework. +// these tests are independent of framework. +describe('Sass', () => { + let fixture; + let devServer; + + before(async () => { + fixture = await loadFixture({ projectRoot: './fixtures/sass/' }); + devServer = await fixture.startDevServer(); + }); + + after(async () => { + devServer && (await devServer.stop()); + }); + + it('shows helpful error on failure', async () => { + const res = await fixture.fetch('/error').then((res) => res.text()); + expect(res).to.include('Undefined variable'); + }); +});