missing keys in removeWhere is no-op

[netlify[bot]]

✅ Deploy Preview for blinkdb canceled.

Name	Link
🔨 Latest commit	973643d8a93c79aea57c25cad6309ed6f9bf9ac6
🔍 Latest deploy log	https://app.netlify.com/sites/blinkdb/deploys/6530e9331c280a0008288ceb

[retorquere]

In this PR, removeMany will still return false when the entities list has duplicates.

[maradotwebp]

In this PR, removeMany will still return false when the entities list has duplicates.

Yep, technically removeMany doesn't return whether all entities where deleted, but whether all items in the parameter list were deleted.

I think it best to change the return value to a number of how many entities were deleted. It's a breaking change, but contains more information and handles duplicates (removeMany([{ id: 1 }, { id: 1 }]) will then return 1).

[codecov[bot]]

Codecov Report

Merging #30 (6658a7d) into main (9274eee) will increase coverage by 0.07%. Report is 27 commits behind head on main. The diff coverage is 92.90%.

:exclamation: Current head 6658a7d differs from pull request most recent head 973643d. Consider uploading reports for the commit 973643d to get more accurate results

@@            Coverage Diff             @@
##             main      #30      +/-   ##
==========================================
+ Coverage   95.08%   95.15%   +0.07%     
==========================================
  Files          73       74       +1     
  Lines        1119     1136      +17     
  Branches      269      263       -6     
==========================================
+ Hits         1064     1081      +17     
  Misses         55       55              

Files	Coverage Δ
packages/db/src/core/insertMany.ts	100.00% <100.00%> (ø)
packages/db/src/core/many.ts	100.00% <100.00%> (ø)
packages/db/src/core/removeMany.ts	100.00% <100.00%> (ø)
packages/db/src/core/table.utils.ts	100.00% <100.00%> (ø)
packages/db/src/core/upsertMany.ts	100.00% <100.00%> (ø)
packages/db/src/core/use.ts	100.00% <ø> (ø)
packages/db/src/core/watch.ts	95.23% <100.00%> (ø)
packages/db/src/events/Dispatcher.ts	100.00% <100.00%> (ø)
packages/db/src/query/select/where.ts	100.00% <100.00%> (ø)
packages/db/src/types.ts	94.73% <ø> (ø)
... and 12 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[retorquere]

In this PR, removeMany will still return false when the entities list has duplicates.

Yep, technically removeMany doesn't return whether all entities where deleted, but whether all items in the parameter list were deleted.

I think it best to change the return value to a number of how many entities were deleted. It's a breaking change, but contains more information and handles duplicates (removeMany([{ id: 1 }, { id: 1 }]) will then return 1).

Do you want me to include this change?

[retorquere]

The switch to counting would amount to

diff --git a/packages/db/src/core/removeMany.ts b/packages/db/src/core/removeMany.ts
index 15b6843..43f5ab2 100644
--- a/packages/db/src/core/removeMany.ts
+++ b/packages/db/src/core/removeMany.ts
@@ -32,9 +32,9 @@ export function removeMany<T extends Entity<T>, P extends PrimaryKeyOf<T>>(
 export function internalRemoveMany<T extends Entity<T>, P extends PrimaryKeyOf<T>>(
   table: Table<T, P>,
   entities: Ids<T, P>[]
-): Promise<boolean> {
+): Promise<number> {
   const events: { entity: T }[] = [];
-  let allEntitiesRemoved = true;
+  let removed = 0;
   for (const entity of entities) {
     const primaryKeyProperty = table[BlinkKey].options.primary;
     const primaryKey = entity[primaryKeyProperty];
@@ -42,10 +42,7 @@ export function internalRemoveMany<T extends Entity<T>, P extends PrimaryKeyOf<T
     const indexes = table[BlinkKey].storage.indexes;
     if (Object.keys(indexes).length > 0) {
       const item = table[BlinkKey].storage.primary.get(primaryKey);
-      if (!item) {
-        allEntitiesRemoved = false;
-        continue;
-      }
+      if (!item) continue;
       for (const property in indexes) {
         const btree = indexes[property]!;
         const key = item[property];
@@ -68,8 +65,8 @@ export function internalRemoveMany<T extends Entity<T>, P extends PrimaryKeyOf<T
     if (hasDeleted) {
       table[BlinkKey].storage.primary.totalItemSize--;
     }
-    allEntitiesRemoved = allEntitiesRemoved && hasDeleted;
+    removed++;
   }
   void table[BlinkKey].events.onRemove.dispatch(events);
-  return Promise.resolve(allEntitiesRemoved);
+  return Promise.resolve(removed);
 }

[maradotwebp]

Do you want me to include this change?

No, I can do that in another PR. Thanks for your code tho.

[maradotwebp]

See https://github.com/blinkdb-js/blinkdb/releases/tag/v0.14.0.