fix: use `active_reaction` as indication to use `source` or `state`

packages/svelte/src/reactivity/map.js

@@ -1,8 +1,8 @@
-/** @import { Source } from '#client' */
+/** @import { Reaction, Source } from '#client' */
 import { DEV } from 'esm-env';
 import { set, source, state } from '../internal/client/reactivity/sources.js';
 import { label, tag } from '../internal/client/dev/tracing.js';
-import { get, push_reaction_value } from '../internal/client/runtime.js';
+import { active_reaction, get, push_reaction_value } from '../internal/client/runtime.js';
 import { increment } from './utils.js';
 
 /**
@@ -56,6 +56,8 @@ export class SvelteMap extends Map {
 	#sources = new Map();
 	#version = state(0);
 	#size = state(0);
+	/**@type {Reaction | null} */
+	#initial_reaction = null;
 
 	/**
 	 * @param {Iterable<readonly [K, V]> | null | undefined} [value]
@@ -63,6 +65,8 @@ export class SvelteMap extends Map {
 	constructor(value) {
 		super();
 
+		this.#initial_reaction = active_reaction;
+
 		if (DEV) {
 			// If the value is invalid then the native exception will fire here
 			value = new Map(value);
@@ -79,17 +83,31 @@ export class SvelteMap extends Map {
 		}
 	}
 
+	/**
+	 * If the active_reaction is the same as the reaction that created this SvelteMap,
+	 * we use state so that it will not be a dependency of the reaction. Otherwise we
+	 * use source so it will be.
+	 *
+	 * @template T
+	 * @param {T} value
+	 * @returns {Source<T>}
+	 */
+	#source(value) {
+		if (this.#initial_reaction === active_reaction) {
+			return state(value);
+		}
+		return source(value);
+	}
+
 	/** @param {K} key */
 	has(key) {
 		var sources = this.#sources;
 		var s = sources.get(key);
-		var is_new = false;
 
 		if (s === undefined) {
 			var ret = super.get(key);
 			if (ret !== undefined) {
-				s = source(0);
+				s = this.#source(0);
-				is_new = true;
 
 				if (DEV) {
 					tag(s, `SvelteMap get(${label(key)})`);
@@ -105,12 +123,6 @@ export class SvelteMap extends Map {
 		}
 
 		get(s);
-		if (is_new) {
-			// if it's a new source we want to push to the reactions values
-			// AFTER we read it so that the effect depends on it but we don't
-			// trigger an unsafe mutation (since it was created within the derived)
-			push_reaction_value(s);
-		}
 		return true;
 	}
 
@@ -127,13 +139,11 @@ export class SvelteMap extends Map {
 	get(key) {
 		var sources = this.#sources;
 		var s = sources.get(key);
-		var is_new = false;
 
 		if (s === undefined) {
 			var ret = super.get(key);
 			if (ret !== undefined) {
-				s = source(0);
+				s = this.#source(0);
-				is_new = true;
 
 				if (DEV) {
 					tag(s, `SvelteMap get(${label(key)})`);
@@ -149,12 +159,6 @@ export class SvelteMap extends Map {
 		}
 
 		get(s);
-		if (is_new) {
-			// if it's a new source we want to push to the reactions values
-			// AFTER we read it so that the effect depends on it but we don't
-			// trigger an unsafe mutation (since it was created within the derived)
-			push_reaction_value(s);
-		}
 		return super.get(key);
 	}
 
@@ -232,12 +236,10 @@ export class SvelteMap extends Map {
 		get(this.#version);
 
 		var sources = this.#sources;
-		var new_sources = new WeakSet();
 		if (this.#size.v !== sources.size) {
 			for (var key of super.keys()) {
 				if (!sources.has(key)) {
-					var s = source(0);
+					var s = this.#source(0);
-					new_sources.add(s);
 					if (DEV) {
 						tag(s, `SvelteMap get(${label(key)})`);
 					}
@@ -249,12 +251,6 @@ export class SvelteMap extends Map {
 
 		for ([, s] of this.#sources) {
 			get(s);
-			if (new_sources.has(s)) {
-				// if it's a new source we want to push to the reactions values
-				// AFTER we read it so that the effect depends on it but we don't
-				// trigger an unsafe mutation (since it was created within the derived)
-				push_reaction_value(s);
-			}
 		}
 	}
 

packages/svelte/src/reactivity/set.js

@@ -1,8 +1,8 @@
-/** @import { Source } from '#client' */
+/** @import { Reaction, Source } from '#client' */
 import { DEV } from 'esm-env';
 import { source, set, state } from '../internal/client/reactivity/sources.js';
 import { label, tag } from '../internal/client/dev/tracing.js';
-import { get, push_reaction_value } from '../internal/client/runtime.js';
+import { active_reaction, get } from '../internal/client/runtime.js';
 import { increment } from './utils.js';
 
 var read_methods = ['forEach', 'isDisjointFrom', 'isSubsetOf', 'isSupersetOf'];
@@ -51,12 +51,17 @@ export class SvelteSet extends Set {
 	#version = state(0);
 	#size = state(0);
 
+	/**@type {Reaction | null}*/
+	#initial_reaction = null;
+
 	/**
 	 * @param {Iterable<T> | null | undefined} [value]
 	 */
 	constructor(value) {
 		super();
 
+		this.#initial_reaction = active_reaction;
+
 		if (DEV) {
 			// If the value is invalid then the native exception will fire here
 			value = new Set(value);
@@ -75,6 +80,22 @@ export class SvelteSet extends Set {
 		if (!inited) this.#init();
 	}
 
+	/**
+	 * If the active_reaction is the same as the reaction that created this SvelteMap,
+	 * we use state so that it will not be a dependency of the reaction. Otherwise we
+	 * use source so it will be.
+	 *
+	 * @template T
+	 * @param {T} value
+	 * @returns {Source<T>}
+	 */
+	#source(value) {
+		if (this.#initial_reaction === active_reaction) {
+			return state(value);
+		}
+		return source(value);
+	}
+
 	// We init as part of the first instance so that we can treeshake this class
 	#init() {
 		inited = true;
@@ -107,7 +128,6 @@ export class SvelteSet extends Set {
 		var has = super.has(value);
 		var sources = this.#sources;
 		var s = sources.get(value);
-		var is_new = false;
 
 		if (s === undefined) {
 			if (!has) {
@@ -117,8 +137,7 @@ export class SvelteSet extends Set {
 				return false;
 			}
 
-			s = source(true);
+			s = this.#source(true);
-			is_new = true;
 
 			if (DEV) {
 				tag(s, `SvelteSet has(${label(value)})`);
@@ -128,9 +147,6 @@ export class SvelteSet extends Set {
 		}
 
 		get(s);
-		if (is_new) {
-			push_reaction_value(s);
-		}
 		return has;
 	}
 

packages/svelte/tests/runtime-runes/samples/side-effect-derived-map/_config.js

@@ -8,7 +8,8 @@ export default test({
 	},
 
 	test({ assert, target }) {
-		const [button1, button2] = target.querySelectorAll('button');
+		const [button1, button2, button3, button4, button5, button6, button7, button8] =
+			target.querySelectorAll('button');
 
 		assert.throws(() => {
 			button1?.click();
@@ -19,5 +20,35 @@ export default test({
 			button2?.click();
 			flushSync();
 		});
+
+		assert.throws(() => {
+			button3?.click();
+			flushSync();
+		}, /state_unsafe_mutation/);
+
+		assert.doesNotThrow(() => {
+			button4?.click();
+			flushSync();
+		});
+
+		assert.throws(() => {
+			button5?.click();
+			flushSync();
+		}, /state_unsafe_mutation/);
+
+		assert.doesNotThrow(() => {
+			button6?.click();
+			flushSync();
+		});
+
+		assert.throws(() => {
+			button7?.click();
+			flushSync();
+		}, /state_unsafe_mutation/);
+
+		assert.doesNotThrow(() => {
+			button8?.click();
+			flushSync();
+		});
 	}
 });

packages/svelte/tests/runtime-runes/samples/side-effect-derived-map/main.svelte

@@ -1,27 +1,101 @@
 <script>
 	import { SvelteMap } from 'svelte/reactivity';
 
-	let visibleExternal = $state(false);
+	let outside_basic = $state(false);
-	let external = new SvelteMap();
+	let outside_basic_map = new SvelteMap();
-	const throws = $derived.by(() => {
+	const throw_basic = $derived.by(() => {
-		external.set(1, 1);
+		outside_basic_map.set(1, 1);
-		return external;
+		return outside_basic_map;
 	});
 
-	let visibleInternal = $state(false);
+	let inside_basic = $state(false);
-	const works = $derived.by(() => {
+	const works_basic = $derived.by(() => {
-		let internal = new SvelteMap();
+		let inside = new SvelteMap();
-		internal.set(1, 1);
+		inside.set(1, 1);
-		return internal;
+		return inside;
+	});
+
+	let outside_has = $state(false);
+	let outside_has_map = new SvelteMap([[1, 1]]);
+	const throw_has = $derived.by(() => {
+		outside_has_map.has(1);
+		outside_has_map.set(1, 2);
+		return outside_has_map;
+	});
+
+	let inside_has = $state(false);
+	const works_has = $derived.by(() => {
+		let inside = new SvelteMap([[1, 1]]);
+		inside.has(1);
+		inside.set(1, 1);
+		return inside;
+	});
+
+	let outside_get = $state(false);
+	let outside_get_map = new SvelteMap([[1, 1]]);
+	const throw_get = $derived.by(() => {
+		outside_get_map.get(1);
+		outside_get_map.set(1, 2);
+		return outside_get_map;
+	});
+
+	let inside_get = $state(false);
+	const works_get = $derived.by(() => {
+		let inside = new SvelteMap([[1, 1]]);
+		inside.get(1);
+		inside.set(1, 1);
+		return inside;
+	});
+
+	let outside_values = $state(false);
+	let outside_values_map = new SvelteMap([[1, 1]]);
+	const throw_values = $derived.by(() => {
+		outside_values_map.values(1);
+		outside_values_map.set(1, 2);
+		return outside_values_map;
+	});
+
+	let inside_values = $state(false);
+	const works_values = $derived.by(() => {
+		let inside = new SvelteMap([[1, 1]]);
+		inside.values();
+		inside.set(1, 1);
+		return inside;
 	});
 </script>
 
-<button onclick={() => (visibleExternal = true)}>external</button>
+<button onclick={() => (outside_basic = true)}>external</button>
-{#if visibleExternal}
+{#if outside_basic}
-	{throws}
+	{throw_basic}
+{/if}
+<button onclick={() => (inside_basic = true)}>internal</button>
+{#if inside_basic}
+	{works_basic}
+{/if}
+
+<button onclick={() => (outside_has = true)}>external</button>
+{#if outside_has}
+	{throw_has}
 {/if}
-<button onclick={() => (visibleInternal = true)}>internal</button>
+<button onclick={() => (inside_has = true)}>internal</button>
-{#if visibleInternal}
+{#if inside_has}
-	{works}
+	{works_has}
 {/if}
 
+<button onclick={() => (outside_get = true)}>external</button>
+{#if outside_get}
+	{throw_get}
+{/if}
+<button onclick={() => (inside_get = true)}>internal</button>
+{#if inside_get}
+	{works_get}
+{/if}
+
+<button onclick={() => (outside_values = true)}>external</button>
+{#if outside_values}
+	{throw_values}
+{/if}
+<button onclick={() => (inside_values = true)}>internal</button>
+{#if inside_values}
+	{works_values}
+{/if}

packages/svelte/tests/runtime-runes/samples/side-effect-derived-set/_config.js

@@ -8,7 +8,7 @@ export default test({
 	},
 
 	test({ assert, target }) {
-		const [button1, button2] = target.querySelectorAll('button');
+		const [button1, button2, button3, button4] = target.querySelectorAll('button');
 
 		assert.throws(() => {
 			button1?.click();
@@ -19,5 +19,15 @@ export default test({
 			button2?.click();
 			flushSync();
 		});
+
+		assert.throws(() => {
+			button3?.click();
+			flushSync();
+		}, /state_unsafe_mutation/);
+
+		assert.doesNotThrow(() => {
+			button4?.click();
+			flushSync();
+		});
 	}
 });

packages/svelte/tests/runtime-runes/samples/side-effect-derived-set/main.svelte

@@ -1,27 +1,52 @@
 <script>
 	import { SvelteSet } from 'svelte/reactivity';
 
-	let visibleExternal = $state(false);
+	let outside_basic = $state(false);
-	let external = new SvelteSet();
+	let outside_basic_set = new SvelteSet();
-	const throws = $derived.by(() => {
+	const throws_basic = $derived.by(() => {
-		external.add(1);
+		outside_basic_set.add(1);
-		return external;
+		return outside_basic_set;
 	})
 
-	let visibleInternal = $state(false);
+	let inside_basic = $state(false);
-	const works = $derived.by(() => {
+	const works_basic = $derived.by(() => {
 		let internal = new SvelteSet();
 		internal.add(1);
 		return internal;
 	})
+
+	let outside_has_delete = $state(false);
+	let outside_has_delete_set = new SvelteSet([1]);
+	const throws_has_delete = $derived.by(() => {
+		outside_has_delete_set.has(1);
+		outside_has_delete_set.delete(1);
+		return outside_has_delete_set;
+	})
+
+	let inside_has_delete = $state(false);
+	const works_has_delete = $derived.by(() => {
+		let internal = new SvelteSet([1]);
+		internal.has(1);
+		internal.delete(1);
+		return internal;
+	})
 </script>
 
-<button onclick={() => (visibleExternal = true)}>external</button>
+<button onclick={() => (outside_basic = true)}>external</button>
-{#if visibleExternal}
+{#if outside_basic}
-	{throws}
+	{throws_basic}
+{/if}
+<button onclick={() => (inside_basic = true)}>internal</button>
+{#if inside_basic}
+	{works_basic}
+{/if}
+
+<button onclick={() => (outside_has_delete = true)}>external</button>
+{#if outside_has_delete}
+	{throws_has_delete}
 {/if}
-<button onclick={() => (visibleInternal = true)}>internal</button>
+<button onclick={() => (inside_has_delete = true)}>internal</button>
-{#if visibleInternal}
+{#if inside_has_delete}
-	{works}
+	{works_has_delete}
 {/if}
 

packages/svelte/tests/runtime-runes/samples/side-effect-derived-spring/_config.js

@@ -0,0 +1,23 @@
+import { flushSync } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	compileOptions: {
+		dev: true,
+		runes: true
+	},
+
+	test({ assert, target }) {
+		const [button1, button2] = target.querySelectorAll('button');
+
+		assert.throws(() => {
+			button1?.click();
+			flushSync();
+		}, /state_unsafe_mutation/);
+
+		assert.doesNotThrow(() => {
+			button2?.click();
+			flushSync();
+		});
+	}
+});

packages/svelte/tests/runtime-runes/samples/side-effect-derived-spring/main.svelte

@@ -0,0 +1,26 @@
+<script>
+	import { Spring } from 'svelte/motion';
+
+	let outside_basic = $state(false);
+	let outside_basic_spring = new Spring(0);
+	const throws_basic = $derived.by(() => {
+		outside_basic_spring.set(1);
+		return outside_basic_spring;
+	})
+
+	let inside_basic = $state(false);
+	const works_basic = $derived.by(() => {
+		let internal = new Spring(0);
+		internal.set(1);
+		return internal;
+	})
+</script>
+
+<button onclick={() => (outside_basic = true)}>external</button>
+{#if outside_basic}
+	{throws_basic}
+{/if}
+<button onclick={() => (inside_basic = true)}>internal</button>
+{#if inside_basic}
+	{works_basic}
+{/if}

packages/svelte/tests/runtime-runes/samples/side-effect-derived-tween/_config.js

@@ -0,0 +1,23 @@
+import { flushSync } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	compileOptions: {
+		dev: true,
+		runes: true
+	},
+
+	test({ assert, target }) {
+		const [button1, button2] = target.querySelectorAll('button');
+
+		assert.throws(() => {
+			button1?.click();
+			flushSync();
+		}, /state_unsafe_mutation/);
+
+		assert.doesNotThrow(() => {
+			button2?.click();
+			flushSync();
+		});
+	}
+});

packages/svelte/tests/runtime-runes/samples/side-effect-derived-tween/main.svelte

@@ -0,0 +1,26 @@
+<script>
+	import { Tween } from 'svelte/motion';
+
+	let outside_basic = $state(false);
+	let outside_basic_tween = new Tween(0);
+	const throws_basic = $derived.by(() => {
+		outside_basic_tween.set(1);
+		return outside_basic_tween;
+	})
+
+	let inside_basic = $state(false);
+	const works_basic = $derived.by(() => {
+		let internal = new Tween(0);
+		internal.set(1);
+		return internal;
+	})
+</script>
+
+<button onclick={() => (outside_basic = true)}>external</button>
+{#if outside_basic}
+	{throws_basic}
+{/if}
+<button onclick={() => (inside_basic = true)}>internal</button>
+{#if inside_basic}
+	{works_basic}
+{/if}