turn reactive_declaration_non_reactive_property into a runtime warning

Author: Rich-Harris

documentation/docs/98-reference/.generated/client-warnings.md

@@ -86,6 +86,12 @@ Mutating a value outside the component that created it is strongly discouraged.
 %component% mutated a value owned by %owner%. This is strongly discouraged. Consider passing values to child components with `bind:`, or use a callback instead
 ```
 
+### reactive_declaration_non_reactive_property
+
+```
+A `$:` statement (%location%) read reactive state that was not visible to the compiler. Updates to this state will not cause the statement to re-run. The behaviour of this code will change if you migrate it to runes mode
+```
+
 ### state_proxy_equality_mismatch
 
 ```

documentation/docs/98-reference/.generated/compile-warnings.md

@@ -726,12 +726,6 @@ Reactive declarations only exist at the top level of the instance script
 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
-```
-
 ### script_context_deprecated
 
 ```

packages/svelte/messages/client-warnings/warnings.md

@@ -54,6 +54,10 @@ The easiest way to log a value as it changes over time is to use the [`$inspect`
 
 > %component% mutated a value owned by %owner%. This is strongly discouraged. Consider passing values to child components with `bind:`, or use a callback instead
 
+## reactive_declaration_non_reactive_property
+
+> A `$:` statement (%location%) read reactive state that was not visible to the compiler. Updates to this state will not cause the statement to re-run. The behaviour of this code will change if you migrate it to runes mode
+
 ## state_proxy_equality_mismatch
 
 > Reactive `$state(...)` proxies and the values they proxy have different identities. Because of this, comparisons with `%operator%` will produce unexpected results

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

@@ -26,10 +26,6 @@
 
 > 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
 
 > State referenced in its own scope will never update. Did you mean to reference it inside a closure?

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

@@ -25,30 +25,5 @@ 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' &&
-						binding.scope.function_depth <= 1)
-				) {
-					if (parent.type !== 'MemberExpression' && parent.type !== 'CallExpression') {
-						w.reactive_declaration_non_reactive_property(node);
-					}
-				}
-			}
-		}
-	}
-
 	context.next();
 }

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

@@ -1,6 +1,8 @@
+/** @import { Location } from 'locate-character' */
 /** @import { Expression, LabeledStatement, Statement } from 'estree' */
 /** @import { ReactiveStatement } from '#compiler' */
 /** @import { ComponentContext } from '../types' */
+import { dev, locator } from '../../../../state.js';
 import * as b from '../../../../utils/builders.js';
 import { build_getter } from '../utils.js';
 
@@ -48,14 +50,18 @@ export function LabeledStatement(node, context) {
 		sequence.push(serialized);
 	}
 
+	const location = dev ? locator(/** @type {number} */ (node.start)) : undefined;
+
 	// these statements will be topologically ordered later
 	context.state.legacy_reactive_statements.set(
 		node,
 		b.stmt(
 			b.call(
 				'$.legacy_pre_effect',
 				sequence.length > 0 ? b.thunk(b.sequence(sequence)) : b.thunk(b.block([])),
-				b.thunk(b.block(body))
+				b.thunk(b.block(body)),
+				location && b.literal(location.line),
+				location && b.literal(location.column)
 			)
 		)
 	);

packages/svelte/src/compiler/warnings.js

@@ -102,7 +102,6 @@ export const codes = [
 	"perf_avoid_nested_class",
 	"reactive_declaration_invalid_placement",
 	"reactive_declaration_module_script_dependency",
-	"reactive_declaration_non_reactive_property",
 	"state_referenced_locally",
 	"store_rune_conflict",
 	"css_unused_selector",
@@ -641,14 +640,6 @@ 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");
-}
-
 /**
  * State referenced in its own scope will never update. Did you mean to reference it inside a closure?
  * @param {null | NodeLike} node

packages/svelte/src/internal/client/dev/location.js

@@ -0,0 +1,25 @@
+import { DEV } from 'esm-env';
+import { FILENAME } from '../../../constants.js';
+import { dev_current_component_function } from '../runtime.js';
+
+/**
+ *
+ * @param {number} [line]
+ * @param {number} [column]
+ */
+export function get_location(line, column) {
+	if (!DEV) return undefined;
+
+	var filename = dev_current_component_function?.[FILENAME];
+	var location = filename && `${filename}:${line}:${column}`;
+
+	return sanitize_location(location);
+}
+
+/**
+ * Prevent devtools trying to make `location` a clickable link by inserting a zero-width space
+ * @param {string | undefined} location
+ */
+export function sanitize_location(location) {
+	return location?.replace(/\//g, '/\u200b');
+}

packages/svelte/src/internal/client/dom/blocks/html.js

@@ -9,6 +9,7 @@ import { hash } from '../../../../utils.js';
 import { DEV } from 'esm-env';
 import { dev_current_component_function } from '../../runtime.js';
 import { get_first_child, get_next_sibling } from '../operations.js';
+import { sanitize_location } from '../../dev/location.js';
 
 /**
  * @param {Element} element
@@ -28,9 +29,7 @@ function check_hash(element, server_hash, value) {
 		location = `in ${dev_current_component_function[FILENAME]}`;
 	}
 
-	w.hydration_html_changed(
-		location?.replace(/\//g, '/\u200b') // prevent devtools trying to make it a clickable link by inserting a zero-width space
-	);
+	w.hydration_html_changed(sanitize_location(location));
 }
 
 /**

packages/svelte/src/internal/client/reactivity/effects.js

@@ -16,7 +16,8 @@ import {
 	set_is_flushing_effect,
 	set_signal_status,
 	untrack,
-	skip_reaction
+	skip_reaction,
+	capture_signals
 } from '../runtime.js';
 import {
 	DIRTY,
@@ -39,10 +40,13 @@ import {
 } from '../constants.js';
 import { set } from './sources.js';
 import * as e from '../errors.js';
+import * as w from '../warnings.js';
 import { DEV } from 'esm-env';
 import { define_property } from '../../shared/utils.js';
 import { get_next_sibling } from '../dom/operations.js';
 import { destroy_derived } from './deriveds.js';
+import { FILENAME } from '../../../constants.js';
+import { get_location } from '../dev/location.js';
 
 /**
  * @param {'$effect' | '$effect.pre' | '$inspect'} rune
@@ -261,14 +265,21 @@ export function effect(fn) {
  * Internal representation of `$: ..`
  * @param {() => any} deps
  * @param {() => void | (() => void)} fn
+ * @param {number} [line]
+ * @param {number} [column]
  */
-export function legacy_pre_effect(deps, fn) {
+export function legacy_pre_effect(deps, fn, line, column) {
 	var context = /** @type {ComponentContextLegacy} */ (component_context);
 
 	/** @type {{ effect: null | Effect, ran: boolean }} */
 	var token = { effect: null, ran: false };
 	context.l.r1.push(token);
 
+	if (DEV) {
+		var location = get_location(line, column);
+		var explicit_deps = capture_signals(deps);
+	}
+
 	token.effect = render_effect(() => {
 		deps();
 
@@ -278,7 +289,16 @@ export function legacy_pre_effect(deps, fn) {
 
 		token.ran = true;
 		set(context.l.r2, true);
-		untrack(fn);
+
+		if (DEV) {
+			var implicit_deps = capture_signals(() => untrack(fn));
+
+			for (const signal of implicit_deps) {
+				if (!explicit_deps.has(signal)) {
+					w.reactive_declaration_non_reactive_property(/** @type {string} */ (location));
+				}
+			}
+		}
 	});
 }