feat: better compiler warnings for non-reactive dependencies of reactive statements (#12824)

* feat: better compiler warnings for non-reactive dependencies of reactive statements

* fix

* regenerate

Author: Rich-Harris

.changeset/strange-pillows-greet.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+feat: better compiler warnings for non-reactive dependencies of reactive statements

packages/svelte/messages/compile-warnings/script.md

@@ -26,9 +26,13 @@
 
 > Reactive declarations only exist at the top level of the instance script
 
-## reactive_declaration_module_script
+## reactive_declaration_module_script_dependency
 
-> All dependencies of the reactive declaration are declared in a module script and will not be reactive
+> Reassignments of module-level declarations will not cause reactive statements to update
+
+## reactive_declaration_non_reactive_property
+
+> Properties of objects and arrays are not reactive unless in runes mode. Changes to this property will not cause the reactive statement to update
 
 ## state_referenced_locally
 

packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js

@@ -109,5 +109,13 @@ export function Identifier(node, context) {
 		) {
 			w.state_referenced_locally(node);
 		}
+
+		if (
+			context.state.reactive_statement &&
+			binding.scope === context.state.analysis.module.scope &&
+			binding.reassigned
+		) {
+			w.reactive_declaration_module_script_dependency(node);
+		}
 	}
 }

packages/svelte/src/compiler/phases/2-analyze/visitors/LabeledStatement.js

@@ -4,6 +4,7 @@
 import * as e from '../../../errors.js';
 import { extract_identifiers, object } from '../../../utils/ast.js';
 import * as w from '../../../warnings.js';
+import { is_state_source } from '../../3-transform/client/utils.js';
 
 /**
  * @param {LabeledStatement} node
@@ -66,15 +67,6 @@ export function LabeledStatement(node, context) {
 
 			context.state.reactive_statements.set(node, reactive_statement);
 
-			if (
-				reactive_statement.dependencies.length &&
-				reactive_statement.dependencies.every(
-					(d) => d.scope === context.state.analysis.module.scope && d.declaration_kind !== 'const'
-				)
-			) {
-				w.reactive_declaration_module_script(node);
-			}
-
 			if (
 				node.body.type === 'ExpressionStatement' &&
 				node.body.expression.type === 'AssignmentExpression'

packages/svelte/src/compiler/phases/2-analyze/visitors/MemberExpression.js

@@ -1,6 +1,8 @@
-/** @import { MemberExpression } from 'estree' */
+/** @import { MemberExpression, Node } from 'estree' */
 /** @import { Context } from '../types' */
 import * as e from '../../../errors.js';
+import * as w from '../../../warnings.js';
+import { object } from '../../../utils/ast.js';
 import { is_pure, is_safe_identifier } from './shared/utils.js';
 
 /**
@@ -23,5 +25,29 @@ export function MemberExpression(node, context) {
 		context.state.analysis.needs_context = true;
 	}
 
+	if (context.state.reactive_statement) {
+		const left = object(node);
+
+		if (left !== null) {
+			const binding = context.state.scope.get(left.name);
+
+			if (binding && binding.kind === 'normal') {
+				const parent = /** @type {Node} */ (context.path.at(-1));
+
+				if (
+					binding.scope === context.state.analysis.module.scope ||
+					binding.declaration_kind === 'import' ||
+					(binding.initial &&
+						binding.initial.type !== 'ArrayExpression' &&
+						binding.initial.type !== 'ObjectExpression')
+				) {
+					if (parent.type !== 'MemberExpression' && parent.type !== 'CallExpression') {
+						w.reactive_declaration_non_reactive_property(node);
+					}
+				}
+			}
+		}
+	}
+
 	context.next();
 }

packages/svelte/src/compiler/phases/3-transform/client/utils.js

@@ -1,6 +1,7 @@
 /** @import { ArrowFunctionExpression, Expression, FunctionDeclaration, FunctionExpression, Identifier, Pattern, PrivateIdentifier, Statement } from 'estree' */
 /** @import { Binding, SvelteNode } from '#compiler' */
 /** @import { ClientTransformState, ComponentClientTransformState, ComponentContext } from './types.js' */
+/** @import { Analysis } from '../../types.js' */
 /** @import { Scope } from '../../scope.js' */
 import * as b from '../../../utils/builders.js';
 import { extract_identifiers, is_simple_expression } from '../../../utils/ast.js';
@@ -16,13 +17,13 @@ import { get_value } from './visitors/shared/declarations.js';
 
 /**
  * @param {Binding} binding
- * @param {ClientTransformState} state
+ * @param {Analysis} analysis
  * @returns {boolean}
  */
-export function is_state_source(binding, state) {
+export function is_state_source(binding, analysis) {
 	return (
 		(binding.kind === 'state' || binding.kind === 'raw_state') &&
-		(!state.analysis.immutable || binding.reassigned || state.analysis.accessors)
+		(!analysis.immutable || binding.reassigned || analysis.accessors)
 	);
 }
 

packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js

@@ -126,7 +126,7 @@ export function VariableDeclaration(node, context) {
 					if (rune === '$state' && should_proxy(value, context.state.scope)) {
 						value = b.call('$.proxy', value);
 					}
-					if (is_state_source(binding, context.state)) {
+					if (is_state_source(binding, context.state.analysis)) {
 						value = b.call('$.source', value);
 					}
 					return value;

packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/declarations.js

@@ -18,7 +18,7 @@ export function get_value(node) {
 export function add_state_transformers(context) {
 	for (const [name, binding] of context.state.scope.declarations) {
 		if (
-			is_state_source(binding, context.state) ||
+			is_state_source(binding, context.state.analysis) ||
 			binding.kind === 'derived' ||
 			binding.kind === 'legacy_reactive'
 		) {

packages/svelte/src/compiler/warnings.js

@@ -101,7 +101,8 @@ export const codes = [
 	"perf_avoid_inline_class",
 	"perf_avoid_nested_class",
 	"reactive_declaration_invalid_placement",
-	"reactive_declaration_module_script",
+	"reactive_declaration_module_script_dependency",
+	"reactive_declaration_non_reactive_property",
 	"state_referenced_locally",
 	"store_rune_conflict",
 	"css_unused_selector",
@@ -630,11 +631,19 @@ export function reactive_declaration_invalid_placement(node) {
 }
 
 /**
- * All dependencies of the reactive declaration are declared in a module script and will not be reactive
+ * Reassignments of module-level declarations will not cause reactive statements to update
  * @param {null | NodeLike} node
  */
-export function reactive_declaration_module_script(node) {
-	w(node, "reactive_declaration_module_script", "All dependencies of the reactive declaration are declared in a module script and will not be reactive");
+export function reactive_declaration_module_script_dependency(node) {
+	w(node, "reactive_declaration_module_script_dependency", "Reassignments of module-level declarations will not cause reactive statements to update");
+}
+
+/**
+ * Properties of objects and arrays are not reactive unless in runes mode. Changes to this property will not cause the reactive statement to update
+ * @param {null | NodeLike} node
+ */
+export function reactive_declaration_non_reactive_property(node) {
+	w(node, "reactive_declaration_non_reactive_property", "Properties of objects and arrays are not reactive unless in runes mode. Changes to this property will not cause the reactive statement to update");
 }
 
 /**

packages/svelte/tests/validator/samples/reactive-module-variable/input.svelte

@@ -1,6 +1,11 @@
 <script context="module">
-	let foo;
+	let foo = 0;
+
+	export function update() {
+		foo += 1;
+	}
 </script>
+
 <script>
 	$: bar = foo;
 </script>