Skip to content

fix: use state instead of source in reactive classes #16239

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Jul 9, 2025
6 changes: 5 additions & 1 deletion packages/svelte/src/internal/client/runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,8 @@ let write_version = 1;
/** @type {number} Used to version each read of a source of derived to avoid duplicating depedencies inside a reaction */
let read_version = 0;

export let update_version = read_version;

// If we are working with a get() chain that has no active container,
// to prevent memory leaks, we skip adding the reaction.
export let skip_reaction = false;
Expand Down Expand Up @@ -265,6 +267,7 @@ export function update_reaction(reaction) {
var previous_reaction_sources = source_ownership;
var previous_component_context = component_context;
var previous_untracking = untracking;
var previous_update_version = update_version;

var flags = reaction.f;

Expand All @@ -278,7 +281,7 @@ export function update_reaction(reaction) {
source_ownership = null;
set_component_context(reaction.ctx);
untracking = false;
read_version++;
update_version = ++read_version;

reaction.f |= EFFECT_IS_UPDATING;

Expand Down Expand Up @@ -366,6 +369,7 @@ export function update_reaction(reaction) {
source_ownership = previous_reaction_sources;
set_component_context(previous_component_context);
untracking = previous_untracking;
update_version = previous_update_version;

reaction.f ^= EFFECT_IS_UPDATING;
}
Expand Down
12 changes: 6 additions & 6 deletions packages/svelte/src/motion/spring.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { writable } from '../store/shared/index.js';
import { loop } from '../internal/client/loop.js';
import { raf } from '../internal/client/timing.js';
import { is_date } from './utils.js';
import { set, source } from '../internal/client/reactivity/sources.js';
import { set, state } from '../internal/client/reactivity/sources.js';
import { render_effect } from '../internal/client/reactivity/effects.js';
import { tag } from '../internal/client/dev/tracing.js';
import { get } from '../internal/client/runtime.js';
Expand Down Expand Up @@ -170,9 +170,9 @@ export function spring(value, opts = {}) {
* @since 5.8.0
*/
export class Spring {
#stiffness = source(0.15);
#damping = source(0.8);
#precision = source(0.01);
#stiffness = state(0.15);
#damping = state(0.8);
#precision = state(0.01);

#current;
#target;
Expand All @@ -194,8 +194,8 @@ export class Spring {
* @param {SpringOpts} [options]
*/
constructor(value, options = {}) {
this.#current = DEV ? tag(source(value), 'Spring.current') : source(value);
this.#target = DEV ? tag(source(value), 'Spring.target') : source(value);
this.#current = DEV ? tag(state(value), 'Spring.current') : state(value);
this.#target = DEV ? tag(state(value), 'Spring.target') : state(value);

if (typeof options.stiffness === 'number') this.#stiffness.v = clamp(options.stiffness, 0, 1);
if (typeof options.damping === 'number') this.#damping.v = clamp(options.damping, 0, 1);
Expand Down
6 changes: 3 additions & 3 deletions packages/svelte/src/motion/tweened.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { raf } from '../internal/client/timing.js';
import { loop } from '../internal/client/loop.js';
import { linear } from '../easing/index.js';
import { is_date } from './utils.js';
import { set, source } from '../internal/client/reactivity/sources.js';
import { set, state } from '../internal/client/reactivity/sources.js';
import { tag } from '../internal/client/dev/tracing.js';
import { get, render_effect } from 'svelte/internal/client';
import { DEV } from 'esm-env';
Expand Down Expand Up @@ -191,8 +191,8 @@ export class Tween {
* @param {TweenedOptions<T>} options
*/
constructor(value, options = {}) {
this.#current = source(value);
this.#target = source(value);
this.#current = state(value);
this.#target = state(value);
this.#defaults = options;

if (DEV) {
Expand Down
25 changes: 19 additions & 6 deletions packages/svelte/src/reactivity/map.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
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 } from '../internal/client/runtime.js';
import { get, update_version } from '../internal/client/runtime.js';
import { increment } from './utils.js';

/**
Expand Down Expand Up @@ -56,6 +56,7 @@ export class SvelteMap extends Map {
#sources = new Map();
#version = state(0);
#size = state(0);
#update_version = update_version || -1;

/**
* @param {Iterable<readonly [K, V]> | null | undefined} [value]
Expand All @@ -79,6 +80,19 @@ export class SvelteMap extends Map {
}
}

/**
* If the source is being created inside the same reaction as the SvelteMap instance,
* 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) {
return update_version === this.#update_version ? state(value) : source(value);
}

/** @param {K} key */
has(key) {
var sources = this.#sources;
Expand All @@ -87,7 +101,7 @@ export class SvelteMap extends Map {
if (s === undefined) {
var ret = super.get(key);
if (ret !== undefined) {
s = source(0);
s = this.#source(0);

if (DEV) {
tag(s, `SvelteMap get(${label(key)})`);
Expand Down Expand Up @@ -123,7 +137,7 @@ export class SvelteMap extends Map {
if (s === undefined) {
var ret = super.get(key);
if (ret !== undefined) {
s = source(0);
s = this.#source(0);

if (DEV) {
tag(s, `SvelteMap get(${label(key)})`);
Expand Down Expand Up @@ -154,7 +168,7 @@ export class SvelteMap extends Map {
var version = this.#version;

if (s === undefined) {
s = source(0);
s = this.#source(0);

if (DEV) {
tag(s, `SvelteMap get(${label(key)})`);
Expand Down Expand Up @@ -219,8 +233,7 @@ export class SvelteMap extends Map {
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);
if (DEV) {
tag(s, `SvelteMap get(${label(key)})`);
}
Expand Down
18 changes: 16 additions & 2 deletions packages/svelte/src/reactivity/set.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
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 } from '../internal/client/runtime.js';
import { get, update_version } from '../internal/client/runtime.js';
import { increment } from './utils.js';

var read_methods = ['forEach', 'isDisjointFrom', 'isSubsetOf', 'isSupersetOf'];
Expand Down Expand Up @@ -50,6 +50,7 @@ export class SvelteSet extends Set {
#sources = new Map();
#version = state(0);
#size = state(0);
#update_version = update_version || -1;

/**
* @param {Iterable<T> | null | undefined} [value]
Expand All @@ -75,6 +76,19 @@ export class SvelteSet extends Set {
if (!inited) this.#init();
}

/**
* If the source is being created inside the same reaction as the SvelteSet instance,
* 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) {
return update_version === this.#update_version ? state(value) : source(value);
}

// We init as part of the first instance so that we can treeshake this class
#init() {
inited = true;
Expand Down Expand Up @@ -116,7 +130,7 @@ export class SvelteSet extends Set {
return false;
}

s = source(true);
s = this.#source(true);

if (DEV) {
tag(s, `SvelteSet has(${label(value)})`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
});
}
});
Original file line number Diff line number Diff line change
@@ -1,27 +1,101 @@
<script>
import { SvelteMap } from 'svelte/reactivity';

let visibleExternal = $state(false);
let external = new SvelteMap();
const throws = $derived.by(() => {
external.set(1, 1);
return external;
let outside_basic = $state(false);
let outside_basic_map = new SvelteMap();
const throw_basic = $derived.by(() => {
outside_basic_map.set(1, 1);
return outside_basic_map;
});

let visibleInternal = $state(false);
const works = $derived.by(() => {
let internal = new SvelteMap();
internal.set(1, 1);
return internal;
let inside_basic = $state(false);
const works_basic = $derived.by(() => {
let inside = new SvelteMap();
inside.set(1, 1);
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>
{#if visibleExternal}
{throws}
<button onclick={() => (outside_basic = true)}>external</button>
{#if outside_basic}
{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>
{#if visibleInternal}
{works}
<button onclick={() => (inside_has = true)}>internal</button>
{#if inside_has}
{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}
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -19,5 +19,15 @@ export default test({
button2?.click();
flushSync();
});

assert.throws(() => {
button3?.click();
flushSync();
}, /state_unsafe_mutation/);

assert.doesNotThrow(() => {
button4?.click();
flushSync();
});
}
});
Loading