From 1434f48f7c53897905a1758b9318450c6b356522 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Wed, 30 Oct 2024 15:33:15 +0100 Subject: [PATCH] fix: make compiler error extend from `Error` (#14036) We originally didn't extend from `Error` anymore because its fields are of no real value to us, and has problems with serialization in a worker context. Turns out this was a mistake, because various build tools rely on errors being thrown as something that extends Error, else they try to wrap it in their own error. We therefore revert that change while still trying to preserve most of the advantages of not extending `Error`, namely nuking the useless stack trace and making sure the message is enumerable. --- .changeset/wet-timers-shop.md | 5 ++++ .../templates/compile-errors.js | 23 ++++++++++++++++--- packages/svelte/src/compiler/errors.js | 22 +++++++++++++++--- .../src/compiler/utils/compile_diagnostic.js | 2 -- .../impossible-migrate-with-errors/_config.js | 2 -- packages/svelte/tests/migrate/test.ts | 2 +- 6 files changed, 45 insertions(+), 11 deletions(-) create mode 100644 .changeset/wet-timers-shop.md diff --git a/.changeset/wet-timers-shop.md b/.changeset/wet-timers-shop.md new file mode 100644 index 0000000000..55d12496e0 --- /dev/null +++ b/.changeset/wet-timers-shop.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: make compiler error extend from `Error` diff --git a/packages/svelte/scripts/process-messages/templates/compile-errors.js b/packages/svelte/scripts/process-messages/templates/compile-errors.js index a407699e1b..0dc30b2c01 100644 --- a/packages/svelte/scripts/process-messages/templates/compile-errors.js +++ b/packages/svelte/scripts/process-messages/templates/compile-errors.js @@ -2,8 +2,9 @@ import { CompileDiagnostic } from './utils/compile_diagnostic.js'; /** @typedef {{ start?: number, end?: number }} NodeLike */ -class InternalCompileError extends CompileDiagnostic { - name = 'CompileError'; +class InternalCompileError extends Error { + message = ''; // ensure this property is enumerable + #diagnostic; /** * @param {string} code @@ -11,7 +12,23 @@ class InternalCompileError extends CompileDiagnostic { * @param {[number, number] | undefined} position */ constructor(code, message, position) { - super(code, message, position); + super(message); + this.stack = ''; // avoid unnecessary noise; don't set it as a class property or it becomes enumerable + + // We want to extend from Error so that various bundler plugins properly handle it. + // But we also want to share the same object shape with that of warnings, therefore + // we create an instance of the shared class an copy over its properties. + this.#diagnostic = new CompileDiagnostic(code, message, position); + Object.assign(this, this.#diagnostic); + this.name = 'CompileError'; + } + + toString() { + return this.#diagnostic.toString(); + } + + toJSON() { + return this.#diagnostic.toJSON(); } } diff --git a/packages/svelte/src/compiler/errors.js b/packages/svelte/src/compiler/errors.js index 299154c988..8f1dae98b1 100644 --- a/packages/svelte/src/compiler/errors.js +++ b/packages/svelte/src/compiler/errors.js @@ -3,8 +3,9 @@ import { CompileDiagnostic } from './utils/compile_diagnostic.js'; /** @typedef {{ start?: number, end?: number }} NodeLike */ -class InternalCompileError extends CompileDiagnostic { - name = 'CompileError'; +class InternalCompileError extends Error { + message = ''; // ensure this property is enumerable + #diagnostic; /** * @param {string} code @@ -12,7 +13,22 @@ class InternalCompileError extends CompileDiagnostic { * @param {[number, number] | undefined} position */ constructor(code, message, position) { - super(code, message, position); + super(message); + this.stack = ''; // avoid unnecessary noise; don't set it as a class property or it becomes enumerable + // We want to extend from Error so that various bundler plugins properly handle it. + // But we also want to share the same object shape with that of warnings, therefore + // we create an instance of the shared class an copy over its properties. + this.#diagnostic = new CompileDiagnostic(code, message, position); + Object.assign(this, this.#diagnostic); + this.name = 'CompileError'; + } + + toString() { + return this.#diagnostic.toString(); + } + + toJSON() { + return this.#diagnostic.toJSON(); } } diff --git a/packages/svelte/src/compiler/utils/compile_diagnostic.js b/packages/svelte/src/compiler/utils/compile_diagnostic.js index 374a20b8b5..db938cf2bd 100644 --- a/packages/svelte/src/compiler/utils/compile_diagnostic.js +++ b/packages/svelte/src/compiler/utils/compile_diagnostic.js @@ -51,8 +51,6 @@ function get_code_frame(source, line, column) { /** @implements {ICompileDiagnostic} */ export class CompileDiagnostic { name = 'CompileDiagnostic'; - // adding an empty stack so that vite will show the file and frame during build - stack = ''; /** * @param {string} code diff --git a/packages/svelte/tests/migrate/samples/impossible-migrate-with-errors/_config.js b/packages/svelte/tests/migrate/samples/impossible-migrate-with-errors/_config.js index 57496dcba7..9e4b7222af 100644 --- a/packages/svelte/tests/migrate/samples/impossible-migrate-with-errors/_config.js +++ b/packages/svelte/tests/migrate/samples/impossible-migrate-with-errors/_config.js @@ -19,8 +19,6 @@ export default test({ 3: unterminated template ^`, message: 'Unexpected end of input', - name: 'CompileError', - stack: '', position: [30, 30], start: { character: 30, diff --git a/packages/svelte/tests/migrate/test.ts b/packages/svelte/tests/migrate/test.ts index a4b48f2fa7..a74bc80488 100644 --- a/packages/svelte/tests/migrate/test.ts +++ b/packages/svelte/tests/migrate/test.ts @@ -28,7 +28,7 @@ const { test, run } = suite(async (config, cwd) => { if (config.errors) { console.error = (...args) => { - errors.push(...args); + errors.push(...args.map((arg) => arg.toJSON?.() ?? arg)); }; }