Skip to content

Commit 6b47d48

Browse files
megothssNullVoxPopuli
authored andcommitted
[BUGFIX] Fix tracked collections delete() returning true for non-existent entries
All four tracked collections (TrackedMap, TrackedSet, TrackedWeakMap, TrackedWeakSet) incorrectly returned true from delete() when the key/value did not exist. Per ECMA-262, delete() must return false if the element was not present. Also adds return-value assertions to the existing delete tests.
1 parent e9c61cc commit 6b47d48

8 files changed

Lines changed: 12 additions & 8 deletions

File tree

packages/@glimmer/validator/lib/collections/map.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ class TrackedMap<K = unknown, V = unknown> implements Map<K, V> {
151151
}
152152

153153
delete(key: K): boolean {
154-
if (!this.#vals.has(key)) return true;
154+
if (!this.#vals.has(key)) return false;
155155

156156
this.#dirtyStorageFor(key);
157157
DIRTY_TAG(this.#collection);

packages/@glimmer/validator/lib/collections/set.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ class TrackedSet<T = unknown> implements Set<T> {
140140
}
141141

142142
delete(value: T): boolean {
143-
if (!this.#vals.has(value)) return true;
143+
if (!this.#vals.has(value)) return false;
144144

145145
this.#dirtyStorageFor(value);
146146
DIRTY_TAG(this.#collection);

packages/@glimmer/validator/lib/collections/weak-map.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ class TrackedWeakMap<K extends WeakKey = object, V = unknown> implements WeakMap
6969
}
7070

7171
delete(key: K): boolean {
72-
if (!this.#vals.has(key)) return true;
72+
if (!this.#vals.has(key)) return false;
7373

7474
this.#dirtyStorageFor(key);
7575

packages/@glimmer/validator/lib/collections/weak-set.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ class TrackedWeakSet<T extends WeakKey = object> implements WeakSet<T> {
7171
}
7272

7373
delete(value: T): boolean {
74-
if (!this.#vals.has(value)) return true;
74+
if (!this.#vals.has(value)) return false;
7575

7676
this.#dirtyStorageFor(value);
7777

packages/@glimmer/validator/test/collections/map-test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,11 +181,12 @@ module('@glimmer/validator: trackedMap', function () {
181181
const map = trackedMap();
182182

183183
assert.false(map.has(0));
184+
assert.false(map.delete(0), 'returns false when key does not exist');
184185

185186
map.set(0, 123);
186187
assert.true(map.has(0));
187188

188-
map.delete(0);
189+
assert.true(map.delete(0), 'returns true when key exists');
189190
assert.false(map.has(0));
190191
});
191192

packages/@glimmer/validator/test/collections/set-test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -348,11 +348,12 @@ module('@glimmer/validator: trackedSet', function () {
348348
const set = trackedSet();
349349

350350
assert.false(set.has(0));
351+
assert.false(set.delete(0), 'returns false when value does not exist');
351352

352353
set.add(0);
353354
assert.true(set.has(0));
354355

355-
set.delete(0);
356+
assert.true(set.delete(0), 'returns true when value exists');
356357
assert.false(set.has(0));
357358
});
358359

packages/@glimmer/validator/test/collections/weak-map-test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,12 @@ module('@glimmer/validator: trackedWeakMap()', function () {
6868
const map = trackedWeakMap();
6969

7070
assert.false(map.has(obj));
71+
assert.false(map.delete(obj), 'returns false when key does not exist');
7172

7273
map.set(obj, 123);
7374
assert.true(map.has(obj));
7475

75-
map.delete(obj);
76+
assert.true(map.delete(obj), 'returns true when key exists');
7677
assert.false(map.has(obj));
7778
});
7879
});

packages/@glimmer/validator/test/collections/weak-set-test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,12 @@ module('@glimmer/validator: trackedWeakSet()', function () {
4747
const set = trackedWeakSet();
4848

4949
assert.false(set.has(obj));
50+
assert.false(set.delete(obj), 'returns false when value does not exist');
5051

5152
set.add(obj);
5253
assert.true(set.has(obj));
5354

54-
set.delete(obj);
55+
assert.true(set.delete(obj), 'returns true when value exists');
5556
assert.false(set.has(obj));
5657
});
5758
});

0 commit comments

Comments
 (0)