diff --git a/packages/svelte/src/reactivity/map.js b/packages/svelte/src/reactivity/map.js index bb52cdafc2..e4a54d1792 100644 --- a/packages/svelte/src/reactivity/map.js +++ b/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 | 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} + */ + #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); - is_new = true; + s = this.#source(0); 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); - is_new = true; + s = this.#source(0); 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); - new_sources.add(s); + var s = this.#source(0); 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); - } } } diff --git a/packages/svelte/src/reactivity/set.js b/packages/svelte/src/reactivity/set.js index 7bb5da8705..3f6c0db8e8 100644 --- a/packages/svelte/src/reactivity/set.js +++ b/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 | 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} + */ + #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); - is_new = true; + s = this.#source(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; } diff --git a/packages/svelte/tests/runtime-runes/samples/side-effect-derived-map/_config.js b/packages/svelte/tests/runtime-runes/samples/side-effect-derived-map/_config.js index 5ad6f57e31..c10dc7fb55 100644 --- a/packages/svelte/tests/runtime-runes/samples/side-effect-derived-map/_config.js +++ b/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(); + }); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/side-effect-derived-map/main.svelte b/packages/svelte/tests/runtime-runes/samples/side-effect-derived-map/main.svelte index bdd5ccb75c..c37f37ceb6 100644 --- a/packages/svelte/tests/runtime-runes/samples/side-effect-derived-map/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/side-effect-derived-map/main.svelte @@ -1,27 +1,101 @@ - -{#if visibleExternal} - {throws} + +{#if outside_basic} + {throw_basic} +{/if} + +{#if inside_basic} + {works_basic} +{/if} + + +{#if outside_has} + {throw_has} {/if} - -{#if visibleInternal} - {works} + +{#if inside_has} + {works_has} {/if} + +{#if outside_get} + {throw_get} +{/if} + +{#if inside_get} + {works_get} +{/if} + + +{#if outside_values} + {throw_values} +{/if} + +{#if inside_values} + {works_values} +{/if} diff --git a/packages/svelte/tests/runtime-runes/samples/side-effect-derived-set/_config.js b/packages/svelte/tests/runtime-runes/samples/side-effect-derived-set/_config.js index 5ad6f57e31..5cf066fb8a 100644 --- a/packages/svelte/tests/runtime-runes/samples/side-effect-derived-set/_config.js +++ b/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(); + }); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/side-effect-derived-set/main.svelte b/packages/svelte/tests/runtime-runes/samples/side-effect-derived-set/main.svelte index 8564f6e7c4..1d6735ba64 100644 --- a/packages/svelte/tests/runtime-runes/samples/side-effect-derived-set/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/side-effect-derived-set/main.svelte @@ -1,27 +1,52 @@ - -{#if visibleExternal} - {throws} + +{#if outside_basic} + {throws_basic} +{/if} + +{#if inside_basic} + {works_basic} +{/if} + + +{#if outside_has_delete} + {throws_has_delete} {/if} - -{#if visibleInternal} - {works} + +{#if inside_has_delete} + {works_has_delete} {/if} diff --git a/packages/svelte/tests/runtime-runes/samples/side-effect-derived-spring/_config.js b/packages/svelte/tests/runtime-runes/samples/side-effect-derived-spring/_config.js new file mode 100644 index 0000000000..5ad6f57e31 --- /dev/null +++ b/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(); + }); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/side-effect-derived-spring/main.svelte b/packages/svelte/tests/runtime-runes/samples/side-effect-derived-spring/main.svelte new file mode 100644 index 0000000000..b0818deca9 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/side-effect-derived-spring/main.svelte @@ -0,0 +1,26 @@ + + + +{#if outside_basic} + {throws_basic} +{/if} + +{#if inside_basic} + {works_basic} +{/if} \ No newline at end of file diff --git a/packages/svelte/tests/runtime-runes/samples/side-effect-derived-tween/_config.js b/packages/svelte/tests/runtime-runes/samples/side-effect-derived-tween/_config.js new file mode 100644 index 0000000000..5ad6f57e31 --- /dev/null +++ b/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(); + }); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/side-effect-derived-tween/main.svelte b/packages/svelte/tests/runtime-runes/samples/side-effect-derived-tween/main.svelte new file mode 100644 index 0000000000..bd007f2b50 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/side-effect-derived-tween/main.svelte @@ -0,0 +1,26 @@ + + + +{#if outside_basic} + {throws_basic} +{/if} + +{#if inside_basic} + {works_basic} +{/if} \ No newline at end of file