From 4412fe61f496b83343af57a8932d857ad0c27ac3 Mon Sep 17 00:00:00 2001 From: Nate Moore Date: Tue, 19 Jul 2022 16:47:31 -0500 Subject: [PATCH] Improve error handling (#3859) * feat: tag JSX exports with correct renderer * feat(error): enhance generic errors with frame * feat(error): surface errors from streaming response * feat(error): use vite overlay to display errors * chore: fix build issues * feat: use custom logger to hide vite errors for known packages * chore: move error-react-spectrum to e2e test * chore: add todo comment * test: fix error overlay handling * refactor: extract overlay message to util * test(e2e): update shared component tests * fix: give error overlay more time * refactor: move errors tests to e2e * fix: appease ts * test: move sass error to e2e tests * fix: scope optimizeDeps to `src/pages/**/*` * chore: update lockfile * chore: update test script * chore: log error overlay * chore: log error tests * chore: update playwright config * test(e2e): update errors tests * test(e2e): fix overlay util * test(e2e): fix test utils * test(e2e): try timeout * test(e2e): give up on overlay tests * fix: typo * fix: typo * refactor: collapse definition * fix: let errors throw * chore: revert scanner change * chore: refactor err.plugin handling * chore: add clarifying comments * fix: make astro:renderer non enumerable * chore: update comments * refactor: replace astro:renderer string with Symbol * chore: add comment about tagged components * feat: improve error overlay when hint exists Co-authored-by: Nate Moore --- .../astro/e2e/error-react-spectrum.test.js | 24 ++++ packages/astro/e2e/error-sass.test.js | 24 ++++ packages/astro/e2e/errors.test.js | 67 +++++++++++ .../error-react-spectrum/astro.config.mjs | 0 .../error-react-spectrum/package.json | 2 +- .../src/pages/index.astro | 0 .../fixtures/error-sass}/package.json | 2 +- .../error-sass/src/pages/index.astro} | 0 .../fixtures/errors/astro.config.mjs | 0 .../fixtures/errors/package.json | 2 +- .../errors/src/components/JSRuntimeError.js | 0 .../errors/src/components/JSSyntaxError.js | 0 .../src/components/PreactRuntimeError.jsx | 0 .../src/components/PreactSyntaxError.jsx | 0 .../src/components/ReactRuntimeError.jsx | 0 .../src/components/ReactSyntaxError.jsx | 0 .../src/components/SolidRuntimeError.jsx | 0 .../src/components/SolidSyntaxError.jsx | 0 .../components/SvelteDirectiveError.svelte | 0 .../src/components/SvelteRuntimeError.svelte | 0 .../src/components/SvelteSyntaxError.svelte | 0 .../errors/src/components/VueRuntimeError.vue | 0 .../errors/src/components/VueSyntaxError.vue | 0 .../src/pages/astro-client-media-error.astro | 0 .../astro-frontmatter-syntax-error.astro | 0 .../src/pages/astro-hydration-error.astro | 0 .../src/pages/astro-runtime-error.astro | 0 .../errors/src/pages/astro-syntax-error.astro | 0 .../errors/src/pages/import-not-found.astro | 0 .../errors/src/pages/js-runtime-error.astro | 0 .../errors/src/pages/js-syntax-error.astro | 0 .../src/pages/preact-runtime-error.astro | 0 .../src/pages/preact-syntax-error.astro | 0 .../src/pages/react-runtime-error.astro | 0 .../errors/src/pages/react-syntax-error.astro | 0 .../src/pages/solid-runtime-error.astro | 0 .../errors/src/pages/solid-syntax-error.astro | 0 .../src/pages/svelte-runtime-error.astro | 0 .../src/pages/svelte-syntax-error.astro | 0 .../errors/src/pages/vue-runtime-error.astro | 0 .../errors/src/pages/vue-syntax-error.astro | 0 .../preact-component/src/pages/mdx.mdx | 2 +- .../react-component/src/pages/mdx.mdx | 1 + .../solid-component/src/pages/mdx.mdx | 1 + .../svelte-component/src/pages/mdx.mdx | 1 + .../fixtures/vue-component/src/pages/mdx.mdx | 1 + .../astro/e2e/preact-compat-component.test.js | 9 +- packages/astro/e2e/preact-component.test.js | 11 +- packages/astro/e2e/react-component.test.js | 11 +- packages/astro/e2e/shared-component-tests.js | 2 +- packages/astro/e2e/solid-component.test.js | 11 +- packages/astro/e2e/svelte-component.test.js | 15 ++- packages/astro/e2e/test-utils.js | 10 +- packages/astro/e2e/vue-component.test.js | 15 ++- packages/astro/package.json | 2 +- packages/astro/playwright.config.js | 5 +- packages/astro/src/core/create-vite.ts | 5 +- packages/astro/src/core/errors.ts | 109 +++++++++++++++--- packages/astro/src/core/util.ts | 6 +- packages/astro/src/runtime/server/index.ts | 72 ++++++++---- packages/astro/src/template/5xx.ts | 80 ------------- .../src/vite-plugin-astro-server/index.ts | 32 +++-- packages/astro/src/vite-plugin-jsx/index.ts | 3 +- packages/astro/src/vite-plugin-jsx/tag.ts | 64 ++++++++++ .../astro/test/error-react-spectrum.test.js | 29 ----- packages/astro/test/errors.test.js | 100 ---------------- packages/astro/test/sass.test.js | 26 ----- pnpm-lock.yaml | 84 +++++++------- 68 files changed, 453 insertions(+), 375 deletions(-) create mode 100644 packages/astro/e2e/error-react-spectrum.test.js create mode 100644 packages/astro/e2e/error-sass.test.js create mode 100644 packages/astro/e2e/errors.test.js rename packages/astro/{test => e2e}/fixtures/error-react-spectrum/astro.config.mjs (100%) rename packages/astro/{test => e2e}/fixtures/error-react-spectrum/package.json (84%) rename packages/astro/{test => e2e}/fixtures/error-react-spectrum/src/pages/index.astro (100%) rename packages/astro/{test/fixtures/sass => e2e/fixtures/error-sass}/package.json (80%) rename packages/astro/{test/fixtures/sass/src/pages/error.astro => e2e/fixtures/error-sass/src/pages/index.astro} (100%) rename packages/astro/{test => e2e}/fixtures/errors/astro.config.mjs (100%) rename packages/astro/{test => e2e}/fixtures/errors/package.json (92%) rename packages/astro/{test => e2e}/fixtures/errors/src/components/JSRuntimeError.js (100%) rename packages/astro/{test => e2e}/fixtures/errors/src/components/JSSyntaxError.js (100%) rename packages/astro/{test => e2e}/fixtures/errors/src/components/PreactRuntimeError.jsx (100%) rename packages/astro/{test => e2e}/fixtures/errors/src/components/PreactSyntaxError.jsx (100%) rename packages/astro/{test => e2e}/fixtures/errors/src/components/ReactRuntimeError.jsx (100%) rename packages/astro/{test => e2e}/fixtures/errors/src/components/ReactSyntaxError.jsx (100%) rename packages/astro/{test => e2e}/fixtures/errors/src/components/SolidRuntimeError.jsx (100%) rename packages/astro/{test => e2e}/fixtures/errors/src/components/SolidSyntaxError.jsx (100%) rename packages/astro/{test => e2e}/fixtures/errors/src/components/SvelteDirectiveError.svelte (100%) rename packages/astro/{test => e2e}/fixtures/errors/src/components/SvelteRuntimeError.svelte (100%) rename packages/astro/{test => e2e}/fixtures/errors/src/components/SvelteSyntaxError.svelte (100%) rename packages/astro/{test => e2e}/fixtures/errors/src/components/VueRuntimeError.vue (100%) rename packages/astro/{test => e2e}/fixtures/errors/src/components/VueSyntaxError.vue (100%) rename packages/astro/{test => e2e}/fixtures/errors/src/pages/astro-client-media-error.astro (100%) rename packages/astro/{test => e2e}/fixtures/errors/src/pages/astro-frontmatter-syntax-error.astro (100%) rename packages/astro/{test => e2e}/fixtures/errors/src/pages/astro-hydration-error.astro (100%) rename packages/astro/{test => e2e}/fixtures/errors/src/pages/astro-runtime-error.astro (100%) rename packages/astro/{test => e2e}/fixtures/errors/src/pages/astro-syntax-error.astro (100%) rename packages/astro/{test => e2e}/fixtures/errors/src/pages/import-not-found.astro (100%) rename packages/astro/{test => e2e}/fixtures/errors/src/pages/js-runtime-error.astro (100%) rename packages/astro/{test => e2e}/fixtures/errors/src/pages/js-syntax-error.astro (100%) rename packages/astro/{test => e2e}/fixtures/errors/src/pages/preact-runtime-error.astro (100%) rename packages/astro/{test => e2e}/fixtures/errors/src/pages/preact-syntax-error.astro (100%) rename packages/astro/{test => e2e}/fixtures/errors/src/pages/react-runtime-error.astro (100%) rename packages/astro/{test => e2e}/fixtures/errors/src/pages/react-syntax-error.astro (100%) rename packages/astro/{test => e2e}/fixtures/errors/src/pages/solid-runtime-error.astro (100%) rename packages/astro/{test => e2e}/fixtures/errors/src/pages/solid-syntax-error.astro (100%) rename packages/astro/{test => e2e}/fixtures/errors/src/pages/svelte-runtime-error.astro (100%) rename packages/astro/{test => e2e}/fixtures/errors/src/pages/svelte-syntax-error.astro (100%) rename packages/astro/{test => e2e}/fixtures/errors/src/pages/vue-runtime-error.astro (100%) rename packages/astro/{test => e2e}/fixtures/errors/src/pages/vue-syntax-error.astro (100%) delete mode 100644 packages/astro/src/template/5xx.ts create mode 100644 packages/astro/src/vite-plugin-jsx/tag.ts delete mode 100644 packages/astro/test/error-react-spectrum.test.js delete mode 100644 packages/astro/test/errors.test.js delete mode 100644 packages/astro/test/sass.test.js diff --git a/packages/astro/e2e/error-react-spectrum.test.js b/packages/astro/e2e/error-react-spectrum.test.js new file mode 100644 index 000000000..c8515cc42 --- /dev/null +++ b/packages/astro/e2e/error-react-spectrum.test.js @@ -0,0 +1,24 @@ +import { expect } from '@playwright/test'; +import { testFactory, getErrorOverlayMessage } from './test-utils.js'; + +const test = testFactory({ root: './fixtures/error-react-spectrum/' }); + +let devServer; + +test.beforeEach(async ({ astro }) => { + devServer = await astro.startDevServer(); +}); + +test.afterEach(async ({ astro }) => { + await devServer.stop(); + astro.resetAllFiles(); +}); + +test.describe('Error: React Spectrum', () => { + test('overlay', async ({ page, astro }) => { + await page.goto(astro.resolveUrl('/')); + + const message = await getErrorOverlayMessage(page); + expect(message).toMatch('@adobe/react-spectrum is not compatible') + }); +}); diff --git a/packages/astro/e2e/error-sass.test.js b/packages/astro/e2e/error-sass.test.js new file mode 100644 index 000000000..638d5e337 --- /dev/null +++ b/packages/astro/e2e/error-sass.test.js @@ -0,0 +1,24 @@ +import { expect } from '@playwright/test'; +import { testFactory, getErrorOverlayMessage } from './test-utils.js'; + +const test = testFactory({ root: './fixtures/error-sass/' }); + +let devServer; + +test.beforeEach(async ({ astro }) => { + devServer = await astro.startDevServer(); +}); + +test.afterEach(async ({ astro }) => { + await devServer.stop(); + astro.resetAllFiles(); +}); + +test.describe('Error: Sass', () => { + test('overlay', async ({ page, astro }) => { + await page.goto(astro.resolveUrl('/')); + + const message = await getErrorOverlayMessage(page); + expect(message).toMatch('Undefined variable') + }); +}); diff --git a/packages/astro/e2e/errors.test.js b/packages/astro/e2e/errors.test.js new file mode 100644 index 000000000..e917ce7ba --- /dev/null +++ b/packages/astro/e2e/errors.test.js @@ -0,0 +1,67 @@ +import { expect } from '@playwright/test'; +import { testFactory, getErrorOverlayMessage } from './test-utils.js'; + +const test = testFactory({ root: './fixtures/errors/' }); + +let devServer; + +test.beforeEach(async ({ astro }) => { + devServer = await astro.startDevServer(); +}); + +test.afterEach(async ({ astro }) => { + await devServer.stop(); + astro.resetAllFiles(); +}); + +test.describe('Error display', () => { + test('detect syntax errors in template', async ({ page, astro }) => { + await page.goto(astro.resolveUrl('/astro-syntax-error')); + + const message = await getErrorOverlayMessage(page); + expect(message).toMatch('Unexpected "}"') + + await Promise.all([ + // Wait for page reload + page.waitForNavigation(), + // Edit the component file + await astro.editFile('./src/pages/astro-syntax-error.astro', () => `

No syntax error

`) + ]); + + expect(await page.locator('vite-error-overlay').count()).toEqual(0); + }); + + test('shows useful error when frontmatter import is not found', async ({ page, astro }) => { + await page.goto(astro.resolveUrl('/import-not-found')); + + const message = await getErrorOverlayMessage(page); + expect(message).toMatch('failed to load module for ssr: ../abc.astro') + + await Promise.all([ + // Wait for page reload + page.waitForNavigation(), + // Edit the component file + astro.editFile('./src/pages/import-not-found.astro', () => `

No import error

`) + ]); + + expect(await page.locator('vite-error-overlay').count()).toEqual(0); + }); + + test('framework errors recover when fixed', async ({ page, astro }) => { + await page.goto(astro.resolveUrl('/svelte-syntax-error')); + + const message = await getErrorOverlayMessage(page); + expect(message).toMatch(' attempted to close an element that was not open') + + await Promise.all([ + // Wait for page reload + page.waitForNavigation(), + // Edit the component file + astro.editFile('./src/components/SvelteSyntaxError.svelte', () => `

No mismatch

`) + ]); + + expect(await page.locator('vite-error-overlay').count()).toEqual(0); + }); + + +}); diff --git a/packages/astro/test/fixtures/error-react-spectrum/astro.config.mjs b/packages/astro/e2e/fixtures/error-react-spectrum/astro.config.mjs similarity index 100% rename from packages/astro/test/fixtures/error-react-spectrum/astro.config.mjs rename to packages/astro/e2e/fixtures/error-react-spectrum/astro.config.mjs diff --git a/packages/astro/test/fixtures/error-react-spectrum/package.json b/packages/astro/e2e/fixtures/error-react-spectrum/package.json similarity index 84% rename from packages/astro/test/fixtures/error-react-spectrum/package.json rename to packages/astro/e2e/fixtures/error-react-spectrum/package.json index e06316aa9..f52e71585 100644 --- a/packages/astro/test/fixtures/error-react-spectrum/package.json +++ b/packages/astro/e2e/fixtures/error-react-spectrum/package.json @@ -1,5 +1,5 @@ { - "name": "@test/error-react-spectrum", + "name": "@e2e/error-react-spectrum", "version": "0.0.0", "private": true, "dependencies": { diff --git a/packages/astro/test/fixtures/error-react-spectrum/src/pages/index.astro b/packages/astro/e2e/fixtures/error-react-spectrum/src/pages/index.astro similarity index 100% rename from packages/astro/test/fixtures/error-react-spectrum/src/pages/index.astro rename to packages/astro/e2e/fixtures/error-react-spectrum/src/pages/index.astro diff --git a/packages/astro/test/fixtures/sass/package.json b/packages/astro/e2e/fixtures/error-sass/package.json similarity index 80% rename from packages/astro/test/fixtures/sass/package.json rename to packages/astro/e2e/fixtures/error-sass/package.json index 62074943c..f52820ad2 100644 --- a/packages/astro/test/fixtures/sass/package.json +++ b/packages/astro/e2e/fixtures/error-sass/package.json @@ -1,5 +1,5 @@ { - "name": "@test/sass", + "name": "@e2e/error-sass", "version": "0.0.0", "private": true, "dependencies": { diff --git a/packages/astro/test/fixtures/sass/src/pages/error.astro b/packages/astro/e2e/fixtures/error-sass/src/pages/index.astro similarity index 100% rename from packages/astro/test/fixtures/sass/src/pages/error.astro rename to packages/astro/e2e/fixtures/error-sass/src/pages/index.astro diff --git a/packages/astro/test/fixtures/errors/astro.config.mjs b/packages/astro/e2e/fixtures/errors/astro.config.mjs similarity index 100% rename from packages/astro/test/fixtures/errors/astro.config.mjs rename to packages/astro/e2e/fixtures/errors/astro.config.mjs diff --git a/packages/astro/test/fixtures/errors/package.json b/packages/astro/e2e/fixtures/errors/package.json similarity index 92% rename from packages/astro/test/fixtures/errors/package.json rename to packages/astro/e2e/fixtures/errors/package.json index 3438ca92a..1821e074f 100644 --- a/packages/astro/test/fixtures/errors/package.json +++ b/packages/astro/e2e/fixtures/errors/package.json @@ -1,5 +1,5 @@ { - "name": "@test/errors", + "name": "@e2e/errors", "version": "0.0.0", "private": true, "dependencies": { diff --git a/packages/astro/test/fixtures/errors/src/components/JSRuntimeError.js b/packages/astro/e2e/fixtures/errors/src/components/JSRuntimeError.js similarity index 100% rename from packages/astro/test/fixtures/errors/src/components/JSRuntimeError.js rename to packages/astro/e2e/fixtures/errors/src/components/JSRuntimeError.js diff --git a/packages/astro/test/fixtures/errors/src/components/JSSyntaxError.js b/packages/astro/e2e/fixtures/errors/src/components/JSSyntaxError.js similarity index 100% rename from packages/astro/test/fixtures/errors/src/components/JSSyntaxError.js rename to packages/astro/e2e/fixtures/errors/src/components/JSSyntaxError.js diff --git a/packages/astro/test/fixtures/errors/src/components/PreactRuntimeError.jsx b/packages/astro/e2e/fixtures/errors/src/components/PreactRuntimeError.jsx similarity index 100% rename from packages/astro/test/fixtures/errors/src/components/PreactRuntimeError.jsx rename to packages/astro/e2e/fixtures/errors/src/components/PreactRuntimeError.jsx diff --git a/packages/astro/test/fixtures/errors/src/components/PreactSyntaxError.jsx b/packages/astro/e2e/fixtures/errors/src/components/PreactSyntaxError.jsx similarity index 100% rename from packages/astro/test/fixtures/errors/src/components/PreactSyntaxError.jsx rename to packages/astro/e2e/fixtures/errors/src/components/PreactSyntaxError.jsx diff --git a/packages/astro/test/fixtures/errors/src/components/ReactRuntimeError.jsx b/packages/astro/e2e/fixtures/errors/src/components/ReactRuntimeError.jsx similarity index 100% rename from packages/astro/test/fixtures/errors/src/components/ReactRuntimeError.jsx rename to packages/astro/e2e/fixtures/errors/src/components/ReactRuntimeError.jsx diff --git a/packages/astro/test/fixtures/errors/src/components/ReactSyntaxError.jsx b/packages/astro/e2e/fixtures/errors/src/components/ReactSyntaxError.jsx similarity index 100% rename from packages/astro/test/fixtures/errors/src/components/ReactSyntaxError.jsx rename to packages/astro/e2e/fixtures/errors/src/components/ReactSyntaxError.jsx diff --git a/packages/astro/test/fixtures/errors/src/components/SolidRuntimeError.jsx b/packages/astro/e2e/fixtures/errors/src/components/SolidRuntimeError.jsx similarity index 100% rename from packages/astro/test/fixtures/errors/src/components/SolidRuntimeError.jsx rename to packages/astro/e2e/fixtures/errors/src/components/SolidRuntimeError.jsx diff --git a/packages/astro/test/fixtures/errors/src/components/SolidSyntaxError.jsx b/packages/astro/e2e/fixtures/errors/src/components/SolidSyntaxError.jsx similarity index 100% rename from packages/astro/test/fixtures/errors/src/components/SolidSyntaxError.jsx rename to packages/astro/e2e/fixtures/errors/src/components/SolidSyntaxError.jsx diff --git a/packages/astro/test/fixtures/errors/src/components/SvelteDirectiveError.svelte b/packages/astro/e2e/fixtures/errors/src/components/SvelteDirectiveError.svelte similarity index 100% rename from packages/astro/test/fixtures/errors/src/components/SvelteDirectiveError.svelte rename to packages/astro/e2e/fixtures/errors/src/components/SvelteDirectiveError.svelte diff --git a/packages/astro/test/fixtures/errors/src/components/SvelteRuntimeError.svelte b/packages/astro/e2e/fixtures/errors/src/components/SvelteRuntimeError.svelte similarity index 100% rename from packages/astro/test/fixtures/errors/src/components/SvelteRuntimeError.svelte rename to packages/astro/e2e/fixtures/errors/src/components/SvelteRuntimeError.svelte diff --git a/packages/astro/test/fixtures/errors/src/components/SvelteSyntaxError.svelte b/packages/astro/e2e/fixtures/errors/src/components/SvelteSyntaxError.svelte similarity index 100% rename from packages/astro/test/fixtures/errors/src/components/SvelteSyntaxError.svelte rename to packages/astro/e2e/fixtures/errors/src/components/SvelteSyntaxError.svelte diff --git a/packages/astro/test/fixtures/errors/src/components/VueRuntimeError.vue b/packages/astro/e2e/fixtures/errors/src/components/VueRuntimeError.vue similarity index 100% rename from packages/astro/test/fixtures/errors/src/components/VueRuntimeError.vue rename to packages/astro/e2e/fixtures/errors/src/components/VueRuntimeError.vue diff --git a/packages/astro/test/fixtures/errors/src/components/VueSyntaxError.vue b/packages/astro/e2e/fixtures/errors/src/components/VueSyntaxError.vue similarity index 100% rename from packages/astro/test/fixtures/errors/src/components/VueSyntaxError.vue rename to packages/astro/e2e/fixtures/errors/src/components/VueSyntaxError.vue diff --git a/packages/astro/test/fixtures/errors/src/pages/astro-client-media-error.astro b/packages/astro/e2e/fixtures/errors/src/pages/astro-client-media-error.astro similarity index 100% rename from packages/astro/test/fixtures/errors/src/pages/astro-client-media-error.astro rename to packages/astro/e2e/fixtures/errors/src/pages/astro-client-media-error.astro diff --git a/packages/astro/test/fixtures/errors/src/pages/astro-frontmatter-syntax-error.astro b/packages/astro/e2e/fixtures/errors/src/pages/astro-frontmatter-syntax-error.astro similarity index 100% rename from packages/astro/test/fixtures/errors/src/pages/astro-frontmatter-syntax-error.astro rename to packages/astro/e2e/fixtures/errors/src/pages/astro-frontmatter-syntax-error.astro diff --git a/packages/astro/test/fixtures/errors/src/pages/astro-hydration-error.astro b/packages/astro/e2e/fixtures/errors/src/pages/astro-hydration-error.astro similarity index 100% rename from packages/astro/test/fixtures/errors/src/pages/astro-hydration-error.astro rename to packages/astro/e2e/fixtures/errors/src/pages/astro-hydration-error.astro diff --git a/packages/astro/test/fixtures/errors/src/pages/astro-runtime-error.astro b/packages/astro/e2e/fixtures/errors/src/pages/astro-runtime-error.astro similarity index 100% rename from packages/astro/test/fixtures/errors/src/pages/astro-runtime-error.astro rename to packages/astro/e2e/fixtures/errors/src/pages/astro-runtime-error.astro diff --git a/packages/astro/test/fixtures/errors/src/pages/astro-syntax-error.astro b/packages/astro/e2e/fixtures/errors/src/pages/astro-syntax-error.astro similarity index 100% rename from packages/astro/test/fixtures/errors/src/pages/astro-syntax-error.astro rename to packages/astro/e2e/fixtures/errors/src/pages/astro-syntax-error.astro diff --git a/packages/astro/test/fixtures/errors/src/pages/import-not-found.astro b/packages/astro/e2e/fixtures/errors/src/pages/import-not-found.astro similarity index 100% rename from packages/astro/test/fixtures/errors/src/pages/import-not-found.astro rename to packages/astro/e2e/fixtures/errors/src/pages/import-not-found.astro diff --git a/packages/astro/test/fixtures/errors/src/pages/js-runtime-error.astro b/packages/astro/e2e/fixtures/errors/src/pages/js-runtime-error.astro similarity index 100% rename from packages/astro/test/fixtures/errors/src/pages/js-runtime-error.astro rename to packages/astro/e2e/fixtures/errors/src/pages/js-runtime-error.astro diff --git a/packages/astro/test/fixtures/errors/src/pages/js-syntax-error.astro b/packages/astro/e2e/fixtures/errors/src/pages/js-syntax-error.astro similarity index 100% rename from packages/astro/test/fixtures/errors/src/pages/js-syntax-error.astro rename to packages/astro/e2e/fixtures/errors/src/pages/js-syntax-error.astro diff --git a/packages/astro/test/fixtures/errors/src/pages/preact-runtime-error.astro b/packages/astro/e2e/fixtures/errors/src/pages/preact-runtime-error.astro similarity index 100% rename from packages/astro/test/fixtures/errors/src/pages/preact-runtime-error.astro rename to packages/astro/e2e/fixtures/errors/src/pages/preact-runtime-error.astro diff --git a/packages/astro/test/fixtures/errors/src/pages/preact-syntax-error.astro b/packages/astro/e2e/fixtures/errors/src/pages/preact-syntax-error.astro similarity index 100% rename from packages/astro/test/fixtures/errors/src/pages/preact-syntax-error.astro rename to packages/astro/e2e/fixtures/errors/src/pages/preact-syntax-error.astro diff --git a/packages/astro/test/fixtures/errors/src/pages/react-runtime-error.astro b/packages/astro/e2e/fixtures/errors/src/pages/react-runtime-error.astro similarity index 100% rename from packages/astro/test/fixtures/errors/src/pages/react-runtime-error.astro rename to packages/astro/e2e/fixtures/errors/src/pages/react-runtime-error.astro diff --git a/packages/astro/test/fixtures/errors/src/pages/react-syntax-error.astro b/packages/astro/e2e/fixtures/errors/src/pages/react-syntax-error.astro similarity index 100% rename from packages/astro/test/fixtures/errors/src/pages/react-syntax-error.astro rename to packages/astro/e2e/fixtures/errors/src/pages/react-syntax-error.astro diff --git a/packages/astro/test/fixtures/errors/src/pages/solid-runtime-error.astro b/packages/astro/e2e/fixtures/errors/src/pages/solid-runtime-error.astro similarity index 100% rename from packages/astro/test/fixtures/errors/src/pages/solid-runtime-error.astro rename to packages/astro/e2e/fixtures/errors/src/pages/solid-runtime-error.astro diff --git a/packages/astro/test/fixtures/errors/src/pages/solid-syntax-error.astro b/packages/astro/e2e/fixtures/errors/src/pages/solid-syntax-error.astro similarity index 100% rename from packages/astro/test/fixtures/errors/src/pages/solid-syntax-error.astro rename to packages/astro/e2e/fixtures/errors/src/pages/solid-syntax-error.astro diff --git a/packages/astro/test/fixtures/errors/src/pages/svelte-runtime-error.astro b/packages/astro/e2e/fixtures/errors/src/pages/svelte-runtime-error.astro similarity index 100% rename from packages/astro/test/fixtures/errors/src/pages/svelte-runtime-error.astro rename to packages/astro/e2e/fixtures/errors/src/pages/svelte-runtime-error.astro diff --git a/packages/astro/test/fixtures/errors/src/pages/svelte-syntax-error.astro b/packages/astro/e2e/fixtures/errors/src/pages/svelte-syntax-error.astro similarity index 100% rename from packages/astro/test/fixtures/errors/src/pages/svelte-syntax-error.astro rename to packages/astro/e2e/fixtures/errors/src/pages/svelte-syntax-error.astro diff --git a/packages/astro/test/fixtures/errors/src/pages/vue-runtime-error.astro b/packages/astro/e2e/fixtures/errors/src/pages/vue-runtime-error.astro similarity index 100% rename from packages/astro/test/fixtures/errors/src/pages/vue-runtime-error.astro rename to packages/astro/e2e/fixtures/errors/src/pages/vue-runtime-error.astro diff --git a/packages/astro/test/fixtures/errors/src/pages/vue-syntax-error.astro b/packages/astro/e2e/fixtures/errors/src/pages/vue-syntax-error.astro similarity index 100% rename from packages/astro/test/fixtures/errors/src/pages/vue-syntax-error.astro rename to packages/astro/e2e/fixtures/errors/src/pages/vue-syntax-error.astro diff --git a/packages/astro/e2e/fixtures/preact-component/src/pages/mdx.mdx b/packages/astro/e2e/fixtures/preact-component/src/pages/mdx.mdx index 76952d942..0ba961550 100644 --- a/packages/astro/e2e/fixtures/preact-component/src/pages/mdx.mdx +++ b/packages/astro/e2e/fixtures/preact-component/src/pages/mdx.mdx @@ -1,4 +1,4 @@ - +export { default } from '../components/Layout.astro'; import Counter from '../components/Counter.jsx'; import PreactComponent from '../components/JSXComponent.jsx'; diff --git a/packages/astro/e2e/fixtures/react-component/src/pages/mdx.mdx b/packages/astro/e2e/fixtures/react-component/src/pages/mdx.mdx index be13d365e..fe6520933 100644 --- a/packages/astro/e2e/fixtures/react-component/src/pages/mdx.mdx +++ b/packages/astro/e2e/fixtures/react-component/src/pages/mdx.mdx @@ -1,3 +1,4 @@ +export { default } from '../components/Layout.astro'; import Counter from '../components/Counter.jsx'; import ReactComponent from '../components/JSXComponent.jsx'; diff --git a/packages/astro/e2e/fixtures/solid-component/src/pages/mdx.mdx b/packages/astro/e2e/fixtures/solid-component/src/pages/mdx.mdx index fcd3e3391..16186ba53 100644 --- a/packages/astro/e2e/fixtures/solid-component/src/pages/mdx.mdx +++ b/packages/astro/e2e/fixtures/solid-component/src/pages/mdx.mdx @@ -1,3 +1,4 @@ +export { default } from '../components/Layout.astro'; import Counter from '../components/Counter.jsx'; import SolidComponent from '../components/SolidComponent.jsx'; diff --git a/packages/astro/e2e/fixtures/svelte-component/src/pages/mdx.mdx b/packages/astro/e2e/fixtures/svelte-component/src/pages/mdx.mdx index 0550f3b9f..60a59a509 100644 --- a/packages/astro/e2e/fixtures/svelte-component/src/pages/mdx.mdx +++ b/packages/astro/e2e/fixtures/svelte-component/src/pages/mdx.mdx @@ -1,3 +1,4 @@ +export { default } from '../components/Layout.astro'; import Counter from '../components/Counter.svelte'; import SvelteComponent from '../components/SvelteComponent.svelte'; diff --git a/packages/astro/e2e/fixtures/vue-component/src/pages/mdx.mdx b/packages/astro/e2e/fixtures/vue-component/src/pages/mdx.mdx index b5d2285e5..5d0a141b0 100644 --- a/packages/astro/e2e/fixtures/vue-component/src/pages/mdx.mdx +++ b/packages/astro/e2e/fixtures/vue-component/src/pages/mdx.mdx @@ -1,3 +1,4 @@ +export { default } from '../components/Layout.astro'; import Counter from '../components/Counter.vue'; import VueComponent from '../components/VueComponent.vue'; diff --git a/packages/astro/e2e/preact-compat-component.test.js b/packages/astro/e2e/preact-compat-component.test.js index db4bc8ae9..cb01983e9 100644 --- a/packages/astro/e2e/preact-compat-component.test.js +++ b/packages/astro/e2e/preact-compat-component.test.js @@ -2,18 +2,23 @@ import { prepareTestFactory } from './shared-component-tests.js'; const { test, createTests } = prepareTestFactory({ root: './fixtures/preact-compat-component/' }); +const config = { + counterComponentFilePath: './src/components/Counter.jsx', + componentFilePath: './src/components/JSXComponent.jsx', +} + test.describe('preact/compat components in Astro files', () => { createTests({ + ...config, pageUrl: '/', pageSourceFilePath: './src/pages/index.astro', - componentFilePath: './src/components/JSXComponent.jsx', }); }); test.describe('preact/compat components in Markdown files', () => { createTests({ + ...config, pageUrl: '/markdown/', pageSourceFilePath: './src/pages/markdown.md', - componentFilePath: './src/components/JSXComponent.jsx', }); }); diff --git a/packages/astro/e2e/preact-component.test.js b/packages/astro/e2e/preact-component.test.js index 98f723cf8..6d9a0b4b9 100644 --- a/packages/astro/e2e/preact-component.test.js +++ b/packages/astro/e2e/preact-component.test.js @@ -2,26 +2,31 @@ import { prepareTestFactory } from './shared-component-tests.js'; const { test, createTests } = prepareTestFactory({ root: './fixtures/preact-component/' }); +const config = { + counterComponentFilePath: './src/components/Counter.jsx', + componentFilePath: './src/components/JSXComponent.jsx', +} + test.describe('Preact components in Astro files', () => { createTests({ + ...config, pageUrl: '/', pageSourceFilePath: './src/pages/index.astro', - componentFilePath: './src/components/JSXComponent.jsx', }); }); test.describe('Preact components in Markdown files', () => { createTests({ + ...config, pageUrl: '/markdown/', pageSourceFilePath: './src/pages/markdown.md', - componentFilePath: './src/components/JSXComponent.jsx', }); }); test.describe('Preact components in MDX files', () => { createTests({ + ...config, pageUrl: '/mdx/', pageSourceFilePath: './src/pages/mdx.mdx', - componentFilePath: './src/components/JSXComponent.jsx', }); }); diff --git a/packages/astro/e2e/react-component.test.js b/packages/astro/e2e/react-component.test.js index 7aba75a0c..3442564df 100644 --- a/packages/astro/e2e/react-component.test.js +++ b/packages/astro/e2e/react-component.test.js @@ -2,26 +2,31 @@ import { prepareTestFactory } from './shared-component-tests.js'; const { test, createTests } = prepareTestFactory({ root: './fixtures/react-component/' }); +const config = { + counterComponentFilePath: './src/components/Counter.jsx', + componentFilePath: './src/components/JSXComponent.jsx', +} + test.describe('React components in Astro files', () => { createTests({ + ...config, pageUrl: '/', pageSourceFilePath: './src/pages/index.astro', - componentFilePath: './src/components/JSXComponent.jsx', }); }); test.describe('React components in Markdown files', () => { createTests({ + ...config, pageUrl: '/markdown/', pageSourceFilePath: './src/pages/markdown.md', - componentFilePath: './src/components/JSXComponent.jsx', }); }); test.describe('React components in MDX files', () => { createTests({ + ...config, pageUrl: '/mdx/', pageSourceFilePath: './src/pages/mdx.mdx', - componentFilePath: './src/components/JSXComponent.jsx', }); }); diff --git a/packages/astro/e2e/shared-component-tests.js b/packages/astro/e2e/shared-component-tests.js index ce66f23b2..3c223d964 100644 --- a/packages/astro/e2e/shared-component-tests.js +++ b/packages/astro/e2e/shared-component-tests.js @@ -1,5 +1,5 @@ import { expect } from '@playwright/test'; -import { testFactory } from './test-utils.js'; +import { testFactory, getErrorOverlayMessage } from './test-utils.js'; export function prepareTestFactory(opts) { const test = testFactory(opts); diff --git a/packages/astro/e2e/solid-component.test.js b/packages/astro/e2e/solid-component.test.js index 033829b04..e1f34587d 100644 --- a/packages/astro/e2e/solid-component.test.js +++ b/packages/astro/e2e/solid-component.test.js @@ -2,26 +2,31 @@ import { prepareTestFactory } from './shared-component-tests.js'; const { test, createTests } = prepareTestFactory({ root: './fixtures/solid-component/' }); +const config = { + componentFilePath: './src/components/SolidComponent.jsx', + counterComponentFilePath: './src/components/Counter.jsx', +} + test.describe('Solid components in Astro files', () => { createTests({ + ...config, pageUrl: '/', pageSourceFilePath: './src/pages/index.astro', - componentFilePath: './src/components/SolidComponent.jsx', }); }); test.describe('Solid components in Markdown files', () => { createTests({ + ...config, pageUrl: '/markdown/', pageSourceFilePath: './src/pages/markdown.md', - componentFilePath: './src/components/SolidComponent.jsx', }); }); test.describe('Solid components in MDX files', () => { createTests({ + ...config, pageUrl: '/mdx/', pageSourceFilePath: './src/pages/mdx.mdx', - componentFilePath: './src/components/SolidComponent.jsx', }); }); diff --git a/packages/astro/e2e/svelte-component.test.js b/packages/astro/e2e/svelte-component.test.js index 7533d7cb7..637d74270 100644 --- a/packages/astro/e2e/svelte-component.test.js +++ b/packages/astro/e2e/svelte-component.test.js @@ -2,29 +2,32 @@ import { prepareTestFactory } from './shared-component-tests.js'; const { test, createTests } = prepareTestFactory({ root: './fixtures/svelte-component/' }); +const config = { + componentFilePath: './src/components/SvelteComponent.svelte', + counterComponentFilePath: './src/components/Counter.svelte', + counterCssFilePath: './src/components/Counter.svelte', +} + test.describe('Svelte components in Astro files', () => { createTests({ + ...config, pageUrl: '/', pageSourceFilePath: './src/pages/index.astro', - componentFilePath: './src/components/SvelteComponent.svelte', - counterCssFilePath: './src/components/Counter.svelte', }); }); test.describe('Svelte components in Markdown files', () => { createTests({ + ...config, pageUrl: '/markdown/', pageSourceFilePath: './src/pages/markdown.md', - componentFilePath: './src/components/SvelteComponent.svelte', - counterCssFilePath: './src/components/Counter.svelte', }); }); test.describe('Svelte components in MDX files', () => { createTests({ + ...config, pageUrl: '/mdx/', pageSourceFilePath: './src/pages/mdx.mdx', - componentFilePath: './src/components/SvelteComponent.svelte', - counterCssFilePath: './src/components/Counter.svelte', }); }); diff --git a/packages/astro/e2e/test-utils.js b/packages/astro/e2e/test-utils.js index 33edb1e3b..ba5e28053 100644 --- a/packages/astro/e2e/test-utils.js +++ b/packages/astro/e2e/test-utils.js @@ -1,4 +1,4 @@ -import { test as testBase } from '@playwright/test'; +import { test as testBase, expect } from '@playwright/test'; import { loadFixture as baseLoadFixture } from '../test/test-utils.js'; export function loadFixture(inlineConfig) { @@ -29,3 +29,11 @@ export function testFactory(inlineConfig) { return test; } + +export async function getErrorOverlayMessage(page) { + const overlay = await page.waitForSelector('vite-error-overlay', { strict: true, timeout: 10 * 1000 }) + + expect(overlay).toBeTruthy() + + return await overlay.$$eval('.message-body', (m) => m[0].textContent) +} diff --git a/packages/astro/e2e/vue-component.test.js b/packages/astro/e2e/vue-component.test.js index d975600dc..ccc18df2b 100644 --- a/packages/astro/e2e/vue-component.test.js +++ b/packages/astro/e2e/vue-component.test.js @@ -2,29 +2,32 @@ import { prepareTestFactory } from './shared-component-tests.js'; const { test, createTests } = prepareTestFactory({ root: './fixtures/vue-component/' }); +const config = { + componentFilePath: './src/components/VueComponent.vue', + counterCssFilePath: './src/components/Counter.vue', + counterComponentFilePath: './src/components/Counter.vue', +} + test.describe('Vue components in Astro files', () => { createTests({ + ...config, pageUrl: '/', pageSourceFilePath: './src/pages/index.astro', - componentFilePath: './src/components/VueComponent.vue', - counterCssFilePath: './src/components/Counter.vue', }); }); test.describe('Vue components in Markdown files', () => { createTests({ + ...config, pageUrl: '/markdown/', pageSourceFilePath: './src/pages/markdown.md', - componentFilePath: './src/components/VueComponent.vue', - counterCssFilePath: './src/components/Counter.vue', }); }); test.describe('Vue components in MDX files', () => { createTests({ + ...config, pageUrl: '/mdx/', pageSourceFilePath: './src/pages/mdx.mdx', - componentFilePath: './src/components/VueComponent.vue', - counterCssFilePath: './src/components/Counter.vue', }); }); diff --git a/packages/astro/package.json b/packages/astro/package.json index 631a12895..a5c7dfd97 100644 --- a/packages/astro/package.json +++ b/packages/astro/package.json @@ -76,7 +76,7 @@ "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 --ignore **/errors.test.js && mocha --timeout 20000 **/lit-element.test.js && mocha --timeout 20000 **/errors.test.js", + "test": "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" diff --git a/packages/astro/playwright.config.js b/packages/astro/playwright.config.js index 205c330d7..6dc2b01f4 100644 --- a/packages/astro/playwright.config.js +++ b/packages/astro/playwright.config.js @@ -9,12 +9,12 @@ const config = { * Maximum time expect() should wait for the condition to be met. * For example in `await expect(locator).toHaveText();` */ - timeout: 5000, + timeout: 3000, }, /* Fail the build on CI if you accidentally left test in the source code. */ forbidOnly: !!process.env.CI, /* Retry on CI only */ - retries: process.env.CI ? 5 : 0, + retries: process.env.CI ? 3 : 0, /* Opt out of parallel tests on CI. */ workers: 1, /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */ @@ -33,6 +33,7 @@ const config = { use: { browserName: 'chromium', channel: 'chrome', + args: ["--use-gl=egl"] }, }, ], diff --git a/packages/astro/src/core/create-vite.ts b/packages/astro/src/core/create-vite.ts index 6d0ceb7a5..176471e43 100644 --- a/packages/astro/src/core/create-vite.ts +++ b/packages/astro/src/core/create-vite.ts @@ -4,6 +4,7 @@ import type { LogOptions } from './logger/core'; import fs from 'fs'; import { fileURLToPath } from 'url'; import * as vite from 'vite'; +import { createCustomViteLogger } from './errors.js'; import astroPostprocessVitePlugin from '../vite-plugin-astro-postprocess/index.js'; import astroViteServerPlugin from '../vite-plugin-astro-server/index.js'; import astroVitePlugin from '../vite-plugin-astro/index.js'; @@ -60,7 +61,7 @@ export async function createVite( clearScreen: false, // we want to control the output, not Vite logLevel: 'warn', // log warnings and errors only optimizeDeps: { - entries: ['src/**/*'], // Try and scan a user’s project (won’t catch everything), + entries: ['src/**/*'], exclude: ['node-fetch'], }, plugins: [ @@ -133,6 +134,8 @@ export async function createVite( sortPlugins(result.plugins); } + result.customLogger = createCustomViteLogger(result.logLevel ?? 'warn'); + return result; } diff --git a/packages/astro/src/core/errors.ts b/packages/astro/src/core/errors.ts index 793cde1d1..2926ad291 100644 --- a/packages/astro/src/core/errors.ts +++ b/packages/astro/src/core/errors.ts @@ -1,8 +1,12 @@ -import eol from 'eol'; import type { BuildResult } from 'esbuild'; -import fs from 'fs'; -import type { ViteDevServer } from 'vite'; +import type { ViteDevServer, ErrorPayload, LogLevel, Logger } from 'vite'; import type { SSRError } from '../@types/astro'; + +import { fileURLToPath } from 'node:url'; +import { createLogger } from 'vite'; +import eol from 'eol'; +import fs from 'fs'; +import stripAnsi from 'strip-ansi'; import { codeFrame, createSafeError } from './util.js'; export enum AstroErrorCodes { @@ -34,11 +38,12 @@ export function cleanErrorStack(stack: string) { return stack .split(/\n/g) .filter((l) => /^\s*at/.test(l)) + .map(l => l.replace(/\/@fs\//g, '/')) .join('\n'); } /** Update the error message to correct any vite-isms that we don't want to expose to the user. */ -export function fixViteErrorMessage(_err: unknown, server: ViteDevServer) { +export function fixViteErrorMessage(_err: unknown, server: ViteDevServer, filePath?: URL) { const err = createSafeError(_err); // Vite will give you better stacktraces, using sourcemaps. server.ssrFixStacktrace(err); @@ -47,6 +52,18 @@ export function fixViteErrorMessage(_err: unknown, server: ViteDevServer) { if (err.message === 'import.meta.glob() can only accept string literals.') { err.message = 'Astro.glob() and import.meta.glob() can only accept string literals.'; } + if (filePath && /failed to load module for ssr:/.test(err.message)) { + const importName = err.message.split('for ssr:').at(1)?.trim(); + if (importName) { + const content = fs.readFileSync(fileURLToPath(filePath)).toString(); + const lns = content.split('\n') + const line = lns.findIndex(ln => ln.includes(importName)); + const column = lns[line].indexOf(importName); + if (!(err as any).id) { + (err as any).id = `${fileURLToPath(filePath)}:${line + 1}:${column + 1}` + } + } + } return err; } @@ -55,7 +72,21 @@ const incompatiblePackages = { }; const incompatPackageExp = new RegExp(`(${Object.keys(incompatiblePackages).join('|')})`); -function generateHint(err: ErrorWithMetadata): string | undefined { + +export function createCustomViteLogger(logLevel: LogLevel): Logger { + const viteLogger = createLogger(logLevel); + const logger: Logger = { + ...viteLogger, + error(msg, options?) { + // Silence warnings from incompatible packages (we log better errors for these) + if (incompatPackageExp.test(msg)) return; + return viteLogger.error(msg, options); + }, + } + return logger; +} + +function generateHint(err: ErrorWithMetadata, filePath?: URL): string | undefined { if (/Unknown file extension \"\.(jsx|vue|svelte|astro|css)\" for /.test(err.message)) { return 'You likely need to add this package to `vite.ssr.noExternal` in your astro config file.'; } else { @@ -72,29 +103,54 @@ function generateHint(err: ErrorWithMetadata): string | undefined { * Takes any error-like object and returns a standardized Error + metadata object. * Useful for consistent reporting regardless of where the error surfaced from. */ -export function collectErrorMetadata(e: any): ErrorWithMetadata { - // normalize error stack line-endings to \n +export function collectErrorMetadata(e: any, filePath?: URL): ErrorWithMetadata { + const err = e as SSRError; + if ((e as any).stack) { + // normalize error stack line-endings to \n (e as any).stack = eol.lf((e as any).stack); + // derive error location from stack (if possible) + const stackText = stripAnsi(e.stack); + // TODO: this could be better, `src` might be something else + const possibleFilePath = err.pluginCode || err.id || stackText.split('\n').find(ln => ln.includes('src') || ln.includes('node_modules')); + const source = possibleFilePath?.replace(/^[^(]+\(([^)]+).*$/, '$1'); + const [file, line, column] = source?.split(':') ?? []; + if (!err.loc && line && column) { + err.loc = { + file, + line: Number.parseInt(line), + column: Number.parseInt(column) + } + } + + // Derive plugin from stack (if possible) + if (!err.plugin) { + err.plugin = + /withastro\/astro\/packages\/integrations\/([\w-]+)/gmi.exec(stackText)?.at(1) || + /(@astrojs\/[\w-]+)\/(server|client|index)/gmi.exec(stackText)?.at(1) || + undefined; + } + + // Normalize stack (remove `/@fs/` urls, etc) + err.stack = cleanErrorStack(e.stack) } if (e.name === 'YAMLException') { - const err = e as SSRError; err.loc = { file: (e as any).id, line: (e as any).mark.line, column: (e as any).mark.column }; err.message = (e as any).reason; + } - if (!err.frame) { - try { - const fileContents = fs.readFileSync(err.loc.file!, 'utf8'); - err.frame = codeFrame(fileContents, err.loc); - } catch {} - } + if (!err.frame && err.loc) { + try { + const fileContents = fs.readFileSync(err.loc.file!, 'utf8'); + const frame = codeFrame(fileContents, err.loc); + err.frame = frame; + } catch {} } // Astro error (thrown by esbuild so it needs to be formatted for Vite) if (Array.isArray((e as any).errors)) { const { location, pluginName, text } = (e as BuildResult).errors[0]; - const err = e as SSRError; if (location) { err.loc = { file: location.file, line: location.line, column: location.column }; err.id = err.id || location?.file; @@ -111,11 +167,28 @@ export function collectErrorMetadata(e: any): ErrorWithMetadata { if (pluginName) { err.plugin = pluginName; } - err.hint = generateHint(err); + err.hint = generateHint(err, filePath); return err; } // Generic error (probably from Vite, and already formatted) - e.hint = generateHint(e); - return e; + err.hint = generateHint(e, filePath); + return err; +} + +export function getViteErrorPayload(err: ErrorWithMetadata): ErrorPayload { + let plugin = err.plugin; + if (!plugin && err.hint) { + plugin = 'astro'; + } + const message = `${err.message}\n\n${err.hint ?? ''}`; + return { + type: 'error', + err: { + ...err, + plugin, + message: message.trim(), + stack: err.stack, + } + } } diff --git a/packages/astro/src/core/util.ts b/packages/astro/src/core/util.ts index 0bbc0a72a..36f445236 100644 --- a/packages/astro/src/core/util.ts +++ b/packages/astro/src/core/util.ts @@ -79,7 +79,7 @@ export function createSafeError(err: any): Error { /** generate code frame from esbuild error */ export function codeFrame(src: string, loc: ErrorPayload['err']['loc']): string { if (!loc) return ''; - const lines = eol.lf(src).split('\n'); + const lines = eol.lf(src).split('\n').map(ln => ln.replace(/\t/g, ' ')); // grab 2 lines before, and 3 lines after focused line const visibleLines = []; for (let n = -2; n <= 2; n++) { @@ -98,9 +98,7 @@ export function codeFrame(src: string, loc: ErrorPayload['err']['loc']): string output += isFocusedLine ? '> ' : ' '; output += `${lineNo + 1} | ${lines[lineNo]}\n`; if (isFocusedLine) - output += `${[...new Array(gutterWidth)].join(' ')} | ${[...new Array(loc.column)].join( - ' ' - )}^\n`; + output += `${Array.from({ length: gutterWidth }).join(' ')} | ${Array.from({ length: loc.column }).join(' ')}^\n`; } return output; } diff --git a/packages/astro/src/runtime/server/index.ts b/packages/astro/src/runtime/server/index.ts index 4a5bbac6e..501dfd828 100644 --- a/packages/astro/src/runtime/server/index.ts +++ b/packages/astro/src/runtime/server/index.ts @@ -156,6 +156,7 @@ export function mergeSlots(...slotted: unknown[]) { } export const Fragment = Symbol.for('astro:fragment'); +export const Renderer = Symbol.for('astro:renderer'); export const ClientOnlyPlaceholder = 'astro-client-only'; function guessRenderers(componentUrl?: string): string[] { @@ -182,6 +183,17 @@ function formatList(values: string[]): string { const rendererAliases = new Map([['solid', 'solid-js']]); +/** @internal Assosciate JSX components with a specific renderer (see /src/vite-plugin-jsx/tag.ts) */ +export function __astro_tag_component__(Component: unknown, rendererName: string) { + if (!Component) return; + if (typeof Component !== 'function') return; + Object.defineProperty(Component, Renderer, { + value: rendererName, + enumerable: false, + writable: false + }) +} + export async function renderComponent( result: SSRResult, displayName: string, @@ -270,20 +282,30 @@ Did you mean to add ${formatList(probableRendererNames.map((r) => '`' + r + '`') // Call the renderers `check` hook to see if any claim this component. let renderer: SSRLoadedRenderer | undefined; if (metadata.hydrate !== 'only') { - let error; - for (const r of renderers) { - try { - if (await r.ssr.check.call({ result }, Component, props, children)) { - renderer = r; - break; - } - } catch (e) { - error ??= e; - } + // If this component ran through `__astro_tag_component__`, we already know + // which renderer to match to and can skip the usual `check` calls. + // This will help us throw most relevant error message for modules with runtime errors + if (Component && (Component as any)[Renderer]) { + const rendererName = (Component as any)[Renderer]; + renderer = renderers.find(({ name }) => name === rendererName); } - if (error) { - throw error; + if (!renderer) { + let error; + for (const r of renderers) { + try { + if (await r.ssr.check.call({ result }, Component, props, children)) { + renderer = r; + break; + } + } catch (e) { + error ??= e; + } + } + + if (error) { + throw error; + } } if (!renderer && typeof HTMLElement === 'function' && componentIsHTMLElement(Component)) { @@ -298,9 +320,9 @@ Did you mean to add ${formatList(probableRendererNames.map((r) => '`' + r + '`') const rendererName = rendererAliases.has(passedName) ? rendererAliases.get(passedName) : passedName; - renderer = renderers.filter( + renderer = renderers.find( ({ name }) => name === `@astrojs/${rendererName}` || name === rendererName - )[0]; + ); } // Attempt: user only has a single renderer, default to that if (!renderer && renderers.length === 1) { @@ -766,17 +788,21 @@ export async function renderPage( start(controller) { async function read() { let i = 0; - for await (const chunk of iterable) { - let html = chunk.toString(); - if (i === 0) { - if (!/\n')); + try { + for await (const chunk of iterable) { + let html = chunk.toString(); + if (i === 0) { + if (!/\n')); + } } + controller.enqueue(encoder.encode(html)); + i++; } - controller.enqueue(encoder.encode(html)); - i++; + controller.close(); + } catch (e) { + controller.error(e) } - controller.close(); } read(); }, @@ -851,7 +877,7 @@ export async function* renderAstroComponent( if (value || value === 0) { for await (const chunk of _render(value)) { yield markHTMLString(chunk); - } + } } } } diff --git a/packages/astro/src/template/5xx.ts b/packages/astro/src/template/5xx.ts deleted file mode 100644 index 0354fbbfb..000000000 --- a/packages/astro/src/template/5xx.ts +++ /dev/null @@ -1,80 +0,0 @@ -import { encode } from 'html-entities'; -import { baseCSS } from './css.js'; - -interface ErrorTemplateOptions { - /** a short description of the error */ - message: string; - /** information about where the error occurred */ - stack?: string; - /** HTTP error code */ - statusCode?: number; - /** HTML */ - tabTitle: string; - /** page title */ - title: string; - /** show user a URL for more info or action to take */ - url?: string; -} - -/** Display all errors */ -export default function template({ - title, - url, - message, - stack, - statusCode, - tabTitle, -}: ErrorTemplateOptions): string { - let error = url ? message.replace(url, '') : message; - return `<!doctype html> - <html lang="en"> - <head> - <meta charset="UTF-8"> - <title>${tabTitle} - - - -
-
- -

${ - statusCode ? `${statusCode}: ` : '' - }${title}

-
- 
${encode(error)}
- ${url ? `${url}` : ''} - 
${encode(stack)}
-
- - `; -} diff --git a/packages/astro/src/vite-plugin-astro-server/index.ts b/packages/astro/src/vite-plugin-astro-server/index.ts index 0b8cfd9b7..d12df2d61 100644 --- a/packages/astro/src/vite-plugin-astro-server/index.ts +++ b/packages/astro/src/vite-plugin-astro-server/index.ts @@ -5,9 +5,8 @@ import type { AstroConfig, ManifestData } from '../@types/astro'; import type { SSROptions } from '../core/render/dev/index'; import { Readable } from 'stream'; -import stripAnsi from 'strip-ansi'; import { call as callEndpoint } from '../core/endpoint/dev/index.js'; -import { collectErrorMetadata, fixViteErrorMessage } from '../core/errors.js'; +import { collectErrorMetadata, ErrorWithMetadata, fixViteErrorMessage, getViteErrorPayload } from '../core/errors.js'; import { error, info, LogOptions, warn } from '../core/logger/core.js'; import * as msg from '../core/messages.js'; import { appendForwardSlash } from '../core/path.js'; @@ -18,7 +17,6 @@ import { createRequest } from '../core/request.js'; import { createRouteManifest, matchRoute } from '../core/routing/index.js'; import { createSafeError, isBuildingToSSR, resolvePages } from '../core/util.js'; import notFoundTemplate, { subpathNotUsedTemplate } from '../template/4xx.js'; -import serverErrorTemplate from '../template/5xx.js'; interface AstroPluginOptions { config: AstroConfig; @@ -151,19 +149,14 @@ async function handle500Response( origin: string, req: http.IncomingMessage, res: http.ServerResponse, - err: any + err: ErrorWithMetadata ) { - const pathname = decodeURI(new URL('./index.html', origin + req.url).pathname); - const html = serverErrorTemplate({ - statusCode: 500, - title: 'Internal Error', - tabTitle: '500: Error', - message: stripAnsi(err.hint ?? err.message), - url: err.url || undefined, - stack: truncateString(stripAnsi(err.stack), 500), - }); - const transformedHtml = await viteServer.transformIndexHtml(pathname, html); - writeHtmlResponse(res, 500, transformedHtml); + res.on('close', () => setTimeout(() => viteServer.ws.send(getViteErrorPayload(err)), 200)) + if (res.headersSent) { + res.end() + } else { + writeHtmlResponse(res, 500, `${err.name}`); + } } function getCustom404Route(config: AstroConfig, manifest: ManifestData) { @@ -232,6 +225,7 @@ async function handleRequest( clientAddress: buildingToSSR ? req.socket.remoteAddress : undefined, }); + let filePath: URL|undefined; try { if (!pathname.startsWith(devRoot)) { log404(logging, pathname); @@ -252,7 +246,7 @@ async function handleRequest( } } - const filePath = new URL(`./${route.component}`, config.root); + filePath = new URL(`./${route.component}`, config.root); const preloadedComponent = await preload({ astroConfig: config, filePath, viteServer }); const [, mod] = preloadedComponent; // attempt to get static paths @@ -330,10 +324,10 @@ async function handleRequest( return await writeSSRResult(result, res); } } catch (_err) { - const err = fixViteErrorMessage(createSafeError(_err), viteServer); - const errorWithMetadata = collectErrorMetadata(_err); + const err = fixViteErrorMessage(createSafeError(_err), viteServer, filePath); + const errorWithMetadata = collectErrorMetadata(err); error(logging, null, msg.formatErrorMessage(errorWithMetadata)); - handle500Response(viteServer, origin, req, res, err); + handle500Response(viteServer, origin, req, res, errorWithMetadata); } } diff --git a/packages/astro/src/vite-plugin-jsx/index.ts b/packages/astro/src/vite-plugin-jsx/index.ts index a08460204..03d2882ad 100644 --- a/packages/astro/src/vite-plugin-jsx/index.ts +++ b/packages/astro/src/vite-plugin-jsx/index.ts @@ -5,6 +5,7 @@ import type { LogOptions } from '../core/logger/core.js'; import type { PluginMetadata } from '../vite-plugin-astro/types'; import babel from '@babel/core'; +import tagExportsPlugin from './tag.js'; import * as eslexer from 'es-module-lexer'; import esbuild from 'esbuild'; import * as colors from 'kleur/colors'; @@ -55,7 +56,7 @@ async function transformJSX({ }: TransformJSXOptions): Promise { const { jsxTransformOptions } = renderer; const options = await jsxTransformOptions!({ mode, ssr }); - const plugins = [...(options.plugins || [])]; + const plugins = [...(options.plugins || []), tagExportsPlugin({ rendererName: renderer.name })]; const result = await babel.transformAsync(code, { presets: options.presets, plugins, diff --git a/packages/astro/src/vite-plugin-jsx/tag.ts b/packages/astro/src/vite-plugin-jsx/tag.ts new file mode 100644 index 000000000..4bb3b2341 --- /dev/null +++ b/packages/astro/src/vite-plugin-jsx/tag.ts @@ -0,0 +1,64 @@ +import type { PluginObj } from '@babel/core'; +import * as t from '@babel/types'; + +/** + * This plugin handles every file that runs through our JSX plugin. + * Since we statically match every JSX file to an Astro renderer based on import scanning, + * it would be helpful to embed some of that metadata at runtime. + * + * This plugin crawls each export in the file and "tags" each export with a given `rendererName`. + * This allows us to automatically match a component to a renderer and skip the usual `check()` calls. + */ +export default function tagExportsWithRenderer({ rendererName }: { rendererName: string }): PluginObj { + return { + visitor: { + Program: { + // Inject `import { __astro_tag_component__ } from 'astro/server/index.js'` + enter(path) { + path.node.body.splice( + 0, + 0, + t.importDeclaration( + [t.importSpecifier(t.identifier('__astro_tag_component__'), t.identifier('__astro_tag_component__'))], + t.stringLiteral('astro/server/index.js') + ) + ); + }, + // For each export we found, inject `__astro_tag_component__(exportName, rendererName)` + exit(path, state) { + const exportedIds = state.get('astro:tags') + if (exportedIds) { + for (const id of exportedIds) { + path.node.body.push( + t.expressionStatement(t.callExpression(t.identifier('__astro_tag_component__'), [t.identifier(id), t.stringLiteral(rendererName)])) + ) + } + } + } + }, + ExportDeclaration(path, state) { + const node = path.node; + if (node.exportKind === 'type') return; + if (node.type === 'ExportAllDeclaration') return; + + if (node.type === 'ExportNamedDeclaration') { + if (t.isFunctionDeclaration(node.declaration)) { + if (node.declaration.id?.name) { + const id = node.declaration.id.name; + const tags = state.get('astro:tags') ?? [] + state.set('astro:tags', [...tags, id]) + } + } + } else if (node.type === 'ExportDefaultDeclaration') { + if (t.isFunctionDeclaration(node.declaration)) { + if (node.declaration.id?.name) { + const id = node.declaration.id.name; + const tags = state.get('astro:tags') ?? [] + state.set('astro:tags', [...tags, id]) + } + } + } + }, + } + }; +} diff --git a/packages/astro/test/error-react-spectrum.test.js b/packages/astro/test/error-react-spectrum.test.js deleted file mode 100644 index a16e8632a..000000000 --- a/packages/astro/test/error-react-spectrum.test.js +++ /dev/null @@ -1,29 +0,0 @@ -import { isWindows, loadFixture } from './test-utils.js'; -import { expect } from 'chai'; -import * as cheerio from 'cheerio'; - -describe('Error packages: react-spectrum', () => { - if (isWindows) return; - - /** @type {import('./test-utils').Fixture} */ - let fixture; - /** @type {import('./test-utils').DevServer} */ - let devServer; - - before(async () => { - fixture = await loadFixture({ - root: './fixtures/error-react-spectrum', - }); - }); - after(async () => { - devServer && devServer.stop(); - }); - - it('properly detect syntax errors in template', async () => { - devServer = await fixture.startDevServer(); - let html = await fixture.fetch('/').then((res) => res.text()); - let $ = cheerio.load(html); - const msg = $('.error-message').text(); - expect(msg).to.match(/@adobe\/react-spectrum is not compatible/); - }); -}); diff --git a/packages/astro/test/errors.test.js b/packages/astro/test/errors.test.js deleted file mode 100644 index 01fe61b9a..000000000 --- a/packages/astro/test/errors.test.js +++ /dev/null @@ -1,100 +0,0 @@ -import { isWindows, isLinux, loadFixture } from './test-utils.js'; -import { expect } from 'chai'; -import * as cheerio from 'cheerio'; - -describe('Error display', () => { - if (isWindows) return; - - /** @type {import('./test-utils').Fixture} */ - let fixture; - - before(async () => { - fixture = await loadFixture({ - root: './fixtures/errors', - }); - }); - - /** - * TODO: Track down reliability issue - * - * After fixing a syntax error on one page, the dev server hangs on the hmr.js request. - * This is specific to a project that has other framework component errors, - * in this case the fixture has multiple broken pages and components. - * - * The issue could be internal to vite, the hmr.js request triggers connect:dispatcher - * events but vite:load is never actually called. - */ - describe.skip('Astro template syntax', async () => { - let devServer; - - beforeEach(async () => { - devServer = await fixture.startDevServer(); - }); - - afterEach(async () => { - await devServer.stop(); - }); - - it('properly detect syntax errors in template', async () => { - let html = await fixture.fetch('/astro-syntax-error').then((res) => res.text()); - - // 1. Verify an error message is being shown. - let $ = cheerio.load(html); - expect($('.statusMessage').text()).to.equal('Internal Error'); - expect($('.error-message').text()).to.contain('Unexpected "}"'); - - // 2. Edit the file, fixing the error - await fixture.editFile('./src/pages/astro-syntax-error.astro', `

No syntax error

`); - - // 3. Verify that the file is fixed. - html = await fixture.fetch('/astro-syntax-error').then((res) => res.text()); - $ = cheerio.load(html); - expect($('h1').text()).to.equal('No syntax error'); - }); - - it('shows useful error when frontmatter import is not found', async () => { - let html = await fixture.fetch('/import-not-found').then((res) => res.text()); - - // 1. Verify an error message is being shown. - let $ = cheerio.load(html); - expect($('.statusMessage').text()).to.equal('Internal Error'); - expect($('.error-message').text()).to.equal('failed to load module for ssr: ../abc.astro'); - - // 2. Edit the file, fixing the error - await fixture.editFile('./src/pages/import-not-found.astro', '

No import error

'); - - // 3. Verify that the file is fixed. - html = await fixture.fetch('/import-not-found').then((res) => res.text()); - $ = cheerio.load(html); - expect($('h1').text()).to.equal('No import error'); - }); - }); - - describe('Framework components', function () { - let devServer; - - before(async () => { - devServer = await fixture.startDevServer(); - }); - - after(async () => { - await devServer.stop(); - }); - - it('Errors recover when fixed', async () => { - let html = await fixture.fetch('/svelte-syntax-error').then((res) => res.text()); - - // 1. Verify an error message is being shown. - let $ = cheerio.load(html); - expect($('.statusMessage').text()).to.equal('Internal Error'); - - // 2. Edit the file, fixing the error - await fixture.editFile('./src/components/SvelteSyntaxError.svelte', `

No mismatch

`); - - // 3. Verify that the file is fixed. - html = await fixture.fetch('/svelte-syntax-error').then((res) => res.text()); - $ = cheerio.load(html); - expect($('h1').text()).to.equal('No mismatch'); - }); - }); -}); diff --git a/packages/astro/test/sass.test.js b/packages/astro/test/sass.test.js deleted file mode 100644 index 07c1f1499..000000000 --- a/packages/astro/test/sass.test.js +++ /dev/null @@ -1,26 +0,0 @@ -import { expect } from 'chai'; -import os from 'os'; -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({ root: './fixtures/sass/' }); - devServer = await fixture.startDevServer(); - }); - - after(async () => { - await devServer.stop(); - }); - - // TODO: Sass cannot be found on macOS for some reason... Vite issue? - const test = os.platform() === 'darwin' ? it.skip : it; - test('shows helpful error on failure', async () => { - const text = await fixture.fetch('/error').then((res) => res.text()); - expect(text).to.include('Undefined variable'); - }); -}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 01ec6c10d..8bcedfd28 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -700,6 +700,48 @@ importers: '@astrojs/vue': link:../../../../integrations/vue astro: link:../../.. + packages/astro/e2e/fixtures/error-react-spectrum: + specifiers: + '@adobe/react-spectrum': ^3.18.0 + '@astrojs/react': workspace:* + astro: workspace:* + react: ^18.1.0 + react-dom: ^18.1.0 + dependencies: + '@adobe/react-spectrum': 3.19.0_biqbaboplfbrettd7655fr4n2y + '@astrojs/react': link:../../../../integrations/react + astro: link:../../.. + react: 18.2.0 + react-dom: 18.2.0_react@18.2.0 + + packages/astro/e2e/fixtures/error-sass: + specifiers: + astro: workspace:* + sass: ^1.52.2 + dependencies: + astro: link:../../.. + sass: 1.53.0 + + packages/astro/e2e/fixtures/errors: + specifiers: + '@astrojs/preact': workspace:* + '@astrojs/react': workspace:* + '@astrojs/solid-js': workspace:* + '@astrojs/svelte': workspace:* + '@astrojs/vue': workspace:* + astro: workspace:* + react: ^18.1.0 + react-dom: ^18.1.0 + dependencies: + '@astrojs/preact': link:../../../../integrations/preact + '@astrojs/react': link:../../../../integrations/react + '@astrojs/solid-js': link:../../../../integrations/solid + '@astrojs/svelte': link:../../../../integrations/svelte + '@astrojs/vue': link:../../../../integrations/vue + astro: link:../../.. + react: 18.2.0 + react-dom: 18.2.0_react@18.2.0 + packages/astro/e2e/fixtures/lit-component: specifiers: '@astrojs/lit': workspace:* @@ -1467,40 +1509,6 @@ importers: '@astrojs/preact': link:../../../../integrations/preact astro: link:../../.. - packages/astro/test/fixtures/error-react-spectrum: - specifiers: - '@adobe/react-spectrum': ^3.18.0 - '@astrojs/react': workspace:* - astro: workspace:* - react: ^18.1.0 - react-dom: ^18.1.0 - dependencies: - '@adobe/react-spectrum': 3.19.0_biqbaboplfbrettd7655fr4n2y - '@astrojs/react': link:../../../../integrations/react - astro: link:../../.. - react: 18.2.0 - react-dom: 18.2.0_react@18.2.0 - - packages/astro/test/fixtures/errors: - specifiers: - '@astrojs/preact': workspace:* - '@astrojs/react': workspace:* - '@astrojs/solid-js': workspace:* - '@astrojs/svelte': workspace:* - '@astrojs/vue': workspace:* - astro: workspace:* - react: ^18.1.0 - react-dom: ^18.1.0 - dependencies: - '@astrojs/preact': link:../../../../integrations/preact - '@astrojs/react': link:../../../../integrations/react - '@astrojs/solid-js': link:../../../../integrations/solid - '@astrojs/svelte': link:../../../../integrations/svelte - '@astrojs/vue': link:../../../../integrations/vue - astro: link:../../.. - react: 18.2.0 - react-dom: 18.2.0_react@18.2.0 - packages/astro/test/fixtures/fetch: specifiers: '@astrojs/preact': workspace:* @@ -1679,14 +1687,6 @@ importers: dependencies: astro: link:../../.. - packages/astro/test/fixtures/sass: - specifiers: - astro: workspace:* - sass: ^1.52.2 - dependencies: - astro: link:../../.. - sass: 1.53.0 - packages/astro/test/fixtures/slots-preact: specifiers: '@astrojs/preact': workspace:*