Skip to content

Commit

Permalink
fix: more robust handling of var declarations (#12949)
Browse files Browse the repository at this point in the history
* 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 <rich.harris@vercel.com>
  • Loading branch information
dummdidumm and Rich-Harris authored Aug 21, 2024
1 parent 70ed8cd commit 01d32e5
Show file tree
Hide file tree
Showing 8 changed files with 62 additions and 3 deletions.
5 changes: 5 additions & 0 deletions .changeset/spotty-trees-provide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: more robust handling of var declarations
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
14 changes: 13 additions & 1 deletion packages/svelte/src/compiler/phases/scope.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions packages/svelte/src/internal/client/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ export {
export { set_text } from './render.js';
export {
get,
safe_get,
invalidate_inner_signals,
flush_sync,
tick,
Expand Down
10 changes: 10 additions & 0 deletions packages/svelte/src/internal/client/runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<V> | 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.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { test } from '../../test';

export default test({
test({ assert, logs }) {
assert.deepEqual(logs, [undefined, undefined, 10, 20, 0, 1]);
}
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<script>
console.log(foo, double);
var foo = $state(10);
var double = $derived(foo * 2);
console.log(foo, double);
function wrap(initial) {
var _value = $state(initial);
return {
get() {
return _value;
},
set(state) {
_value = state;
}
};
}
var wrapped = wrap(0);
console.log(wrapped.get());
wrapped.set(1);
console.log(wrapped.get());
</script>

0 comments on commit 01d32e5

Please sign in to comment.