Skip to content

Commit 2e69ed4

Browse files
committed
fix: use active_reaction as indication to use source or state
1 parent de8053e commit 2e69ed4

File tree

10 files changed

+318
-68
lines changed

10 files changed

+318
-68
lines changed

packages/svelte/src/reactivity/map.js

Lines changed: 25 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
/** @import { Source } from '#client' */
1+
/** @import { Reaction, Source } from '#client' */
22
import { DEV } from 'esm-env';
33
import { set, source, state } from '../internal/client/reactivity/sources.js';
44
import { label, tag } from '../internal/client/dev/tracing.js';
5-
import { get, push_reaction_value } from '../internal/client/runtime.js';
5+
import { active_reaction, get, push_reaction_value } from '../internal/client/runtime.js';
66
import { increment } from './utils.js';
77

88
/**
@@ -56,13 +56,17 @@ export class SvelteMap extends Map {
5656
#sources = new Map();
5757
#version = state(0);
5858
#size = state(0);
59+
/**@type {Reaction | null} */
60+
#initial_reaction = null;
5961

6062
/**
6163
* @param {Iterable<readonly [K, V]> | null | undefined} [value]
6264
*/
6365
constructor(value) {
6466
super();
6567

68+
this.#initial_reaction = active_reaction;
69+
6670
if (DEV) {
6771
// If the value is invalid then the native exception will fire here
6872
value = new Map(value);
@@ -79,17 +83,31 @@ export class SvelteMap extends Map {
7983
}
8084
}
8185

86+
/**
87+
* If the active_reaction is the same as the reaction that created this SvelteMap,
88+
* we use state so that it will not be a dependency of the reaction. Otherwise we
89+
* use source so it will be.
90+
*
91+
* @template T
92+
* @param {T} value
93+
* @returns {Source<T>}
94+
*/
95+
#source(value) {
96+
if (this.#initial_reaction === active_reaction) {
97+
return state(value);
98+
}
99+
return source(value);
100+
}
101+
82102
/** @param {K} key */
83103
has(key) {
84104
var sources = this.#sources;
85105
var s = sources.get(key);
86-
var is_new = false;
87106

88107
if (s === undefined) {
89108
var ret = super.get(key);
90109
if (ret !== undefined) {
91-
s = source(0);
92-
is_new = true;
110+
s = this.#source(0);
93111

94112
if (DEV) {
95113
tag(s, `SvelteMap get(${label(key)})`);
@@ -105,12 +123,6 @@ export class SvelteMap extends Map {
105123
}
106124

107125
get(s);
108-
if (is_new) {
109-
// if it's a new source we want to push to the reactions values
110-
// AFTER we read it so that the effect depends on it but we don't
111-
// trigger an unsafe mutation (since it was created within the derived)
112-
push_reaction_value(s);
113-
}
114126
return true;
115127
}
116128

@@ -127,13 +139,11 @@ export class SvelteMap extends Map {
127139
get(key) {
128140
var sources = this.#sources;
129141
var s = sources.get(key);
130-
var is_new = false;
131142

132143
if (s === undefined) {
133144
var ret = super.get(key);
134145
if (ret !== undefined) {
135-
s = source(0);
136-
is_new = true;
146+
s = this.#source(0);
137147

138148
if (DEV) {
139149
tag(s, `SvelteMap get(${label(key)})`);
@@ -149,12 +159,6 @@ export class SvelteMap extends Map {
149159
}
150160

151161
get(s);
152-
if (is_new) {
153-
// if it's a new source we want to push to the reactions values
154-
// AFTER we read it so that the effect depends on it but we don't
155-
// trigger an unsafe mutation (since it was created within the derived)
156-
push_reaction_value(s);
157-
}
158162
return super.get(key);
159163
}
160164

@@ -232,12 +236,10 @@ export class SvelteMap extends Map {
232236
get(this.#version);
233237

234238
var sources = this.#sources;
235-
var new_sources = new WeakSet();
236239
if (this.#size.v !== sources.size) {
237240
for (var key of super.keys()) {
238241
if (!sources.has(key)) {
239-
var s = source(0);
240-
new_sources.add(s);
242+
var s = this.#source(0);
241243
if (DEV) {
242244
tag(s, `SvelteMap get(${label(key)})`);
243245
}
@@ -249,12 +251,6 @@ export class SvelteMap extends Map {
249251

250252
for ([, s] of this.#sources) {
251253
get(s);
252-
if (new_sources.has(s)) {
253-
// if it's a new source we want to push to the reactions values
254-
// AFTER we read it so that the effect depends on it but we don't
255-
// trigger an unsafe mutation (since it was created within the derived)
256-
push_reaction_value(s);
257-
}
258254
}
259255
}
260256

packages/svelte/src/reactivity/set.js

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
/** @import { Source } from '#client' */
1+
/** @import { Reaction, Source } from '#client' */
22
import { DEV } from 'esm-env';
33
import { source, set, state } from '../internal/client/reactivity/sources.js';
44
import { label, tag } from '../internal/client/dev/tracing.js';
5-
import { get, push_reaction_value } from '../internal/client/runtime.js';
5+
import { active_reaction, get } from '../internal/client/runtime.js';
66
import { increment } from './utils.js';
77

88
var read_methods = ['forEach', 'isDisjointFrom', 'isSubsetOf', 'isSupersetOf'];
@@ -51,12 +51,17 @@ export class SvelteSet extends Set {
5151
#version = state(0);
5252
#size = state(0);
5353

54+
/**@type {Reaction | null}*/
55+
#initial_reaction = null;
56+
5457
/**
5558
* @param {Iterable<T> | null | undefined} [value]
5659
*/
5760
constructor(value) {
5861
super();
5962

63+
this.#initial_reaction = active_reaction;
64+
6065
if (DEV) {
6166
// If the value is invalid then the native exception will fire here
6267
value = new Set(value);
@@ -75,6 +80,22 @@ export class SvelteSet extends Set {
7580
if (!inited) this.#init();
7681
}
7782

83+
/**
84+
* If the active_reaction is the same as the reaction that created this SvelteMap,
85+
* we use state so that it will not be a dependency of the reaction. Otherwise we
86+
* use source so it will be.
87+
*
88+
* @template T
89+
* @param {T} value
90+
* @returns {Source<T>}
91+
*/
92+
#source(value) {
93+
if (this.#initial_reaction === active_reaction) {
94+
return state(value);
95+
}
96+
return source(value);
97+
}
98+
7899
// We init as part of the first instance so that we can treeshake this class
79100
#init() {
80101
inited = true;
@@ -107,7 +128,6 @@ export class SvelteSet extends Set {
107128
var has = super.has(value);
108129
var sources = this.#sources;
109130
var s = sources.get(value);
110-
var is_new = false;
111131

112132
if (s === undefined) {
113133
if (!has) {
@@ -117,8 +137,7 @@ export class SvelteSet extends Set {
117137
return false;
118138
}
119139

120-
s = source(true);
121-
is_new = true;
140+
s = this.#source(true);
122141

123142
if (DEV) {
124143
tag(s, `SvelteSet has(${label(value)})`);
@@ -128,9 +147,6 @@ export class SvelteSet extends Set {
128147
}
129148

130149
get(s);
131-
if (is_new) {
132-
push_reaction_value(s);
133-
}
134150
return has;
135151
}
136152

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

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ export default test({
88
},
99

1010
test({ assert, target }) {
11-
const [button1, button2] = target.querySelectorAll('button');
11+
const [button1, button2, button3, button4, button5, button6, button7, button8] =
12+
target.querySelectorAll('button');
1213

1314
assert.throws(() => {
1415
button1?.click();
@@ -19,5 +20,35 @@ export default test({
1920
button2?.click();
2021
flushSync();
2122
});
23+
24+
assert.throws(() => {
25+
button3?.click();
26+
flushSync();
27+
}, /state_unsafe_mutation/);
28+
29+
assert.doesNotThrow(() => {
30+
button4?.click();
31+
flushSync();
32+
});
33+
34+
assert.throws(() => {
35+
button5?.click();
36+
flushSync();
37+
}, /state_unsafe_mutation/);
38+
39+
assert.doesNotThrow(() => {
40+
button6?.click();
41+
flushSync();
42+
});
43+
44+
assert.throws(() => {
45+
button7?.click();
46+
flushSync();
47+
}, /state_unsafe_mutation/);
48+
49+
assert.doesNotThrow(() => {
50+
button8?.click();
51+
flushSync();
52+
});
2253
}
2354
});
Lines changed: 90 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,101 @@
11
<script>
22
import { SvelteMap } from 'svelte/reactivity';
33
4-
let visibleExternal = $state(false);
5-
let external = new SvelteMap();
6-
const throws = $derived.by(() => {
7-
external.set(1, 1);
8-
return external;
4+
let outside_basic = $state(false);
5+
let outside_basic_map = new SvelteMap();
6+
const throw_basic = $derived.by(() => {
7+
outside_basic_map.set(1, 1);
8+
return outside_basic_map;
99
});
1010
11-
let visibleInternal = $state(false);
12-
const works = $derived.by(() => {
13-
let internal = new SvelteMap();
14-
internal.set(1, 1);
15-
return internal;
11+
let inside_basic = $state(false);
12+
const works_basic = $derived.by(() => {
13+
let inside = new SvelteMap();
14+
inside.set(1, 1);
15+
return inside;
16+
});
17+
18+
let outside_has = $state(false);
19+
let outside_has_map = new SvelteMap([[1, 1]]);
20+
const throw_has = $derived.by(() => {
21+
outside_has_map.has(1);
22+
outside_has_map.set(1, 2);
23+
return outside_has_map;
24+
});
25+
26+
let inside_has = $state(false);
27+
const works_has = $derived.by(() => {
28+
let inside = new SvelteMap([[1, 1]]);
29+
inside.has(1);
30+
inside.set(1, 1);
31+
return inside;
32+
});
33+
34+
let outside_get = $state(false);
35+
let outside_get_map = new SvelteMap([[1, 1]]);
36+
const throw_get = $derived.by(() => {
37+
outside_get_map.get(1);
38+
outside_get_map.set(1, 2);
39+
return outside_get_map;
40+
});
41+
42+
let inside_get = $state(false);
43+
const works_get = $derived.by(() => {
44+
let inside = new SvelteMap([[1, 1]]);
45+
inside.get(1);
46+
inside.set(1, 1);
47+
return inside;
48+
});
49+
50+
let outside_values = $state(false);
51+
let outside_values_map = new SvelteMap([[1, 1]]);
52+
const throw_values = $derived.by(() => {
53+
outside_values_map.values(1);
54+
outside_values_map.set(1, 2);
55+
return outside_values_map;
56+
});
57+
58+
let inside_values = $state(false);
59+
const works_values = $derived.by(() => {
60+
let inside = new SvelteMap([[1, 1]]);
61+
inside.values();
62+
inside.set(1, 1);
63+
return inside;
1664
});
1765
</script>
1866

19-
<button onclick={() => (visibleExternal = true)}>external</button>
20-
{#if visibleExternal}
21-
{throws}
67+
<button onclick={() => (outside_basic = true)}>external</button>
68+
{#if outside_basic}
69+
{throw_basic}
70+
{/if}
71+
<button onclick={() => (inside_basic = true)}>internal</button>
72+
{#if inside_basic}
73+
{works_basic}
74+
{/if}
75+
76+
<button onclick={() => (outside_has = true)}>external</button>
77+
{#if outside_has}
78+
{throw_has}
2279
{/if}
23-
<button onclick={() => (visibleInternal = true)}>internal</button>
24-
{#if visibleInternal}
25-
{works}
80+
<button onclick={() => (inside_has = true)}>internal</button>
81+
{#if inside_has}
82+
{works_has}
2683
{/if}
2784

85+
<button onclick={() => (outside_get = true)}>external</button>
86+
{#if outside_get}
87+
{throw_get}
88+
{/if}
89+
<button onclick={() => (inside_get = true)}>internal</button>
90+
{#if inside_get}
91+
{works_get}
92+
{/if}
93+
94+
<button onclick={() => (outside_values = true)}>external</button>
95+
{#if outside_values}
96+
{throw_values}
97+
{/if}
98+
<button onclick={() => (inside_values = true)}>internal</button>
99+
{#if inside_values}
100+
{works_values}
101+
{/if}

0 commit comments

Comments
 (0)