diff --git a/.changeset/friendly-lies-camp.md b/.changeset/friendly-lies-camp.md
new file mode 100644
index 000000000000..fe889998686c
--- /dev/null
+++ b/.changeset/friendly-lies-camp.md
@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+feat: warn on references to mutated non-state in template
diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js
index 68f3ac599ffb..8acea30c95d7 100644
--- a/packages/svelte/src/compiler/phases/2-analyze/index.js
+++ b/packages/svelte/src/compiler/phases/2-analyze/index.js
@@ -38,7 +38,7 @@ function js(script, root, allow_reactive_declarations, parent) {
body: []
};
- const { scope, scopes } = create_scopes(ast, root, allow_reactive_declarations, parent);
+ const { scope, scopes } = create_scopes(ast, root, allow_reactive_declarations, false, parent);
return { ast, scope, scopes };
}
@@ -191,7 +191,7 @@ function get_delegated_event(node, context) {
* @returns {import('../types.js').Analysis}
*/
export function analyze_module(ast, options) {
- const { scope, scopes } = create_scopes(ast, new ScopeRoot(), false, null);
+ const { scope, scopes } = create_scopes(ast, new ScopeRoot(), false, false, null);
for (const [name, references] of scope.references) {
if (name[0] !== '$' || ReservedKeywords.includes(name)) continue;
@@ -218,7 +218,7 @@ export function analyze_module(ast, options) {
for (const [, scope] of scopes) {
for (const [name, binding] of scope.declarations) {
if (binding.kind === 'state' && !binding.mutated) {
- warn(warnings, binding.node, [], 'state-rune-not-mutated', name);
+ warn(warnings, binding.node, [], 'state-not-mutated', name);
}
}
}
@@ -242,7 +242,7 @@ export function analyze_component(root, options) {
const module = js(root.module, scope_root, false, null);
const instance = js(root.instance, scope_root, true, module.scope);
- const { scope, scopes } = create_scopes(root.fragment, scope_root, false, instance.scope);
+ const { scope, scopes } = create_scopes(root.fragment, scope_root, false, true, instance.scope);
/** @type {import('../types.js').Template} */
const template = { ast: root.fragment, scope, scopes };
@@ -377,7 +377,7 @@ export function analyze_component(root, options) {
for (const [, scope] of instance.scopes) {
for (const [name, binding] of scope.declarations) {
if (binding.kind === 'state' && !binding.mutated) {
- warn(warnings, binding.node, [], 'state-rune-not-mutated', name);
+ warn(warnings, binding.node, [], 'state-not-mutated', name);
}
}
}
@@ -414,6 +414,15 @@ export function analyze_component(root, options) {
analysis.reactive_statements = order_reactive_statements(analysis.reactive_statements);
}
+ // warn on any nonstate declarations that are a) mutated and b) referenced in the template
+ for (const scope of [module.scope, instance.scope]) {
+ for (const [name, binding] of scope.declarations) {
+ if (binding.kind === 'normal' && binding.mutated && binding.referenced_in_template) {
+ warn(warnings, binding.node, [], 'non-state-reference', name);
+ }
+ }
+ }
+
analysis.stylesheet.validate(analysis);
for (const element of analysis.elements) {
diff --git a/packages/svelte/src/compiler/phases/scope.js b/packages/svelte/src/compiler/phases/scope.js
index 86d0eab67a58..2143f4660cf9 100644
--- a/packages/svelte/src/compiler/phases/scope.js
+++ b/packages/svelte/src/compiler/phases/scope.js
@@ -91,6 +91,7 @@ export class Scope {
const binding = {
node,
references: [],
+ referenced_in_template: false,
legacy_dependencies: [],
initial,
mutated: false,
@@ -168,20 +169,22 @@ export class Scope {
/**
* @param {import('estree').Identifier} node
* @param {import('#compiler').SvelteNode[]} path
+ * @param {boolean} is_template
*/
- reference(node, path) {
+ reference(node, path, is_template) {
let references = this.references.get(node.name);
if (!references) this.references.set(node.name, (references = []));
references.push({ node, path });
- const declaration = this.declarations.get(node.name);
- if (declaration) {
- declaration.references.push({ node, path });
+ const binding = this.declarations.get(node.name);
+ if (binding) {
+ if (is_template) binding.referenced_in_template = true;
+ binding.references.push({ node, path });
} else if (this.#parent) {
- this.#parent.reference(node, path);
+ this.#parent.reference(node, path, is_template);
} else {
- // no declaration was found, and this is the top level scope,
+ // no binding was found, and this is the top level scope,
// which means this is a global
this.root.conflicts.add(node.name);
}
@@ -214,10 +217,11 @@ export class ScopeRoot {
* @param {import('#compiler').SvelteNode} ast
* @param {ScopeRoot} root
* @param {boolean} allow_reactive_declarations
+ * @param {boolean} is_template
* @param {Scope | null} parent
*/
-export function create_scopes(ast, root, allow_reactive_declarations, parent) {
- /** @typedef {{ scope: Scope }} State */
+export function create_scopes(ast, root, allow_reactive_declarations, is_template, parent) {
+ /** @typedef {{ scope: Scope, is_template: boolean }} State */
/**
* A map of node->associated scope. A node appearing in this map does not necessarily mean that it created a scope
@@ -228,9 +232,9 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
scopes.set(ast, scope);
/** @type {State} */
- const state = { scope };
+ const state = { scope, is_template };
- /** @type {[Scope, { node: import('estree').Identifier; path: import('#compiler').SvelteNode[] }][]} */
+ /** @type {[Scope, { node: import('estree').Identifier; path: import('#compiler').SvelteNode[]; is_template: boolean }][]} */
const references = [];
/** @type {[Scope, import('estree').Pattern | import('estree').MemberExpression][]} */
@@ -261,7 +265,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
const scope = state.scope.child(true);
scopes.set(node, scope);
- next({ scope });
+ next({ ...state, scope });
};
/**
@@ -270,7 +274,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
const SvelteFragment = (node, { state, next }) => {
const scope = analyze_let_directives(node, state.scope);
scopes.set(node, scope);
- next({ scope });
+ next({ ...state, scope });
};
/**
@@ -316,7 +320,10 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
Identifier(node, { path, state }) {
const parent = path.at(-1);
if (parent && is_reference(node, /** @type {import('estree').Node} */ (parent))) {
- references.push([state.scope, { node, path: path.slice() }]);
+ references.push([
+ state.scope,
+ { node, path: path.slice(), is_template: state.is_template }
+ ]);
}
},
@@ -339,7 +346,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
}
}
- next({ scope });
+ next({ ...state, scope });
},
SvelteFragment,
@@ -347,7 +354,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
RegularElement: SvelteFragment,
Component(node, { state, visit, path }) {
- state.scope.reference(b.id(node.name), path);
+ state.scope.reference(b.id(node.name), path, false);
// let:x from the default slot is a weird one:
// Its scope only applies to children that are not slots themselves.
@@ -371,7 +378,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
} else if (child.type === 'SnippetBlock') {
visit(child);
} else {
- visit(child, { scope });
+ visit(child, { ...state, scope });
}
}
},
@@ -405,7 +412,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
if (node.id) scope.declare(node.id, 'normal', 'function');
add_params(scope, node.params);
- next({ scope });
+ next({ scope, is_template: false });
},
FunctionDeclaration(node, { state, next }) {
@@ -415,7 +422,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
scopes.set(node, scope);
add_params(scope, node.params);
- next({ scope });
+ next({ scope, is_template: false });
},
ArrowFunctionExpression(node, { state, next }) {
@@ -423,7 +430,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
scopes.set(node, scope);
add_params(scope, node.params);
- next({ scope });
+ next({ scope, is_template: false });
},
ForStatement: create_block_scope,
@@ -468,7 +475,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
state.scope.declare(id, 'normal', 'let');
}
- next({ scope });
+ next({ ...state, scope });
} else {
next();
}
@@ -506,13 +513,13 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
(node.key.type !== 'Identifier' || !node.index || node.key.name !== node.index);
scope.declare(b.id(node.index), is_keyed ? 'derived' : 'normal', 'const');
}
- if (node.key) visit(node.key, { scope });
+ if (node.key) visit(node.key, { ...state, scope });
// children
for (const child of node.body.nodes) {
- visit(child, { scope });
+ visit(child, { ...state, scope });
}
- if (node.fallback) visit(node.fallback, { scope });
+ if (node.fallback) visit(node.fallback, { ...state, scope });
// Check if inner scope shadows something from outer scope.
// This is necessary because we need access to the array expression of the each block
@@ -579,13 +586,13 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
}
}
- context.next({ scope: child_scope });
+ context.next({ ...state, scope: child_scope });
},
Fragment: (node, context) => {
const scope = context.state.scope.child(node.transparent);
scopes.set(node, scope);
- context.next({ scope });
+ context.next({ ...state, scope });
},
BindDirective(node, context) {
@@ -622,8 +629,8 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
// we do this after the fact, so that we don't need to worry
// about encountering references before their declarations
- for (const [scope, { node, path }] of references) {
- scope.reference(node, path);
+ for (const [scope, { node, path, is_template }] of references) {
+ scope.reference(node, path, is_template);
}
for (const [scope, node] of updates) {
diff --git a/packages/svelte/src/compiler/types/index.d.ts b/packages/svelte/src/compiler/types/index.d.ts
index 2c5480eb7f58..04a6c08c9c09 100644
--- a/packages/svelte/src/compiler/types/index.d.ts
+++ b/packages/svelte/src/compiler/types/index.d.ts
@@ -268,6 +268,7 @@ export interface Binding {
initial: null | Expression | FunctionDeclaration | ClassDeclaration | ImportDeclaration;
is_called: boolean;
references: { node: Identifier; path: SvelteNode[] }[];
+ referenced_in_template: boolean;
mutated: boolean;
scope: Scope;
/** For `legacy_reactive`: its reactive dependencies */
diff --git a/packages/svelte/src/compiler/warnings.js b/packages/svelte/src/compiler/warnings.js
index 23e9378c25cd..6c38b1e052a7 100644
--- a/packages/svelte/src/compiler/warnings.js
+++ b/packages/svelte/src/compiler/warnings.js
@@ -22,8 +22,11 @@ const runes = {
`It looks like you're using the $${name} rune, but there is a local binding called ${name}. ` +
`Referencing a local variable with a $ prefix will create a store subscription. Please rename ${name} to avoid the ambiguity.`,
/** @param {string} name */
- 'state-rune-not-mutated': (name) =>
- `${name} is declared with $state(...) but is never updated. Did you mean to create a function that changes its value?`
+ 'state-not-mutated': (name) =>
+ `${name} is declared with $state(...) but is never updated. Did you mean to create a function that changes its value?`,
+ /** @param {string} name */
+ 'non-state-reference': (name) =>
+ `${name} is updated, but is not declared with $state(...). Changing its value will not correctly trigger updates.`
};
/** @satisfies {Warnings} */
diff --git a/packages/svelte/tests/validator/samples/runes-referenced-nonstate/_config.js b/packages/svelte/tests/validator/samples/runes-referenced-nonstate/_config.js
new file mode 100644
index 000000000000..f47bee71df87
--- /dev/null
+++ b/packages/svelte/tests/validator/samples/runes-referenced-nonstate/_config.js
@@ -0,0 +1,3 @@
+import { test } from '../../test';
+
+export default test({});
diff --git a/packages/svelte/tests/validator/samples/runes-referenced-nonstate/input.svelte b/packages/svelte/tests/validator/samples/runes-referenced-nonstate/input.svelte
new file mode 100644
index 000000000000..279cce71098e
--- /dev/null
+++ b/packages/svelte/tests/validator/samples/runes-referenced-nonstate/input.svelte
@@ -0,0 +1,8 @@
+
+
+
+
+{a} + {b} = {a + b}
diff --git a/packages/svelte/tests/validator/samples/runes-referenced-nonstate/warnings.json b/packages/svelte/tests/validator/samples/runes-referenced-nonstate/warnings.json
new file mode 100644
index 000000000000..e42a8d8782b9
--- /dev/null
+++ b/packages/svelte/tests/validator/samples/runes-referenced-nonstate/warnings.json
@@ -0,0 +1,14 @@
+[
+ {
+ "code": "non-state-reference",
+ "message": "b is updated, but is not declared with $state(...). Changing its value will not correctly trigger updates.",
+ "start": {
+ "column": 5,
+ "line": 3
+ },
+ "end": {
+ "column": 6,
+ "line": 3
+ }
+ }
+]
diff --git a/packages/svelte/tests/validator/samples/runes-state-rune-not-mutated/warnings.json b/packages/svelte/tests/validator/samples/runes-state-rune-not-mutated/warnings.json
index 628f1f2e9d00..5d2b639c8d43 100644
--- a/packages/svelte/tests/validator/samples/runes-state-rune-not-mutated/warnings.json
+++ b/packages/svelte/tests/validator/samples/runes-state-rune-not-mutated/warnings.json
@@ -1,6 +1,6 @@
[
{
- "code": "state-rune-not-mutated",
+ "code": "state-not-mutated",
"end": {
"column": 11,
"line": 3