Fix RxCollection.cleanup() return type to match TypeScript declaration

[pubkey]

This PR contains:

• A BUGFIX
• IMPROVED TESTS
• IMPROVED typings

Describe the problem you have without this PR

RxCollection.cleanup() is declared in TypeScript to return Promise<boolean>, where true indicates that all cleanable documents have been removed. However, the cleanup plugin implementation was returning Promise<void> (undefined at runtime) because the result from cleanupRxCollection() was not being returned.

This causes a type-to-runtime mismatch where callers expecting a boolean value receive undefined instead.

Changes Made

1. Fixed return type in cleanupRxCollection() (src/plugins/cleanup/cleanup.ts):

   • Changed function signature to explicitly return Promise<boolean>
   • Return false when the collection is closed
   • Return isDone at the end of the function

2. Fixed return statement in cleanup plugin (src/plugins/cleanup/index.ts):

   • Changed RxCollection.cleanup() return type from Promise<void> to Promise<boolean>
   • Added return statement to propagate the result from cleanupRxCollection()

3. Added comprehensive test (test/unit/cleanup.test.ts):

   • Tests that cleanup(0) returns a boolean value
   • Tests that the return value is true when cleanup completes
   • Tests that cleanup() without arguments also returns a boolean
   • Includes documentation of the bug that was fixed

4. Added changelog entry documenting the fix

Test Plan

The added unit test in test/unit/cleanup.test.ts verifies:

• cleanup(0) returns a boolean (not undefined)
• The return value is true when all deleted documents have been cleaned up
• cleanup() without arguments also returns a boolean

Existing cleanup tests continue to pass, ensuring no regression in cleanup functionality.

https://claude.ai/code/session_01FduixuM3gfecWVpxMCFSiC

[github-actions]

⚠️ Verify Test Reproduction: Tests PASSED without the fix (unexpected)

The changed tests do not fail without the source changes from this PR. Please inspect whether the test changes actually test the bug that the source changes fix.

This workflow runs the changed tests without the source fix to verify they reproduce the bug.

Show output

