Validate Astro frontmatter JS/TS on compiler error (#2115)

* validate the astro component frontmatter ahead of compilation * add test, update existing tests * chore(lint): Prettier fix * Update index.ts * remove macos skip lines, no longer needed Co-authored-by: GitHub Action <[email protected]>
withastro · Dec 7, 2021 · 0ef682c · 0ef682c
1 parent 317c62a
commit 0ef682c
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 110 deletions.
diff --git a/.changeset/dirty-guests-wash.md b/.changeset/dirty-guests-wash.md
@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Improve error message on bad JS/TS frontmatter
diff --git a/packages/astro/src/vite-plugin-astro/index.ts b/packages/astro/src/vite-plugin-astro/index.ts
@@ -10,6 +10,7 @@ import { transform } from '@astrojs/compiler';
 import { AstroDevServer } from '../core/dev/index.js';
 import { getViteTransform, TransformHook, transformWithVite } from './styles.js';
 
+const FRONTMATTER_PARSE_REGEXP = /^\-\-\-(.*)^\-\-\-/ms;
 interface AstroPluginOptions {
   config: AstroConfig;
   devServer?: AstroDevServer;
@@ -87,14 +88,37 @@ export default function astro({ config, devServer }: AstroPluginOptions): vite.P
         // throw CSS transform errors here if encountered
         if (cssTransformError) throw cssTransformError;
 
-        // Compile `.ts` to `.js`
+        // Compile all TypeScript to JavaScript.
+        // Also, catches invalid JS/TS in the compiled output before returning.
         const { code, map } = await esbuild.transform(tsResult.code, { loader: 'ts', sourcemap: 'external', sourcefile: id });
 
         return {
           code,
           map,
         };
       } catch (err: any) {
+        // Verify frontmatter: a common reason that this plugin fails is that
+        // the user provided invalid JS/TS in the component frontmatter.
+        // If the frontmatter is invalid, the `err` object may be a compiler
+        // panic or some other vague/confusing compiled error message.
+        //
+        // Before throwing, it is better to verify the frontmatter here, and
+        // let esbuild throw a more specific exception if the code is invalid.
+        // If frontmatter is valid or cannot be parsed, then continue.
+        const scannedFrontmatter = FRONTMATTER_PARSE_REGEXP.exec(source);
+        if (scannedFrontmatter) {
+          try {
+            await esbuild.transform(scannedFrontmatter[1], { loader: 'ts', sourcemap: false, sourcefile: id });
+          } catch (frontmatterErr: any) {
+            // Improve the error by replacing the phrase "unexpected end of file"
+            // with "unexpected end of frontmatter" in the esbuild error message.
+            if (frontmatterErr && frontmatterErr.message) {
+              frontmatterErr.message = frontmatterErr.message.replace('end of file', 'end of frontmatter');
+            }
+            throw frontmatterErr;
+          }
+        }
+
         // improve compiler errors
         if (err.stack.includes('wasm-function')) {
           const search = new URLSearchParams({

diff --git a/packages/astro/test/errors.test.js b/packages/astro/test/errors.test.js
@@ -1,10 +1,6 @@
 import { expect } from 'chai';
-import os from 'os';
 import { loadFixture } from './test-utils.js';
 
-// TODO: fix these tests on macOS
-const isMacOS = os.platform() === 'darwin';
-
 let fixture;
 let devServer;
 
@@ -21,231 +17,136 @@ before(async () => {
 
 describe('Error display', () => {
   describe('Astro', () => {
-    // This test is redundant w/ runtime error since it no longer produces an Astro syntax error
-    it.skip('syntax error', async () => {
-      if (isMacOS) return;
-
+    it('syntax error in template', async () => {
       const res = await fixture.fetch('/astro-syntax-error');
-
-      // 500 returned
       expect(res.status).to.equal(500);
+      const body = await res.text();
+      console.log(res.body);
+      expect(body).to.include('Unexpected &quot;}&quot;');
+    });
 
-      // error message includes "unrecoverable error"
+    it('syntax error in frontmatter', async () => {
+      const res = await fixture.fetch('/astro-frontmatter-syntax-error');
+      expect(res.status).to.equal(500);
       const body = await res.text();
       console.log(res.body);
-      expect(body).to.include('unrecoverable error');
+      expect(body).to.include('Unexpected end of frontmatter');
     });
 
     it('runtime error', async () => {
-      if (isMacOS) return;
-
       const res = await fixture.fetch('/astro-runtime-error');
-
-      // 500 returned
       expect(res.status).to.equal(500);
-
-      // error message contains error
       const body = await res.text();
       expect(body).to.include('ReferenceError: title is not defined');
-
-      // TODO: improve stacktrace
+      // TODO: improve and test stacktrace
     });
 
     it('hydration error', async () => {
-      if (isMacOS) return;
-
       const res = await fixture.fetch('/astro-hydration-error');
-
-      // 500 returned
       expect(res.status).to.equal(500);
-
-      // error message contains error
       const body = await res.text();
-
-      // error message contains error
       expect(body).to.include('Error: invalid hydration directive');
     });
 
     it('client:media error', async () => {
-      if (isMacOS) return;
-
       const res = await fixture.fetch('/astro-client-media-error');
-
-      // 500 returned
       expect(res.status).to.equal(500);
-
-      // error message contains error
       const body = await res.text();
-
-      // error message contains error
       expect(body).to.include('Error: Media query must be provided');
     });
   });
 
   describe('JS', () => {
     it('syntax error', async () => {
-      if (isMacOS) return;
-
       const res = await fixture.fetch('/js-syntax-error');
-
-      // 500 returnd
       expect(res.status).to.equal(500);
-
-      // error message is helpful
       const body = await res.text();
       expect(body).to.include('Parse failure');
     });
 
     it('runtime error', async () => {
-      if (isMacOS) return;
-
       const res = await fixture.fetch('/js-runtime-error');
-
-      // 500 returnd
       expect(res.status).to.equal(500);
-
-      // error message is helpful
       const body = await res.text();
       expect(body).to.include('ReferenceError: undefinedvar is not defined');
     });
   });
 
   describe('Preact', () => {
     it('syntax error', async () => {
-      if (isMacOS) return;
-
       const res = await fixture.fetch('/preact-syntax-error');
-
-      // 500 returned
       expect(res.status).to.equal(500);
-
-      // error message is helpful
       const body = await res.text();
       expect(body).to.include('Syntax error');
     });
 
     it('runtime error', async () => {
-      if (isMacOS) return;
-
       const res = await fixture.fetch('/preact-runtime-error');
-
-      // 500 returned
       expect(res.status).to.equal(500);
-
-      // error message is helpful
       const body = await res.text();
       expect(body).to.include('Error: PreactRuntimeError');
     });
   });
 
   describe('React', () => {
     it('syntax error', async () => {
-      if (isMacOS) return;
-
       const res = await fixture.fetch('/react-syntax-error');
-
-      // 500 returned
       expect(res.status).to.equal(500);
-
-      // error message is helpful
       const body = await res.text();
       expect(body).to.include('Syntax error');
     });
 
     it('runtime error', async () => {
-      if (isMacOS) return;
-
       const res = await fixture.fetch('/react-runtime-error');
-
-      // 500 returned
       expect(res.status).to.equal(500);
-
-      // error message is helpful
       const body = await res.text();
       expect(body).to.include('Error: ReactRuntimeError');
     });
   });
 
   describe('Solid', () => {
     it('syntax error', async () => {
-      if (isMacOS) return;
-
       const res = await fixture.fetch('/solid-syntax-error');
-
-      // 500 returned
       expect(res.status).to.equal(500);
-
-      // error message is helpful
       const body = await res.text();
       expect(body).to.include('Syntax error');
     });
 
     it('runtime error', async () => {
-      if (isMacOS) return;
-
       const res = await fixture.fetch('/solid-runtime-error');
-
-      // 500 returned
       expect(res.status).to.equal(500);
-
-      // error message is helpful
       const body = await res.text();
       expect(body).to.include('Error: SolidRuntimeError');
     });
   });
 
   describe('Svelte', () => {
     it('syntax error', async () => {
-      if (isMacOS) return;
-
       const res = await fixture.fetch('/svelte-syntax-error');
-
-      // 500 returned
       expect(res.status).to.equal(500);
-
-      // error message is helpful
       const body = await res.text();
       expect(body).to.include('ParseError');
     });
 
     it('runtime error', async () => {
-      if (isMacOS) return;
-
       const res = await fixture.fetch('/svelte-runtime-error');
-
-      // 500 returned
       expect(res.status).to.equal(500);
-
-      // error message is helpful
       const body = await res.text();
       expect(body).to.include('Error: SvelteRuntimeError');
     });
   });
 
   describe('Vue', () => {
     it('syntax error', async () => {
-      if (isMacOS) return;
-
       const res = await fixture.fetch('/vue-syntax-error');
-
       const body = await res.text();
-
-      // 500 returned
       expect(res.status).to.equal(500);
-
-      // error message is helpful
       expect(body).to.include('Parse failure');
     });
 
     it('runtime error', async () => {
-      if (isMacOS) return;
-
       const res = await fixture.fetch('/vue-runtime-error');
-
-      // 500 returned
       expect(res.status).to.equal(500);
-
-      // error message is helpful
       const body = await res.text();
       expect(body).to.match(/Cannot read.*undefined/); // note: error differs slightly between Node versions
     });

diff --git a/packages/astro/test/fixtures/errors/src/pages/astro-frontmatter-syntax-error.astro b/packages/astro/test/fixtures/errors/src/pages/astro-frontmatter-syntax-error.astro
@@ -0,0 +1,4 @@
+---
+{
+---
+<h1>Testing bad JS in frontmatter</h1>