fix: correctly highlight sources reassigned inside trace (#14811)

* fix: correctly highlight sources reassigned inside `trace`

* chore: add missing effect logs

* fix: prevent `null` access on `tracing_expressions` for nested tracing

* chore: add test case for #14853

* fix: types for `$inpect.trace`

Author: paoloricciuti

.changeset/silver-shoes-rule.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: correctly highlight sources reassigned inside `trace`

packages/svelte/src/ambient.d.ts

@@ -433,7 +433,7 @@ declare namespace $inspect {
 	 *   });
 	 * </script>
 	 */
-	export function trace(name: string): void;
+	export function trace(name?: string): void;
 
 	// prevent intellisense from being unhelpful
 	/** @deprecated */

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

@@ -15,7 +15,7 @@ export let tracing_expressions = null;
  */
 function log_entry(signal, entry) {
 	const debug = signal.debug;
-	const value = signal.v;
+	const value = signal.trace_need_increase ? signal.trace_v : signal.v;
 
 	if (value === UNINITIALIZED) {
 		return;
@@ -121,7 +121,7 @@ export function trace(label, fn) {
 			console.groupEnd();
 		}
 
-		if (previously_tracing_expressions !== null) {
+		if (previously_tracing_expressions !== null && tracing_expressions !== null) {
 			for (const [signal, entry] of tracing_expressions.entries) {
 				var prev_entry = previously_tracing_expressions.get(signal);
 

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

@@ -167,11 +167,16 @@ export function set(source, value) {
  */
 export function internal_set(source, value) {
 	if (!source.equals(value)) {
+		var old_value = source.v;
 		source.v = value;
 		source.version = increment_version();
 
 		if (DEV && tracing_mode_flag) {
 			source.updated = get_stack('UpdatedAt');
+			if (active_effect != null) {
+				source.trace_need_increase = true;
+				source.trace_v ??= old_value;
+			}
 		}
 
 		mark_reactions(source, DIRTY);

packages/svelte/src/internal/client/reactivity/types.d.ts

@@ -17,6 +17,8 @@ export interface Value<V = unknown> extends Signal {
 	/** Dev only */
 	created?: Error | null;
 	updated?: Error | null;
+	trace_need_increase?: boolean;
+	trace_v?: V;
 	debug?: null | (() => void);
 }
 

packages/svelte/src/internal/client/runtime.js

@@ -528,6 +528,23 @@ export function update_effect(effect) {
 		effect.teardown = typeof teardown === 'function' ? teardown : null;
 		effect.version = current_version;
 
+		var deps = effect.deps;
+
+		// In DEV, we need to handle a case where $inspect.trace() might
+		// incorrectly state a source dependency has not changed when it has.
+		// That's beacuse that source was changed by the same effect, causing
+		// the versions to match. We can avoid this by incrementing the version
+		if (DEV && tracing_mode_flag && (effect.f & DIRTY) !== 0 && deps !== null) {
+			for (let i = 0; i < deps.length; i++) {
+				var dep = deps[i];
+				if (dep.trace_need_increase) {
+					dep.version = increment_version();
+					dep.trace_need_increase = undefined;
+					dep.trace_v = undefined;
+				}
+			}
+		}
+
 		if (DEV) {
 			dev_effect_stack.push(effect);
 		}

packages/svelte/tests/runtime-runes/samples/inspect-trace-nested/_config.js

@@ -0,0 +1,56 @@
+import { flushSync } from 'svelte';
+import { test } from '../../test';
+
+/**
+ * @param {any[]} logs
+ */
+function normalise_trace_logs(logs) {
+	let normalised = [];
+	for (let i = 0; i < logs.length; i++) {
+		const log = logs[i];
+
+		if (typeof log === 'string' && log.includes('%c')) {
+			const split = log.split('%c');
+			normalised.push({
+				log: (split[0].length !== 0 ? split[0] : split[1]).trim(),
+				highlighted: logs[i + 1] === 'color: CornflowerBlue; font-weight: bold'
+			});
+			i++;
+		} else if (log instanceof Error) {
+			continue;
+		} else {
+			normalised.push({ log });
+		}
+	}
+	return normalised;
+}
+
+export default test({
+	compileOptions: {
+		dev: true
+	},
+
+	test({ assert, target, logs }) {
+		// initial log, everything is highlighted
+
+		assert.deepEqual(normalise_trace_logs(logs), [
+			{ log: 'iife', highlighted: false },
+			{ log: '$state', highlighted: true },
+			{ log: 0 },
+			{ log: 'effect', highlighted: false }
+		]);
+
+		logs.length = 0;
+
+		const button = target.querySelector('button');
+		button?.click();
+		flushSync();
+
+		assert.deepEqual(normalise_trace_logs(logs), [
+			{ log: 'iife', highlighted: false },
+			{ log: '$state', highlighted: true },
+			{ log: 1 },
+			{ log: 'effect', highlighted: false }
+		]);
+	}
+});

packages/svelte/tests/runtime-runes/samples/inspect-trace-nested/main.svelte

@@ -0,0 +1,13 @@
+<script>
+	let count = $state(0);
+
+	$effect(() => {
+		$inspect.trace('effect');
+		(()=>{
+			$inspect.trace("iife");
+			count;
+		})();
+	});
+</script>
+
+<button onclick={() => count++}>{count}</button>

packages/svelte/tests/runtime-runes/samples/inspect-trace-null/_config.js

@@ -0,0 +1,8 @@
+import { test } from '../../test';
+
+export default test({
+	compileOptions: {
+		dev: true
+	},
+	test() {}
+});

packages/svelte/tests/runtime-runes/samples/inspect-trace-null/main.svelte

@@ -0,0 +1,9 @@
+<script>
+	let count = $state(null);
+	$effect(() => {
+		$inspect.trace();
+		count;
+	});
+</script>
+
+<button onclick={()=>count++}>{count}</button>

packages/svelte/tests/runtime-runes/samples/inspect-trace-reassignment/_config.js

@@ -0,0 +1,72 @@
+import { flushSync } from 'svelte';
+import { test } from '../../test';
+
+/**
+ * @param {any[]} logs
+ */
+function normalise_trace_logs(logs) {
+	let normalised = [];
+	for (let i = 0; i < logs.length; i++) {
+		const log = logs[i];
+
+		if (typeof log === 'string' && log.includes('%c')) {
+			const split = log.split('%c');
+			normalised.push({
+				log: (split[0].length !== 0 ? split[0] : split[1]).trim(),
+				highlighted: logs[i + 1] === 'color: CornflowerBlue; font-weight: bold'
+			});
+			i++;
+		} else if (log instanceof Error) {
+			continue;
+		} else {
+			normalised.push({ log });
+		}
+	}
+	return normalised;
+}
+
+export default test({
+	compileOptions: {
+		dev: true
+	},
+
+	test({ assert, target, logs }) {
+		// initial log, everything is highlighted
+
+		assert.deepEqual(normalise_trace_logs(logs), [
+			{ log: 'effect', highlighted: false },
+			{ log: '$state', highlighted: true },
+			{ log: false }
+		]);
+
+		logs.length = 0;
+
+		const button = target.querySelector('button');
+		button?.click();
+		flushSync();
+
+		const input = target.querySelector('input');
+		input?.click();
+		flushSync();
+
+		// checked changed, effect reassign state, values should be correct and be correctly highlighted
+
+		assert.deepEqual(normalise_trace_logs(logs), [
+			{ log: 'effect', highlighted: false },
+			{ log: '$state', highlighted: true },
+			{ log: true },
+			{ log: '$state', highlighted: true },
+			{ log: 1 },
+			{ log: 'effect', highlighted: false },
+			{ log: '$state', highlighted: false },
+			{ log: true },
+			{ log: '$state', highlighted: true },
+			{ log: 2 },
+			{ log: 'effect', highlighted: false },
+			{ log: '$state', highlighted: false },
+			{ log: true },
+			{ log: '$state', highlighted: true },
+			{ log: 3 }
+		]);
+	}
+});

packages/svelte/tests/runtime-runes/samples/inspect-trace-reassignment/main.svelte

@@ -0,0 +1,18 @@
+<script>
+	let count = $state(0);
+	let checked = $state(false);
+
+	$effect(() => {
+		$inspect.trace("effect");
+		if(checked && count > 0 && count < 3){
+			let old = count;
+			// this should not show up in the logs
+			count = 1000;
+			count = old + 1;
+		}
+	});
+</script>
+<input type="checkbox" bind:checked />
+<button onclick={()=>{
+	count++;
+}}>{count}</button>

packages/svelte/types/index.d.ts

@@ -3091,7 +3091,7 @@ declare namespace $inspect {
 	 *   });
 	 * </script>
 	 */
-	export function trace(name: string): void;
+	export function trace(name?: string): void;
 
 	// prevent intellisense from being unhelpful
 	/** @deprecated */