From 57e1c08af27ef3ff8de3a2c21edd9a5efa2492c4 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 27 Nov 2023 09:24:03 -0500 Subject: [PATCH 1/6] warn on nonstate references --- .../src/compiler/phases/2-analyze/index.js | 15 ++++- packages/svelte/src/compiler/phases/scope.js | 61 +++++++++++-------- packages/svelte/src/compiler/types/index.d.ts | 1 + packages/svelte/src/compiler/warnings.js | 5 +- .../runes-referenced-nonstate/_config.js | 3 + .../runes-referenced-nonstate/input.svelte | 8 +++ .../runes-referenced-nonstate/warnings.json | 14 +++++ 7 files changed, 76 insertions(+), 31 deletions(-) create mode 100644 packages/svelte/tests/validator/samples/runes-referenced-nonstate/_config.js create mode 100644 packages/svelte/tests/validator/samples/runes-referenced-nonstate/input.svelte create mode 100644 packages/svelte/tests/validator/samples/runes-referenced-nonstate/warnings.json diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index 68f3ac599ffb..683aacf09117 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; @@ -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 }; @@ -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..f8a2cb061b4b 100644 --- a/packages/svelte/src/compiler/warnings.js +++ b/packages/svelte/src/compiler/warnings.js @@ -23,7 +23,10 @@ const runes = { `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?` + `${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 + } + } +] From f01a15fd05c271fe810afe71dfae038cde9580bd Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 27 Nov 2023 09:25:09 -0500 Subject: [PATCH 2/6] more correct error code - it's the state that isn't mutated, not the rune --- packages/svelte/src/compiler/phases/2-analyze/index.js | 4 ++-- packages/svelte/src/compiler/warnings.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index 683aacf09117..8acea30c95d7 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -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); } } } @@ -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); } } } diff --git a/packages/svelte/src/compiler/warnings.js b/packages/svelte/src/compiler/warnings.js index f8a2cb061b4b..6c38b1e052a7 100644 --- a/packages/svelte/src/compiler/warnings.js +++ b/packages/svelte/src/compiler/warnings.js @@ -22,7 +22,7 @@ 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) => + '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) => From 45dc56b37d92f281a04c8ebbaed1d4bc76bb3197 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 27 Nov 2023 09:25:32 -0500 Subject: [PATCH 3/6] changeset --- .changeset/friendly-lies-camp.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/friendly-lies-camp.md 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 From 289d568357bc7351e788ba1da236c4f83616fba7 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 27 Nov 2023 09:40:19 -0500 Subject: [PATCH 4/6] less invasive approach --- .../src/compiler/phases/2-analyze/index.js | 27 +++++++--- packages/svelte/src/compiler/phases/scope.js | 52 ++++++++----------- .../warnings.json | 2 +- 3 files changed, 45 insertions(+), 36 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index 8acea30c95d7..540a1b99129c 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, false, parent); + const { scope, scopes } = create_scopes(ast, root, allow_reactive_declarations, 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, false, null); + const { scope, scopes } = create_scopes(ast, new ScopeRoot(), false, null); for (const [name, references] of scope.references) { if (name[0] !== '$' || ReservedKeywords.includes(name)) continue; @@ -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, true, instance.scope); + const { scope, scopes } = create_scopes(root.fragment, scope_root, false, instance.scope); /** @type {import('../types.js').Template} */ const template = { ast: root.fragment, scope, scopes }; @@ -416,9 +416,24 @@ export function analyze_component(root, options) { // 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); + outer: for (const [name, binding] of scope.declarations) { + if (binding.kind === 'normal' && binding.mutated) { + for (const { path } of binding.references) { + if (path[0].type !== 'Fragment') continue; + for (let i = 1; i < path.length; i += 1) { + const type = path[i].type; + if ( + type === 'FunctionDeclaration' || + type === 'FunctionExpression' || + type === 'ArrowFunctionExpression' + ) { + continue; + } + } + + warn(warnings, binding.node, [], 'non-state-reference', name); + break outer; + } } } } diff --git a/packages/svelte/src/compiler/phases/scope.js b/packages/svelte/src/compiler/phases/scope.js index 2143f4660cf9..69af8cc33dab 100644 --- a/packages/svelte/src/compiler/phases/scope.js +++ b/packages/svelte/src/compiler/phases/scope.js @@ -169,9 +169,8 @@ export class Scope { /** * @param {import('estree').Identifier} node * @param {import('#compiler').SvelteNode[]} path - * @param {boolean} is_template */ - reference(node, path, is_template) { + reference(node, path) { let references = this.references.get(node.name); if (!references) this.references.set(node.name, (references = [])); @@ -179,10 +178,9 @@ export class Scope { 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, is_template); + this.#parent.reference(node, path); } else { // no binding was found, and this is the top level scope, // which means this is a global @@ -217,11 +215,10 @@ 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, is_template, parent) { - /** @typedef {{ scope: Scope, is_template: boolean }} State */ +export function create_scopes(ast, root, allow_reactive_declarations, parent) { + /** @typedef {{ scope: Scope }} State */ /** * A map of node->associated scope. A node appearing in this map does not necessarily mean that it created a scope @@ -232,9 +229,9 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat scopes.set(ast, scope); /** @type {State} */ - const state = { scope, is_template }; + const state = { scope }; - /** @type {[Scope, { node: import('estree').Identifier; path: import('#compiler').SvelteNode[]; is_template: boolean }][]} */ + /** @type {[Scope, { node: import('estree').Identifier; path: import('#compiler').SvelteNode[] }][]} */ const references = []; /** @type {[Scope, import('estree').Pattern | import('estree').MemberExpression][]} */ @@ -265,7 +262,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat const scope = state.scope.child(true); scopes.set(node, scope); - next({ ...state, scope }); + next({ scope }); }; /** @@ -274,7 +271,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat const SvelteFragment = (node, { state, next }) => { const scope = analyze_let_directives(node, state.scope); scopes.set(node, scope); - next({ ...state, scope }); + next({ scope }); }; /** @@ -320,10 +317,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat 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(), is_template: state.is_template } - ]); + references.push([state.scope, { node, path: path.slice() }]); } }, @@ -346,7 +340,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat } } - next({ ...state, scope }); + next({ scope }); }, SvelteFragment, @@ -354,7 +348,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat RegularElement: SvelteFragment, Component(node, { state, visit, path }) { - state.scope.reference(b.id(node.name), path, false); + state.scope.reference(b.id(node.name), path); // let:x from the default slot is a weird one: // Its scope only applies to children that are not slots themselves. @@ -378,7 +372,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat } else if (child.type === 'SnippetBlock') { visit(child); } else { - visit(child, { ...state, scope }); + visit(child, { scope }); } } }, @@ -412,7 +406,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat if (node.id) scope.declare(node.id, 'normal', 'function'); add_params(scope, node.params); - next({ scope, is_template: false }); + next({ scope }); }, FunctionDeclaration(node, { state, next }) { @@ -422,7 +416,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat scopes.set(node, scope); add_params(scope, node.params); - next({ scope, is_template: false }); + next({ scope }); }, ArrowFunctionExpression(node, { state, next }) { @@ -430,7 +424,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat scopes.set(node, scope); add_params(scope, node.params); - next({ scope, is_template: false }); + next({ scope }); }, ForStatement: create_block_scope, @@ -475,7 +469,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat state.scope.declare(id, 'normal', 'let'); } - next({ ...state, scope }); + next({ scope }); } else { next(); } @@ -513,13 +507,13 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat (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, { ...state, scope }); + if (node.key) visit(node.key, { scope }); // children for (const child of node.body.nodes) { - visit(child, { ...state, scope }); + visit(child, { scope }); } - if (node.fallback) visit(node.fallback, { ...state, scope }); + if (node.fallback) visit(node.fallback, { 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 @@ -586,13 +580,13 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat } } - context.next({ ...state, scope: child_scope }); + context.next({ scope: child_scope }); }, Fragment: (node, context) => { const scope = context.state.scope.child(node.transparent); scopes.set(node, scope); - context.next({ ...state, scope }); + context.next({ scope }); }, BindDirective(node, context) { @@ -629,8 +623,8 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat // 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, is_template }] of references) { - scope.reference(node, path, is_template); + for (const [scope, { node, path }] of references) { + scope.reference(node, path); } for (const [scope, node] of updates) { 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 From 7eb14e7e79cd5dee21401ec7cc83361e6004231e Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 27 Nov 2023 09:43:21 -0500 Subject: [PATCH 5/6] continue not break --- .../svelte/src/compiler/phases/2-analyze/index.js | 2 +- .../samples/runes-referenced-nonstate/input.svelte | 4 +++- .../samples/runes-referenced-nonstate/warnings.json | 12 ++++++++++++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index 540a1b99129c..a278c8927251 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -432,7 +432,7 @@ export function analyze_component(root, options) { } warn(warnings, binding.node, [], 'non-state-reference', name); - break outer; + continue outer; } } } diff --git a/packages/svelte/tests/validator/samples/runes-referenced-nonstate/input.svelte b/packages/svelte/tests/validator/samples/runes-referenced-nonstate/input.svelte index 279cce71098e..fd9d6c3173c6 100644 --- a/packages/svelte/tests/validator/samples/runes-referenced-nonstate/input.svelte +++ b/packages/svelte/tests/validator/samples/runes-referenced-nonstate/input.svelte @@ -1,8 +1,10 @@ -

{a} + {b} = {a + b}

+ +

{a} + {b} + {c} = {a + b + c}

diff --git a/packages/svelte/tests/validator/samples/runes-referenced-nonstate/warnings.json b/packages/svelte/tests/validator/samples/runes-referenced-nonstate/warnings.json index e42a8d8782b9..7e251a4e7022 100644 --- a/packages/svelte/tests/validator/samples/runes-referenced-nonstate/warnings.json +++ b/packages/svelte/tests/validator/samples/runes-referenced-nonstate/warnings.json @@ -10,5 +10,17 @@ "column": 6, "line": 3 } + }, + { + "code": "non-state-reference", + "message": "c is updated, but is not declared with $state(...). Changing its value will not correctly trigger updates.", + "start": { + "column": 5, + "line": 4 + }, + "end": { + "column": 6, + "line": 4 + } } ] From 864272cc4996559d0fd9b9a26733ba3334a81cd6 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 27 Nov 2023 09:43:55 -0500 Subject: [PATCH 6/6] undo --- packages/svelte/src/compiler/phases/scope.js | 1 - packages/svelte/src/compiler/types/index.d.ts | 1 - 2 files changed, 2 deletions(-) diff --git a/packages/svelte/src/compiler/phases/scope.js b/packages/svelte/src/compiler/phases/scope.js index 69af8cc33dab..e958df604ec2 100644 --- a/packages/svelte/src/compiler/phases/scope.js +++ b/packages/svelte/src/compiler/phases/scope.js @@ -91,7 +91,6 @@ export class Scope { const binding = { node, references: [], - referenced_in_template: false, legacy_dependencies: [], initial, mutated: false, diff --git a/packages/svelte/src/compiler/types/index.d.ts b/packages/svelte/src/compiler/types/index.d.ts index 04a6c08c9c09..2c5480eb7f58 100644 --- a/packages/svelte/src/compiler/types/index.d.ts +++ b/packages/svelte/src/compiler/types/index.d.ts @@ -268,7 +268,6 @@ 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 */