Skip to content

Commit 6e863e6

Browse files
authored
feat: warn on referenced mutated nonstate (#9669)
Walk the path and warn if this is a mutated normal variable that's referenced inside a function scope
1 parent 9c44fd7 commit 6e863e6

File tree

8 files changed

+80
-9
lines changed

8 files changed

+80
-9
lines changed

.changeset/friendly-lies-camp.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'svelte': patch
3+
---
4+
5+
feat: warn on references to mutated non-state in template

packages/svelte/src/compiler/phases/2-analyze/index.js

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ export function analyze_module(ast, options) {
218218
for (const [, scope] of scopes) {
219219
for (const [name, binding] of scope.declarations) {
220220
if (binding.kind === 'state' && !binding.mutated) {
221-
warn(warnings, binding.node, [], 'state-rune-not-mutated', name);
221+
warn(warnings, binding.node, [], 'state-not-mutated', name);
222222
}
223223
}
224224
}
@@ -377,7 +377,7 @@ export function analyze_component(root, options) {
377377
for (const [, scope] of instance.scopes) {
378378
for (const [name, binding] of scope.declarations) {
379379
if (binding.kind === 'state' && !binding.mutated) {
380-
warn(warnings, binding.node, [], 'state-rune-not-mutated', name);
380+
warn(warnings, binding.node, [], 'state-not-mutated', name);
381381
}
382382
}
383383
}
@@ -414,6 +414,30 @@ export function analyze_component(root, options) {
414414
analysis.reactive_statements = order_reactive_statements(analysis.reactive_statements);
415415
}
416416

417+
// warn on any nonstate declarations that are a) mutated and b) referenced in the template
418+
for (const scope of [module.scope, instance.scope]) {
419+
outer: for (const [name, binding] of scope.declarations) {
420+
if (binding.kind === 'normal' && binding.mutated) {
421+
for (const { path } of binding.references) {
422+
if (path[0].type !== 'Fragment') continue;
423+
for (let i = 1; i < path.length; i += 1) {
424+
const type = path[i].type;
425+
if (
426+
type === 'FunctionDeclaration' ||
427+
type === 'FunctionExpression' ||
428+
type === 'ArrowFunctionExpression'
429+
) {
430+
continue;
431+
}
432+
}
433+
434+
warn(warnings, binding.node, [], 'non-state-reference', name);
435+
continue outer;
436+
}
437+
}
438+
}
439+
}
440+
417441
analysis.stylesheet.validate(analysis);
418442

419443
for (const element of analysis.elements) {

packages/svelte/src/compiler/phases/scope.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -175,13 +175,13 @@ export class Scope {
175175

176176
references.push({ node, path });
177177

178-
const declaration = this.declarations.get(node.name);
179-
if (declaration) {
180-
declaration.references.push({ node, path });
178+
const binding = this.declarations.get(node.name);
179+
if (binding) {
180+
binding.references.push({ node, path });
181181
} else if (this.#parent) {
182182
this.#parent.reference(node, path);
183183
} else {
184-
// no declaration was found, and this is the top level scope,
184+
// no binding was found, and this is the top level scope,
185185
// which means this is a global
186186
this.root.conflicts.add(node.name);
187187
}

packages/svelte/src/compiler/warnings.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,11 @@ const runes = {
2222
`It looks like you're using the $${name} rune, but there is a local binding called ${name}. ` +
2323
`Referencing a local variable with a $ prefix will create a store subscription. Please rename ${name} to avoid the ambiguity.`,
2424
/** @param {string} name */
25-
'state-rune-not-mutated': (name) =>
26-
`${name} is declared with $state(...) but is never updated. Did you mean to create a function that changes its value?`
25+
'state-not-mutated': (name) =>
26+
`${name} is declared with $state(...) but is never updated. Did you mean to create a function that changes its value?`,
27+
/** @param {string} name */
28+
'non-state-reference': (name) =>
29+
`${name} is updated, but is not declared with $state(...). Changing its value will not correctly trigger updates.`
2730
};
2831

2932
/** @satisfies {Warnings} */
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import { test } from '../../test';
2+
3+
export default test({});
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<script>
2+
let a = $state(1);
3+
let b = 2;
4+
let c = 3;
5+
</script>
6+
7+
<button onclick={() => a += 1}>a += 1</button>
8+
<button onclick={() => b += 1}>b += 1</button>
9+
<button onclick={() => c += 1}>c += 1</button>
10+
<p>{a} + {b} + {c} = {a + b + c}</p>
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
[
2+
{
3+
"code": "non-state-reference",
4+
"message": "b is updated, but is not declared with $state(...). Changing its value will not correctly trigger updates.",
5+
"start": {
6+
"column": 5,
7+
"line": 3
8+
},
9+
"end": {
10+
"column": 6,
11+
"line": 3
12+
}
13+
},
14+
{
15+
"code": "non-state-reference",
16+
"message": "c is updated, but is not declared with $state(...). Changing its value will not correctly trigger updates.",
17+
"start": {
18+
"column": 5,
19+
"line": 4
20+
},
21+
"end": {
22+
"column": 6,
23+
"line": 4
24+
}
25+
}
26+
]

packages/svelte/tests/validator/samples/runes-state-rune-not-mutated/warnings.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[
22
{
3-
"code": "state-rune-not-mutated",
3+
"code": "state-not-mutated",
44
"end": {
55
"column": 11,
66
"line": 3

0 commit comments

Comments
 (0)