Skip to content

Commit e7301af

Browse files
authored
fix: no data duplication in reactive Set/Map (#11200)
* fix: get rid of data duplication in reactive map * fix: get rid of data duplication in reactive set
1 parent 1510c13 commit e7301af

File tree

4 files changed

+53
-11
lines changed

4 files changed

+53
-11
lines changed

packages/svelte/src/reactivity/map.js

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ export class ReactiveMap extends Map {
2828

2929
for (var [key, v] of value) {
3030
sources.set(key, source(v));
31-
super.set(key, v);
3231
}
3332

3433
this.#size.v = sources.size;
@@ -62,7 +61,8 @@ export class ReactiveMap extends Map {
6261
forEach(callbackfn, this_arg) {
6362
get(this.#version);
6463

65-
return super.forEach(callbackfn, this_arg);
64+
var bound_callbackfn = callbackfn.bind(this_arg);
65+
this.#sources.forEach((s, key) => bound_callbackfn(s.v, key, this));
6666
}
6767

6868
/** @param {K} key */
@@ -96,7 +96,7 @@ export class ReactiveMap extends Map {
9696
set(s, value);
9797
}
9898

99-
return super.set(key, value);
99+
return this;
100100
}
101101

102102
/** @param {K} key */
@@ -105,13 +105,14 @@ export class ReactiveMap extends Map {
105105
var s = sources.get(key);
106106

107107
if (s !== undefined) {
108-
sources.delete(key);
108+
var removed = sources.delete(key);
109109
set(this.#size, sources.size);
110110
set(s, /** @type {V} */ (UNINITIALIZED));
111111
this.#increment_version();
112+
return removed;
112113
}
113114

114-
return super.delete(key);
115+
return false;
115116
}
116117

117118
clear() {
@@ -126,7 +127,6 @@ export class ReactiveMap extends Map {
126127
}
127128

128129
sources.clear();
129-
super.clear();
130130
}
131131

132132
keys() {

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,40 @@ test('map.has(...)', () => {
124124
cleanup();
125125
});
126126

127+
test('map.forEach(...)', () => {
128+
const map = new ReactiveMap([
129+
[1, 1],
130+
[2, 2],
131+
[3, 3]
132+
]);
133+
134+
const log: any = [];
135+
const this_arg = {};
136+
137+
map.forEach(function (this: unknown, ...args) {
138+
log.push([...args, this]);
139+
}, this_arg);
140+
141+
assert.deepEqual(log, [
142+
[1, 1, map, this_arg],
143+
[2, 2, map, this_arg],
144+
[3, 3, map, this_arg]
145+
]);
146+
});
147+
148+
test('map.delete(...)', () => {
149+
const map = new ReactiveMap([
150+
[1, 1],
151+
[2, 2],
152+
[3, 3]
153+
]);
154+
155+
assert.equal(map.delete(3), true);
156+
assert.equal(map.delete(3), false);
157+
158+
assert.deepEqual(Array.from(map.values()), [1, 2]);
159+
});
160+
127161
test('map handling of undefined values', () => {
128162
const map = new ReactiveMap();
129163

packages/svelte/src/reactivity/set.js

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ export class ReactiveSet extends Set {
3131

3232
for (var element of value) {
3333
sources.set(element, source(true));
34-
super.add(element);
3534
}
3635

3736
this.#size.v = sources.size;
@@ -97,7 +96,7 @@ export class ReactiveSet extends Set {
9796
this.#increment_version();
9897
}
9998

100-
return super.add(value);
99+
return this;
101100
}
102101

103102
/** @param {T} value */
@@ -106,13 +105,14 @@ export class ReactiveSet extends Set {
106105
var s = sources.get(value);
107106

108107
if (s !== undefined) {
109-
sources.delete(value);
108+
var removed = sources.delete(value);
110109
set(this.#size, sources.size);
111110
set(s, false);
112111
this.#increment_version();
112+
return removed;
113113
}
114114

115-
return super.delete(value);
115+
return false;
116116
}
117117

118118
clear() {
@@ -127,7 +127,6 @@ export class ReactiveSet extends Set {
127127
}
128128

129129
sources.clear();
130-
super.clear();
131130
}
132131

133132
keys() {

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,3 +77,12 @@ test('set.has(...)', () => {
7777

7878
cleanup();
7979
});
80+
81+
test('set.delete(...)', () => {
82+
const set = new ReactiveSet([1, 2, 3]);
83+
84+
assert.equal(set.delete(3), true);
85+
assert.equal(set.delete(3), false);
86+
87+
assert.deepEqual(Array.from(set.values()), [1, 2]);
88+
});

0 commit comments

Comments
 (0)