Skip to content

Commit

Permalink
fix: more robust handling of var declarations
Browse files Browse the repository at this point in the history
- 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)
  • Loading branch information
dummdidumm committed Aug 21, 2024
1 parent af35fb7 commit e0c73ab
Show file tree
Hide file tree
Showing 8 changed files with 65 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 @@ -296,7 +296,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
13 changes: 13 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,19 @@ 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) {
if (!signal) {
return undefined;
}
return 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 e0c73ab

Please sign in to comment.