From 01d32e53a9f445280fb498c9069d998840503c00 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Wed, 21 Aug 2024 22:17:55 +0200 Subject: [PATCH] fix: more robust handling of var declarations (#12949) * fix: more robust handling of var declarations - fixes #12807: state declared with var is now retrieved using a new `safe_get` function, which works like `get` but checks for undefined - fixes #12900: ensure we're not creating another new scope for block statements of function declarations, which resulted in var declarations getting "swallowed" (because they were hoisted to the function declaration scope and never seen by our logic) * simplify --------- Co-authored-by: Rich Harris --- .changeset/spotty-trees-provide.md | 5 ++++ .../client/visitors/VariableDeclaration.js | 2 +- .../client/visitors/shared/declarations.js | 2 +- packages/svelte/src/compiler/phases/scope.js | 14 ++++++++++- packages/svelte/src/internal/client/index.js | 1 + .../svelte/src/internal/client/runtime.js | 10 ++++++++ .../samples/var-declarations/_config.js | 7 ++++++ .../samples/var-declarations/main.svelte | 24 +++++++++++++++++++ 8 files changed, 62 insertions(+), 3 deletions(-) create mode 100644 .changeset/spotty-trees-provide.md create mode 100644 packages/svelte/tests/runtime-runes/samples/var-declarations/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/var-declarations/main.svelte diff --git a/.changeset/spotty-trees-provide.md b/.changeset/spotty-trees-provide.md new file mode 100644 index 0000000000..0652fda049 --- /dev/null +++ b/.changeset/spotty-trees-provide.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: more robust handling of var declarations diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js index 77177e397f..c05c1b6075 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js @@ -284,7 +284,7 @@ export function VariableDeclaration(node, context) { } /** - * Creates the output for a state declaration. + * Creates the output for a state declaration in legacy mode. * @param {VariableDeclarator} declarator * @param {Scope} scope * @param {Expression} value diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/declarations.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/declarations.js index 33566f1c10..0bd8c352f6 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/declarations.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/declarations.js @@ -23,7 +23,7 @@ export function add_state_transformers(context) { binding.kind === 'legacy_reactive' ) { context.state.transform[name] = { - read: get_value, + read: binding.declaration_kind === 'var' ? (node) => b.call('$.safe_get', node) : get_value, assign: (node, value) => { let call = b.call('$.set', node, value); diff --git a/packages/svelte/src/compiler/phases/scope.js b/packages/svelte/src/compiler/phases/scope.js index 66a49f87c7..f0db435b83 100644 --- a/packages/svelte/src/compiler/phases/scope.js +++ b/packages/svelte/src/compiler/phases/scope.js @@ -461,8 +461,20 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) { ForStatement: create_block_scope, ForInStatement: create_block_scope, ForOfStatement: create_block_scope, - BlockStatement: create_block_scope, SwitchStatement: create_block_scope, + BlockStatement(node, context) { + const parent = context.path.at(-1); + if ( + parent?.type === 'FunctionDeclaration' || + parent?.type === 'FunctionExpression' || + parent?.type === 'ArrowFunctionExpression' + ) { + // We already created a new scope for the function + context.next(); + } else { + create_block_scope(node, context); + } + }, ClassDeclaration(node, { state, next }) { if (node.id) state.scope.declare(node.id, 'normal', 'let', node); diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index 1ca71708af..51b9705902 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -127,6 +127,7 @@ export { export { set_text } from './render.js'; export { get, + safe_get, invalidate_inner_signals, flush_sync, tick, diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index b1a52b0878..f22a6c15a4 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -734,6 +734,16 @@ export function get(signal) { return signal.v; } +/** + * Like `get`, but checks for `undefined`. Used for `var` declarations because they can be accessed before being declared + * @template V + * @param {Value | undefined} signal + * @returns {V | undefined} + */ +export function safe_get(signal) { + return signal && get(signal); +} + /** * Invokes a function and captures all signals that are read during the invocation, * then invalidates them. diff --git a/packages/svelte/tests/runtime-runes/samples/var-declarations/_config.js b/packages/svelte/tests/runtime-runes/samples/var-declarations/_config.js new file mode 100644 index 0000000000..46349dd2dc --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/var-declarations/_config.js @@ -0,0 +1,7 @@ +import { test } from '../../test'; + +export default test({ + test({ assert, logs }) { + assert.deepEqual(logs, [undefined, undefined, 10, 20, 0, 1]); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/var-declarations/main.svelte b/packages/svelte/tests/runtime-runes/samples/var-declarations/main.svelte new file mode 100644 index 0000000000..8ed48fcbed --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/var-declarations/main.svelte @@ -0,0 +1,24 @@ +