Skip to content

Commit 74d18ad

Browse files
trueadmtrueadm
committed
fix: improve reactive Map and Set implementations
1 parent fd942b7 commit 74d18ad

File tree

6 files changed

+80
-69
lines changed

6 files changed

+80
-69
lines changed

‎.changeset/eight-jeans-compare.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"svelte": patch
3+
---
4+
5+
fix: improve reactive Map and Set implementations

‎packages/svelte/src/internal/client/dev/inspect.js

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -62,16 +62,6 @@ export function inspect(get_value, inspector = console.log) {
6262
*/
6363
function deep_snapshot(value, visited = new Map()) {
6464
if (typeof value === 'object' && value !== null && !visited.has(value)) {
65-
if (DEV) {
66-
// When dealing with ReactiveMap or ReactiveSet, return normal versions
67-
// so that console.log provides better output versions
68-
if (value instanceof Map && value.constructor !== Map) {
69-
return new Map(value);
70-
}
71-
if (value instanceof Set && value.constructor !== Set) {
72-
return new Set(value);
73-
}
74-
}
7565
const unstated = snapshot(value);
7666

7767
if (unstated !== value) {

‎packages/svelte/src/reactivity/map.js

Lines changed: 39 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,14 @@ import { DEV } from 'esm-env';
22
import { source, set } from '../internal/client/reactivity/sources.js';
33
import { get } from '../internal/client/runtime.js';
44
import { UNINITIALIZED } from '../constants.js';
5-
import { map } from './utils.js';
65

76
/**
87
* @template K
98
* @template V
109
* @extends {Map<K, V>}
1110
*/
1211
export class ReactiveMap extends Map {
13-
/** @type {Map<K, import('#client').Source<V>>} */
12+
/** @type {Map<K, import('#client').Source<symbol>>} */
1413
#sources = new Map();
1514
#version = source(0);
1615
#size = source(0);
@@ -28,7 +27,8 @@ export class ReactiveMap extends Map {
2827
var sources = this.#sources;
2928

3029
for (var [key, v] of value) {
31-
sources.set(key, source(v));
30+
sources.set(key, source(Symbol()));
31+
super.set(key, v);
3232
}
3333

3434
this.#size.v = sources.size;
@@ -41,18 +41,24 @@ export class ReactiveMap extends Map {
4141

4242
/** @param {K} key */
4343
has(key) {
44-
var s = this.#sources.get(key);
44+
var sources = this.#sources;
45+
var s = sources.get(key);
4546

4647
if (s === undefined) {
47-
// We should always track the version in case
48-
// the Set ever gets this value in the future.
49-
get(this.#version);
50-
51-
return false;
48+
// If we're working with a non-primitive then we can generate a signal for it
49+
if (typeof key !== 'object' || key === null) {
50+
s = source(UNINITIALIZED);
51+
sources.set(key, s);
52+
} else {
53+
// We should always track the version in case
54+
// the Set ever gets this value in the future.
55+
get(this.#version);
56+
return false;
57+
}
5258
}
5359

5460
get(s);
55-
return true;
61+
return super.has(key);
5662
}
5763

5864
/**
@@ -62,8 +68,7 @@ export class ReactiveMap extends Map {
6268
forEach(callbackfn, this_arg) {
6369
get(this.#version);
6470

65-
var bound_callbackfn = callbackfn.bind(this_arg);
66-
this.#sources.forEach((s, key) => bound_callbackfn(s.v, key, this));
71+
return super.forEach(callbackfn, this_arg);
6772
}
6873

6974
/** @param {K} key */
@@ -78,7 +83,8 @@ export class ReactiveMap extends Map {
7883
return undefined;
7984
}
8085

81-
return get(s);
86+
get(s);
87+
return super.get(key);
8288
}
8389

8490
/**
@@ -88,32 +94,37 @@ export class ReactiveMap extends Map {
8894
set(key, value) {
8995
var sources = this.#sources;
9096
var s = sources.get(key);
97+
var prev_res = super.get(key);
98+
var res = super.set(key, value);
9199

92100
if (s === undefined) {
93-
sources.set(key, source(value));
94-
set(this.#size, sources.size);
101+
sources.set(key, source(Symbol()));
102+
set(this.#size, super.size);
95103
this.#increment_version();
96-
} else {
97-
set(s, value);
104+
} else if (prev_res !== value) {
105+
set(s, Symbol());
98106
}
99107

100-
return this;
108+
return res;
101109
}
102110

103111
/** @param {K} key */
104112
delete(key) {
105113
var sources = this.#sources;
106114
var s = sources.get(key);
115+
var res = super.delete(key);
107116

108117
if (s !== undefined) {
109-
var removed = sources.delete(key);
110-
set(this.#size, sources.size);
111-
set(s, /** @type {V} */ (UNINITIALIZED));
118+
// Remove non-primitive keys
119+
if (typeof key === 'object' && key !== null) {
120+
sources.delete(key);
121+
}
122+
set(this.#size, super.size);
123+
set(s, UNINITIALIZED);
112124
this.#increment_version();
113-
return removed;
114125
}
115126

116-
return false;
127+
return res;
117128
}
118129

119130
clear() {
@@ -122,31 +133,28 @@ export class ReactiveMap extends Map {
122133
if (sources.size !== 0) {
123134
set(this.#size, 0);
124135
for (var s of sources.values()) {
125-
set(s, /** @type {V} */ (UNINITIALIZED));
136+
set(s, UNINITIALIZED);
126137
}
127138
this.#increment_version();
128139
}
129140

130141
sources.clear();
142+
super.clear();
131143
}
132144

133145
keys() {
134146
get(this.#version);
135-
return this.#sources.keys();
147+
return super.keys();
136148
}
137149

138150
values() {
139151
get(this.#version);
140-
return map(this.#sources.values(), get, 'Map Iterator');
152+
return super.values();
141153
}
142154

143155
entries() {
144156
get(this.#version);
145-
return map(
146-
this.#sources.entries(),
147-
([key, source]) => /** @type {[K, V]} */ ([key, get(source)]),
148-
'Map Iterator'
149-
);
157+
return super.entries();
150158
}
151159

152160
[Symbol.iterator]() {

‎packages/svelte/src/reactivity/map.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ test('map.values()', () => {
3636
map.clear();
3737
});
3838

39-
assert.deepEqual(log, [5, true, [1, 2, 3, 4, 5], 4, false, [1, 2, 4, 5], 0, false, []]);
39+
assert.deepEqual(log, [5, true, [1, 2, 3, 4, 5], 4, false, [1, 2, 4, 5], 0, []]);
4040

4141
cleanup();
4242
});

‎packages/svelte/src/reactivity/set.js

Lines changed: 34 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { DEV } from 'esm-env';
22
import { source, set } from '../internal/client/reactivity/sources.js';
33
import { get } from '../internal/client/runtime.js';
4-
import { map } from './utils.js';
54

65
var read_methods = ['forEach', 'isDisjointFrom', 'isSubsetOf', 'isSupersetOf'];
76
var set_like_methods = ['difference', 'intersection', 'symmetricDifference', 'union'];
@@ -32,6 +31,7 @@ export class ReactiveSet extends Set {
3231

3332
for (var element of value) {
3433
sources.set(element, source(true));
34+
super.add(element);
3535
}
3636

3737
this.#size.v = sources.size;
@@ -51,23 +51,17 @@ export class ReactiveSet extends Set {
5151
// @ts-ignore
5252
proto[method] = function (...v) {
5353
get(this.#version);
54-
// We don't populate the underlying Set, so we need to create a clone using
55-
// our internal values and then pass that to the method.
56-
var clone = new Set(this.values());
5754
// @ts-ignore
58-
return set_proto[method].apply(clone, v);
55+
return set_proto[method].apply(this, v);
5956
};
6057
}
6158

6259
for (const method of set_like_methods) {
6360
// @ts-ignore
6461
proto[method] = function (...v) {
6562
get(this.#version);
66-
// We don't populate the underlying Set, so we need to create a clone using
67-
// our internal values and then pass that to the method.
68-
var clone = new Set(this.values());
6963
// @ts-ignore
70-
var set = /** @type {Set<T>} */ (set_proto[method].apply(clone, v));
64+
var set = /** @type {Set<T>} */ (set_proto[method].apply(this, v));
7165
return new ReactiveSet(set);
7266
};
7367
}
@@ -79,46 +73,57 @@ export class ReactiveSet extends Set {
7973

8074
/** @param {T} value */
8175
has(value) {
82-
var s = this.#sources.get(value);
76+
var sources = this.#sources;
77+
var s = sources.get(value);
8378

8479
if (s === undefined) {
85-
// We should always track the version in case
86-
// the Set ever gets this value in the future.
87-
get(this.#version);
88-
89-
return false;
80+
// If we're working with a non-primitive then we can generate a signal for it
81+
if (typeof value !== 'object' || value === null) {
82+
s = source(false);
83+
sources.set(value, s);
84+
} else {
85+
// We should always track the version in case
86+
// the Set ever gets this value in the future.
87+
get(this.#version);
88+
return false;
89+
}
9090
}
9191

92-
return get(s);
92+
get(s);
93+
return super.has(value);
9394
}
9495

9596
/** @param {T} value */
9697
add(value) {
9798
var sources = this.#sources;
99+
var res = super.add(value);
100+
var s = sources.get(value);
98101

99-
if (!sources.has(value)) {
102+
if (s === undefined) {
100103
sources.set(value, source(true));
101-
set(this.#size, sources.size);
104+
set(this.#size, super.size);
102105
this.#increment_version();
106+
} else {
107+
set(s, true);
103108
}
104109

105-
return this;
110+
return res;
106111
}
107112

108113
/** @param {T} value */
109114
delete(value) {
110115
var sources = this.#sources;
111116
var s = sources.get(value);
117+
var res = super.delete(value);
112118

113119
if (s !== undefined) {
114-
var removed = sources.delete(value);
115-
set(this.#size, sources.size);
120+
sources.delete(value);
121+
set(this.#size, super.size);
116122
set(s, false);
117123
this.#increment_version();
118-
return removed;
119124
}
120125

121-
return false;
126+
return res;
122127
}
123128

124129
clear() {
@@ -133,19 +138,22 @@ export class ReactiveSet extends Set {
133138
}
134139

135140
sources.clear();
141+
super.clear();
136142
}
137143

138144
keys() {
139145
get(this.#version);
140-
return map(this.#sources.keys(), (key) => key, 'Set Iterator');
146+
return super.keys();
141147
}
142148

143149
values() {
144-
return this.keys();
150+
get(this.#version);
151+
return super.values();
145152
}
146153

147154
entries() {
148-
return map(this.keys(), (key) => /** @type {[T, T]} */ ([key, key]), 'Set Iterator');
155+
get(this.#version);
156+
return super.entries();
149157
}
150158

151159
[Symbol.iterator]() {

‎packages/svelte/src/reactivity/set.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ test('set.values()', () => {
3030
set.clear();
3131
});
3232

33-
assert.deepEqual(log, [5, true, [1, 2, 3, 4, 5], 4, false, [1, 2, 4, 5], 0, false, []]);
33+
assert.deepEqual(log, [5, true, [1, 2, 3, 4, 5], 4, false, [1, 2, 4, 5], 0, []]);
3434

3535
cleanup();
3636
});

0 commit comments

Comments
 (0)