Merge pull request #21122 from megothss/fix/tracked-collections-delete-return-value

[BUGFIX] Fix tracked collections delete() returning true for non-existent entries

Author: NullVoxPopuli

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

@@ -151,7 +151,7 @@ class TrackedMap<K = unknown, V = unknown> implements Map<K, V> {
   }
 
   delete(key: K): boolean {
-    if (!this.#vals.has(key)) return true;
+    if (!this.#vals.has(key)) return false;
 
     this.#dirtyStorageFor(key);
     DIRTY_TAG(this.#collection);

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

@@ -140,7 +140,7 @@ class TrackedSet<T = unknown> implements Set<T> {
   }
 
   delete(value: T): boolean {
-    if (!this.#vals.has(value)) return true;
+    if (!this.#vals.has(value)) return false;
 
     this.#dirtyStorageFor(value);
     DIRTY_TAG(this.#collection);

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

@@ -69,7 +69,7 @@ class TrackedWeakMap<K extends WeakKey = object, V = unknown> implements WeakMap
   }
 
   delete(key: K): boolean {
-    if (!this.#vals.has(key)) return true;
+    if (!this.#vals.has(key)) return false;
 
     this.#dirtyStorageFor(key);
 

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

@@ -71,7 +71,7 @@ class TrackedWeakSet<T extends WeakKey = object> implements WeakSet<T> {
   }
 
   delete(value: T): boolean {
-    if (!this.#vals.has(value)) return true;
+    if (!this.#vals.has(value)) return false;
 
     this.#dirtyStorageFor(value);
 

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

@@ -181,11 +181,12 @@ module('@glimmer/validator: trackedMap', function () {
     const map = trackedMap();
 
     assert.false(map.has(0));
+    assert.false(map.delete(0), 'returns false when key does not exist');
 
     map.set(0, 123);
     assert.true(map.has(0));
 
-    map.delete(0);
+    assert.true(map.delete(0), 'returns true when key exists');
     assert.false(map.has(0));
   });
 

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

@@ -348,11 +348,12 @@ module('@glimmer/validator: trackedSet', function () {
     const set = trackedSet();
 
     assert.false(set.has(0));
+    assert.false(set.delete(0), 'returns false when value does not exist');
 
     set.add(0);
     assert.true(set.has(0));
 
-    set.delete(0);
+    assert.true(set.delete(0), 'returns true when value exists');
     assert.false(set.has(0));
   });
 

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

@@ -68,11 +68,12 @@ module('@glimmer/validator: trackedWeakMap()', function () {
     const map = trackedWeakMap();
 
     assert.false(map.has(obj));
+    assert.false(map.delete(obj), 'returns false when key does not exist');
 
     map.set(obj, 123);
     assert.true(map.has(obj));
 
-    map.delete(obj);
+    assert.true(map.delete(obj), 'returns true when key exists');
     assert.false(map.has(obj));
   });
 });

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

@@ -47,11 +47,12 @@ module('@glimmer/validator: trackedWeakSet()', function () {
     const set = trackedWeakSet();
 
     assert.false(set.has(obj));
+    assert.false(set.delete(obj), 'returns false when value does not exist');
 
     set.add(obj);
     assert.true(set.has(obj));
 
-    set.delete(obj);
+    assert.true(set.delete(obj), 'returns true when value exists');
     assert.false(set.has(obj));
   });
 });