...(truncated, showing last 200 of 1040 lines)
      ✔ unset a value on a query
      ✔ .incrementalPatch()
      ✔ .incrementalModify()
    issues
      ✔ query.$ emits after insert with async storage (run 0) (123ms)
      ✔ #7497 findOne subscription + exec does not return correct result (69ms)
      ✔ #6792 queries must never contain an undefined property (69ms)
      ✔ #278 queryCache breaks when pointer out of bounds (69ms)
      ✔ #585 sort by sub-path not working (69ms)
      ✔ #698 Same query producing a different result (69ms)
      ✔ 698#issuecomment-402604237 mutating a returned array should not affect exec-calls afterwards (100ms)
      ✔ #815 Allow null value for strings (68ms)
      ✔ gitter: query with regex does not return correct results (68ms)
      ✔ #2071 RxCollection.findOne().exec() returns deleted document while find().exec() not (68ms)
      ✔ #2213 prepareQuery should handle all comparison operators (68ms)
      ✔ should not mutate the query input (68ms)
      ✔ gitter: mutating find-params causes different results (68ms)
      ✔ #3498 RxQuery returns outdated result in second subscription (69ms)
      ✔ #3631 Sorting a query adds in deleted documents (87ms)
      ✔ #4552 $elemMatch query not working when there are many documents in the collection (87ms)
      ✔ #4586 query-builder copies other param (87ms)
      ✔ #4773 should not return deleted documents when queried by a primary key (92ms)
      ✔ primaryKey with value "constructor", breaks .findOne() (92ms)
      ✔ findOne().remove() should return null when no document matches instead of crashing (92ms)
      ✔ findOne().remove() with selector matching no document should return null (92ms)
      ✔ findOne().remove(true) should throw when no document matches (91ms)
      ✔ findOne().remove(true) should succeed when a document matches (91ms)

  cross-instance.test.js
    ✔ create a multiInstance database
    ✔ create a 2 multiInstance databases
    ✔ get event on db2 when db1 fires
    ✔ should not get the same events twice (132ms)
    ✔ get event on db2 when db1 fires
    ✔ get event on doc2 when doc1 is changed (75ms)
    ✔ should work with encrypted fields (95ms)
    ✔ should work with nested encrypted fields (99ms)
    ✔ should receive events on the other side
    ✔ should receive 2 events (64ms)

  local-documents.test.ts
    ✔ should create a local document
    ✔ should not find the doc because its local
    ✔ should throw if already exists
    ✔ should find the document
    ✔ should find the document twice (doc-cache)
    ✔ should not find non-existing
    ✔ should return the full RxLocaDocument, not just the data (65ms)
    ✔ should modify the data
    ✔ should modify the data
    ✔ should emit null when not exists
    ✔ should emit the document when exists
    ✔ collection: should emit again when state changed (83ms)
    ✔ database: should emit again when state changed (83ms)
    ✔ should insert when not exists (52ms)
    ✔ should update if the document already exists (54ms)
    ✔ should invoke subscription once (123ms)
    ✔ should upsert after remove and create a non-deleted document (54ms)
    ✔ should remove the document (61ms)
    ✔ should be able to use local documents directly on the database (61ms)
    ✔ should stream events over multi-instance (79ms)
    ✔ should emit deleted
    ✔ should emit changes (database)
    ✔ should emit changes (collection) (150ms)
    ✔ BUG insertLocal not send to other instance (79ms)
    ✔ should not conflict with non-local-doc that has same id (182ms)
    ✔ #661 LocalDocument Observer field error (75ms)
    ✔ #663 Document conflicts with LocalDocument in the same Collection (60ms)
    ✔ local documents not persistent on db restart (71ms)
    ✔ doing many upsertLocal() can cause a 404 document not found

  change-event-buffer.test.js
    ✔ should contains some events
    ✔ should delete older events when buffer get over limit
    ✔ check if correct events get removed
    ✔ return null if pointer is no more in buffer (too low)
    ✔ return the right pointer
    ✔ return the correct pointer
    ✔ should run from correctly
    ✔ should throw if pointer to low
    ✔ should getFrom correctly
    ✔ should run correct on remove (42ms)

  reactive-query.test.js
    ✔ get results of array when .subscribe() and filled array later
    ✔ get the updated docs on Collection.insert() (56ms)
    ✔ get the value twice when subscribing 2 times (128ms)
    ✔ get the base-value when subscribing again later (66ms)
    ✔ subscribing many times should not result in many database-requests
    ✔ changing many documents in one write should not lead to many query result emits (81ms)
    ✔ doing insert after subscribe should end with the correct results (82ms)
    ✔ get no change when nothing happens
    ✔ #7075 query results not correct if changes happen faster than the query updates (57ms)
    ✔ #31 do not fire on doc-change when result-doc not affected memory (156ms)
    ✔ ISSUE: should have the document in DocCache when getting it from observe (39ms)
    ✔ #138 : findOne().$ returns every doc if no id given
    ✔ ISSUE emitted-order not correct when doing many incrementalUpserts (418ms)
    ✔ #749 RxQuery subscription returns null as first result when ran immediately after another subscription or exec() (68ms)

  key-compression.test.js
    ✔ should have saved a compressed document (46ms)
    ✔ storage.schema should contain non-compressed schema (46ms)
    ✔ should properly run the compressed query (46ms)
    ✔ replication state should contain key-compressed document data (65ms)
    ✔ #50 compress string array properly (45ms)
    ✔ error on nested null (45ms)
    ✔ query over compressed index (45ms)
    ✔ #5492 should properly run the .count() with key-compression (45ms)
    ✔ #5603 corrupt keys containing square brackets (45ms)
    ✔ #6267 boolean indexes broken with key-compression (44ms)

  event-reduce.test.js
    ✔ should have the same results on given data
    ✔ should work with the key-compression plugin
    ✔ random data: should have the same results as without event-reduce
    ✔ should show re-inserted documents after insert-delete cycle

  cache-replacement-policy.test.js
    ✔ should still get the correct results on exec
    ✔ should still emit on new results (46ms)
    ✔ should have the correct amount
    ✔ BUG wrong count when used with switch map
    ✔ should not crash
    ✔ should not remove queries that have subscribers
    ✔ should remove the unexecuted ones after unExecutedLifetime
    ✔ should remove the oldest ones
    ✔ should run exactly once (49ms)

  query-builder.test.js
    ✔ should make a basic roundtrip
    ✔ should work with only the selector
    ✔ should have path
    ✔ should work with big equal number
    ✔ eq() should use $eq operator so it can coexist with other operators on the same field
    ✔ equals() should use $eq operator so it can coexist with other operators on the same field
    ✔ operator followed by eq() should preserve both conditions

  idle-queue.test.js
    ✔ should be able to call queue on database
    ✔ inserts should always be faster than idle-call

  reactivity.test.ts
    ✔ RxDocument.$$
    ✔ RxDocument.get$$()
    ✔ RxDocument.deleted$$
    ✔ RxDocument[proxy]$$
    ✔ RxLocalDocument.$$
    ✔ RxLocalDocument.get$$()
    ✔ RxLocalDocument.deleted$$
    ✔ RxQuery.find().$$
    ✔ RxQuery.findOne().$$
    ✔ should get the signal and clean up correctly

  reactive-collection.test.js
    ✔ should get a valid event on insert
    ✔ should fire on bulk insert
    ✔ should fire on bulk remove
    ✔ should fire on remove (67ms)
    ✔ should only emit inserts
    ✔ should only emit updates
    ✔ should only emit removes

  reactive-document.test.js
    ✔ should fire on save
    ✔ should observe a single field
    ✔ should observe a nested field
    ✔ get equal values when subscribing again later
    ✔ cannot observe non-existent field
    ✔ deleted$ is true, on delete
    ✔ should not emit when deleted state has not changed (216ms)
    ✔ should emit a RxDocument, not only the document data
    ✔ primary cannot be observed
    ✔ final fields cannot be observed
    ✔ #4546 - should return the same valueObj for the same objPath
    ✔ get$() on nested object path should not emit when unrelated field changes (140ms)

  cleanup.test.js
init.test.js: unhandledRejection
init.test.js: unhandledRejection
------- COULD NOT AWAIT p
AssertionError [ERR_ASSERTION]: cleanup() must return a boolean, not undefined
+ actual - expected

+ 'undefined'
- 'boolean'

    at ContextProxy.<anonymous> (file:///home/runner/work/rxdb/rxdb/test/unit/cleanup.test.ts:355:20)
    at runNextTicks (node:internal/process/task_queues:65:5)
    at listOnTimeout (node:internal/timers:567:9)
    at processTimers (node:internal/timers:541:7)
AssertionError [ERR_ASSERTION]: cleanup() must return a boolean, not undefined
+ actual - expected

+ 'undefined'
- 'boolean'

    at ContextProxy.<anonymous> (file:///home/runner/work/rxdb/rxdb/test/unit/cleanup.test.ts:355:20)
    at runNextTicks (node:internal/process/task_queues:65:5)
    at listOnTimeout (node:internal/timers:567:9)
    at processTimers (node:internal/timers:541:7)

View full workflow run