From 5121aac913f2f2d98edeeb6a0d6f7cd6e7278b51 Mon Sep 17 00:00:00 2001 From: adiguba Date: Sat, 22 Feb 2025 09:34:51 +0100 Subject: [PATCH 1/9] fix: catch error on @const tag in svelte:boundary --- .../3-transform/client/visitors/ConstTag.js | 42 +++++++++++-------- .../client/visitors/SvelteBoundary.js | 5 +++ .../svelte/src/compiler/types/template.d.ts | 5 ++- packages/svelte/types/index.d.ts | 4 +- 4 files changed, 37 insertions(+), 19 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ConstTag.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ConstTag.js index 7e33aea4356b..45396108491c 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ConstTag.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ConstTag.js @@ -1,4 +1,4 @@ -/** @import { Expression, Pattern } from 'estree' */ +/** @import { Expression, Pattern, Identifier } from 'estree' */ /** @import { AST } from '#compiler' */ /** @import { ComponentContext } from '../types' */ import { dev } from '../../../../state.js'; @@ -12,12 +12,16 @@ import { get_value } from './shared/declarations.js'; * @param {ComponentContext} context */ export function ConstTag(node, context) { + /** @type {Identifier} */ + let id; + const declaration = node.declaration.declarations[0]; // TODO we can almost certainly share some code with $derived(...) if (declaration.id.type === 'Identifier') { + id = declaration.id; context.state.init.push( b.const( - declaration.id, + id, create_derived( context.state, b.thunk(/** @type {Expression} */ (context.visit(declaration.init))) @@ -25,16 +29,12 @@ export function ConstTag(node, context) { ) ); - context.state.transform[declaration.id.name] = { read: get_value }; + context.state.transform[id.name] = { read: get_value }; - // we need to eagerly evaluate the expression in order to hit any - // 'Cannot access x before initialization' errors - if (dev) { - context.state.init.push(b.stmt(b.call('$.get', declaration.id))); - } + id = declaration.id; } else { const identifiers = extract_identifiers(declaration.id); - const tmp = b.id(context.state.scope.generate('computed_const')); + id = b.id(context.state.scope.generate('computed_const')); const transform = { ...context.state.transform }; @@ -59,18 +59,26 @@ export function ConstTag(node, context) { ]) ); - context.state.init.push(b.const(tmp, create_derived(context.state, fn))); - - // we need to eagerly evaluate the expression in order to hit any - // 'Cannot access x before initialization' errors - if (dev) { - context.state.init.push(b.stmt(b.call('$.get', tmp))); - } + context.state.init.push(b.const(id, create_derived(context.state, fn))); for (const node of identifiers) { context.state.transform[node.name] = { - read: (node) => b.member(b.call('$.get', tmp), node) + read: (node) => b.member(b.call('$.get', id), node) }; } } + + // we need to eagerly evaluate the expression in order to hit any + // 'Cannot access x before initialization' errors + if (dev) { + const parent = context.path.at(-1); + const boundary = parent?.type === 'SvelteBoundary' ? parent : null; + const statement = b.stmt(b.call('$.get', id)); + if (boundary) { + boundary.const_dev_statements ||= []; + boundary.const_dev_statements.push(statement); + } else { + context.state.init.push(statement); + } + } } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteBoundary.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteBoundary.js index 6da650a591d9..03bf43bd00cc 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteBoundary.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteBoundary.js @@ -1,6 +1,7 @@ /** @import { BlockStatement, Statement, Expression } from 'estree' */ /** @import { AST } from '#compiler' */ /** @import { ComponentContext } from '../types' */ +import { dev } from '../../../../state.js'; import * as b from '../../../../utils/builders.js'; /** @@ -63,6 +64,10 @@ export function SvelteBoundary(node, context) { const block = /** @type {BlockStatement} */ (context.visit({ ...node.fragment, nodes })); + if (dev && node.const_dev_statements) { + block.body.unshift(...node.const_dev_statements); + } + const boundary = b.stmt( b.call('$.boundary', context.state.node, props, b.arrow([b.id('$$anchor')], block)) ); diff --git a/packages/svelte/src/compiler/types/template.d.ts b/packages/svelte/src/compiler/types/template.d.ts index a544cd1dec09..101cdf273253 100644 --- a/packages/svelte/src/compiler/types/template.d.ts +++ b/packages/svelte/src/compiler/types/template.d.ts @@ -15,7 +15,8 @@ import type { Program, ChainExpression, SimpleCallExpression, - SequenceExpression + SequenceExpression, + ExpressionStatement } from 'estree'; import type { Scope } from '../phases/scope'; import type { _CSS } from './css'; @@ -275,6 +276,8 @@ export namespace AST { name: string; attributes: Array; fragment: Fragment; + /** Id */ + const_dev_statements?: ExpressionStatement[]; } export interface Component extends BaseElement { diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index 77d78477ee93..4a83437c97a4 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -622,7 +622,7 @@ declare module 'svelte/animate' { } declare module 'svelte/compiler' { - import type { Expression, Identifier, ArrayExpression, ArrowFunctionExpression, VariableDeclaration, VariableDeclarator, MemberExpression, Node, ObjectExpression, Pattern, Program, ChainExpression, SimpleCallExpression, SequenceExpression } from 'estree'; + import type { Expression, Identifier, ArrayExpression, ArrowFunctionExpression, VariableDeclaration, VariableDeclarator, MemberExpression, Node, ObjectExpression, Pattern, Program, ChainExpression, SimpleCallExpression, SequenceExpression, ExpressionStatement } from 'estree'; import type { SourceMap } from 'magic-string'; import type { Location } from 'locate-character'; /** @@ -1134,6 +1134,8 @@ declare module 'svelte/compiler' { name: string; attributes: Array; fragment: Fragment; + /** Id */ + const_dev_statements?: ExpressionStatement[]; } export interface Component extends BaseElement { From e5771c6897b7b504374bb71dd74342db950e4395 Mon Sep 17 00:00:00 2001 From: adiguba Date: Sat, 22 Feb 2025 09:35:28 +0100 Subject: [PATCH 2/9] changeset --- .changeset/sixty-cats-search.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/sixty-cats-search.md diff --git a/.changeset/sixty-cats-search.md b/.changeset/sixty-cats-search.md new file mode 100644 index 000000000000..26498b7fc1da --- /dev/null +++ b/.changeset/sixty-cats-search.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: catch error on @const tag in svelte:boundary From 2bbebb6dcf26e50fe8e8ccfaacb0dffe4c010c67 Mon Sep 17 00:00:00 2001 From: adiguba Date: Sat, 22 Feb 2025 09:36:23 +0100 Subject: [PATCH 3/9] edit changeset --- .changeset/sixty-cats-search.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/sixty-cats-search.md b/.changeset/sixty-cats-search.md index 26498b7fc1da..6ffd00749437 100644 --- a/.changeset/sixty-cats-search.md +++ b/.changeset/sixty-cats-search.md @@ -2,4 +2,4 @@ 'svelte': patch --- -fix: catch error on @const tag in svelte:boundary +fix: catch error on @const tag in svelte:boundary in DEV mode From a85e1a176be98f48cd5bb382e857281992bb8087 Mon Sep 17 00:00:00 2001 From: adiguba Date: Mon, 24 Feb 2025 23:39:19 +0100 Subject: [PATCH 4/9] test --- .../svelte-boundary-dev-const/_config.js | 33 +++++++++++++ .../svelte-boundary-dev-const/main.svelte | 49 +++++++++++++++++++ 2 files changed, 82 insertions(+) create mode 100644 packages/svelte/tests/runtime-runes/samples/svelte-boundary-dev-const/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/svelte-boundary-dev-const/main.svelte diff --git a/packages/svelte/tests/runtime-runes/samples/svelte-boundary-dev-const/_config.js b/packages/svelte/tests/runtime-runes/samples/svelte-boundary-dev-const/_config.js new file mode 100644 index 000000000000..3c0195ce3495 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/svelte-boundary-dev-const/_config.js @@ -0,0 +1,33 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +// https://github.com/sveltejs/svelte/issues/15368 +export default test({ + compileOptions: { + dev: true + }, + + mode: ['client'], + + html: ` +

BOOM

+

BOOM

+
OK
+
OK
+ `, + + async test({ target, assert, component }) { + component.ok = false; + flushSync(); + + assert.htmlEqual( + target.innerHTML, + ` +

BOOM

+

BOOM

+

BOOM

+

BOOM

+ ` + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/svelte-boundary-dev-const/main.svelte b/packages/svelte/tests/runtime-runes/samples/svelte-boundary-dev-const/main.svelte new file mode 100644 index 000000000000..c606a9bfb64b --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/svelte-boundary-dev-const/main.svelte @@ -0,0 +1,49 @@ + + + +
{throwError()}
+ + {#snippet failed()} +

BOOM

+ {/snippet} + + + + {@const result = throwError()} +
{result}
+ + {#snippet failed()} +

BOOM

+ {/snippet} + + + +
{throwErrorOnUpdate()}
+ + {#snippet failed()} +

BOOM

+ {/snippet} + + + + {@const result = throwErrorOnUpdate()} +
{result}
+ + {#snippet failed()} +

BOOM

+ {/snippet} + From 11c702295d6cb28c975b41d80d007aab21162c38 Mon Sep 17 00:00:00 2001 From: adiguba Date: Tue, 25 Feb 2025 00:08:21 +0100 Subject: [PATCH 5/9] fix --- .../runtime-runes/samples/svelte-boundary-dev-const/main.svelte | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/tests/runtime-runes/samples/svelte-boundary-dev-const/main.svelte b/packages/svelte/tests/runtime-runes/samples/svelte-boundary-dev-const/main.svelte index c606a9bfb64b..30e074c762f6 100644 --- a/packages/svelte/tests/runtime-runes/samples/svelte-boundary-dev-const/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/svelte-boundary-dev-const/main.svelte @@ -9,7 +9,7 @@ if (ok) { return "OK"; } else { - fail(); + throwError(); } } From af46902b5a16bd1e469bdc1f814d8f37e24accf3 Mon Sep 17 00:00:00 2001 From: adiguba Date: Tue, 25 Feb 2025 00:08:34 +0100 Subject: [PATCH 6/9] rollback --- .../3-transform/client/visitors/ConstTag.js | 42 ++++++++----------- packages/svelte/types/index.d.ts | 4 +- 2 files changed, 18 insertions(+), 28 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ConstTag.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ConstTag.js index 45396108491c..7e33aea4356b 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ConstTag.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ConstTag.js @@ -1,4 +1,4 @@ -/** @import { Expression, Pattern, Identifier } from 'estree' */ +/** @import { Expression, Pattern } from 'estree' */ /** @import { AST } from '#compiler' */ /** @import { ComponentContext } from '../types' */ import { dev } from '../../../../state.js'; @@ -12,16 +12,12 @@ import { get_value } from './shared/declarations.js'; * @param {ComponentContext} context */ export function ConstTag(node, context) { - /** @type {Identifier} */ - let id; - const declaration = node.declaration.declarations[0]; // TODO we can almost certainly share some code with $derived(...) if (declaration.id.type === 'Identifier') { - id = declaration.id; context.state.init.push( b.const( - id, + declaration.id, create_derived( context.state, b.thunk(/** @type {Expression} */ (context.visit(declaration.init))) @@ -29,12 +25,16 @@ export function ConstTag(node, context) { ) ); - context.state.transform[id.name] = { read: get_value }; + context.state.transform[declaration.id.name] = { read: get_value }; - id = declaration.id; + // we need to eagerly evaluate the expression in order to hit any + // 'Cannot access x before initialization' errors + if (dev) { + context.state.init.push(b.stmt(b.call('$.get', declaration.id))); + } } else { const identifiers = extract_identifiers(declaration.id); - id = b.id(context.state.scope.generate('computed_const')); + const tmp = b.id(context.state.scope.generate('computed_const')); const transform = { ...context.state.transform }; @@ -59,26 +59,18 @@ export function ConstTag(node, context) { ]) ); - context.state.init.push(b.const(id, create_derived(context.state, fn))); + context.state.init.push(b.const(tmp, create_derived(context.state, fn))); + + // we need to eagerly evaluate the expression in order to hit any + // 'Cannot access x before initialization' errors + if (dev) { + context.state.init.push(b.stmt(b.call('$.get', tmp))); + } for (const node of identifiers) { context.state.transform[node.name] = { - read: (node) => b.member(b.call('$.get', id), node) + read: (node) => b.member(b.call('$.get', tmp), node) }; } } - - // we need to eagerly evaluate the expression in order to hit any - // 'Cannot access x before initialization' errors - if (dev) { - const parent = context.path.at(-1); - const boundary = parent?.type === 'SvelteBoundary' ? parent : null; - const statement = b.stmt(b.call('$.get', id)); - if (boundary) { - boundary.const_dev_statements ||= []; - boundary.const_dev_statements.push(statement); - } else { - context.state.init.push(statement); - } - } } diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index 4a83437c97a4..77d78477ee93 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -622,7 +622,7 @@ declare module 'svelte/animate' { } declare module 'svelte/compiler' { - import type { Expression, Identifier, ArrayExpression, ArrowFunctionExpression, VariableDeclaration, VariableDeclarator, MemberExpression, Node, ObjectExpression, Pattern, Program, ChainExpression, SimpleCallExpression, SequenceExpression, ExpressionStatement } from 'estree'; + import type { Expression, Identifier, ArrayExpression, ArrowFunctionExpression, VariableDeclaration, VariableDeclarator, MemberExpression, Node, ObjectExpression, Pattern, Program, ChainExpression, SimpleCallExpression, SequenceExpression } from 'estree'; import type { SourceMap } from 'magic-string'; import type { Location } from 'locate-character'; /** @@ -1134,8 +1134,6 @@ declare module 'svelte/compiler' { name: string; attributes: Array; fragment: Fragment; - /** Id */ - const_dev_statements?: ExpressionStatement[]; } export interface Component extends BaseElement { From 5b73cc8a71f1f34fa9fc974185ccd46301456c8c Mon Sep 17 00:00:00 2001 From: adiguba Date: Tue, 25 Feb 2025 00:09:15 +0100 Subject: [PATCH 7/9] simpler way to do that --- .../client/visitors/SvelteBoundary.js | 22 ++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteBoundary.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteBoundary.js index 03bf43bd00cc..9228df970375 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteBoundary.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteBoundary.js @@ -36,6 +36,9 @@ export function SvelteBoundary(node, context) { /** @type {Statement[]} */ const external_statements = []; + /** @type {Statement[]} */ + const internal_statements = []; + const snippets_visits = []; // Capture the `failed` implicit snippet prop @@ -54,7 +57,20 @@ export function SvelteBoundary(node, context) { /** @type {Statement[]} */ const init = []; context.visit(child, { ...context.state, init }); - external_statements.push(...init); + + if (dev) { + // In dev we must separate the declarations from the code + // that eagerly evaluate the expression... + for (const statement of init) { + if (statement.type === 'VariableDeclaration') { + external_statements.push(statement); + } else { + internal_statements.push(statement); + } + } + } else { + external_statements.push(...init); + } } else { nodes.push(child); } @@ -64,8 +80,8 @@ export function SvelteBoundary(node, context) { const block = /** @type {BlockStatement} */ (context.visit({ ...node.fragment, nodes })); - if (dev && node.const_dev_statements) { - block.body.unshift(...node.const_dev_statements); + if (dev && internal_statements.length) { + block.body.unshift(...internal_statements); } const boundary = b.stmt( From 4f346bfd6489701b10593b919c0b4dead258364d Mon Sep 17 00:00:00 2001 From: adiguba Date: Tue, 25 Feb 2025 00:18:43 +0100 Subject: [PATCH 8/9] rollback --- packages/svelte/src/compiler/types/template.d.ts | 5 +---- packages/svelte/types/index.d.ts | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/svelte/src/compiler/types/template.d.ts b/packages/svelte/src/compiler/types/template.d.ts index 101cdf273253..a544cd1dec09 100644 --- a/packages/svelte/src/compiler/types/template.d.ts +++ b/packages/svelte/src/compiler/types/template.d.ts @@ -15,8 +15,7 @@ import type { Program, ChainExpression, SimpleCallExpression, - SequenceExpression, - ExpressionStatement + SequenceExpression } from 'estree'; import type { Scope } from '../phases/scope'; import type { _CSS } from './css'; @@ -276,8 +275,6 @@ export namespace AST { name: string; attributes: Array; fragment: Fragment; - /** Id */ - const_dev_statements?: ExpressionStatement[]; } export interface Component extends BaseElement { diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index 4cb66aa87d30..18891b593328 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -623,8 +623,8 @@ declare module 'svelte/animate' { } declare module 'svelte/compiler' { - import type { Expression, Identifier, ArrayExpression, ArrowFunctionExpression, VariableDeclaration, VariableDeclarator, MemberExpression, Node, ObjectExpression, Pattern, Program, ChainExpression, SimpleCallExpression, SequenceExpression } from 'estree'; import type { SourceMap } from 'magic-string'; + import type { Expression, Identifier, ArrayExpression, ArrowFunctionExpression, VariableDeclaration, VariableDeclarator, MemberExpression, Node, ObjectExpression, Pattern, Program, ChainExpression, SimpleCallExpression, SequenceExpression } from 'estree'; import type { Location } from 'locate-character'; /** * `compile` converts your `.svelte` source code into a JavaScript module that exports a component From 0ab422b90fdc6ae091a11fe87fa4ec662388735e Mon Sep 17 00:00:00 2001 From: adiguba Date: Tue, 25 Feb 2025 00:19:53 +0100 Subject: [PATCH 9/9] roolback again --- packages/svelte/types/index.d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index 18891b593328..4c47661af897 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -624,7 +624,7 @@ declare module 'svelte/animate' { declare module 'svelte/compiler' { import type { SourceMap } from 'magic-string'; - import type { Expression, Identifier, ArrayExpression, ArrowFunctionExpression, VariableDeclaration, VariableDeclarator, MemberExpression, Node, ObjectExpression, Pattern, Program, ChainExpression, SimpleCallExpression, SequenceExpression } from 'estree'; + import type { ArrayExpression, ArrowFunctionExpression, VariableDeclaration, VariableDeclarator, Expression, Identifier, MemberExpression, Node, ObjectExpression, Pattern, Program, ChainExpression, SimpleCallExpression, SequenceExpression } from 'estree'; import type { Location } from 'locate-character'; /** * `compile` converts your `.svelte` source code into a JavaScript module that exports a component