Repository: maxnowack/signaldb (ts)

Issue:
[Bug] Debounce skips pushes
Heya,

Thanks for this library, been really enjoying diving into it.

However, I've noticed that when I do multiple actions on different collections at once (e.g. insert multiple rows in different tables straight after each other), some of those actions don't get synced.

I think I've narrowed it down to how the pushes are debounced in the sync manager:

from https://github.com/maxnowack/signaldb/blob/5760bb15f4eebf3a5fb772d28f3304832d685620/packages/base/sync/src/SyncManager.ts#L314:

```typescript
protected deboucedPush = debounce((name: string) => {
    this.pushChanges(name).catch(() => { /* error handler is called in sync */ })
  }, 100)

  protected schedulePush(name: string) {
    this.deboucedPush(name)
  }
```

It looks like `debounce` will skip the first few few inserts, and only perform the last one, as the same `debouncedPush` function  is used for all collections in the sync manager.

For now i could make a separate sync manager per collection as a workaround, but this should be fairly easy to fix.

Pull request description:
fix(sync): per-collection sync debounce
fixes #1470 

also added a debounceTime parameter to the SyncManager constructor, so the debounce can be configured.

Patch:
diff --git a/package-lock.json b/package-lock.json
index 53f04b1..39b3093 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -9453,9 +9453,9 @@
       "license": "MIT"
     },
     "node_modules/happy-dom": {
-      "version": "17.4.2",
-      "resolved": "https://registry.npmjs.org/happy-dom/-/happy-dom-17.4.2.tgz",
-      "integrity": "sha512-3SA9ZdmDXTvJ5OrHfc7TdqfrKEOM15Gc3dMAvKivlaewUsOJQrNnPTQnAb3n/NR0fRVDniFxk9i82D+HhHWZcA==",
+      "version": "17.4.3",
+      "resolved": "https://registry.npmjs.org/happy-dom/-/happy-dom-17.4.3.tgz",
+      "integrity": "sha512-8mDGIKxi2hAg0DkEYjBHPi5QykWiqdNNQQWrwLXLfro1eAZk8+lSnzbUrnU25bamG9PjEQGoFrA32ezQNJQdww==",
       "dev": true,
       "license": "MIT",
       "dependencies": {
@@ -15598,7 +15598,7 @@
         "@maverick-js/signals": "^6.0.0",
         "@testing-library/react": "^16.1.0",
         "@types/react": "^19.0.6",
-        "happy-dom": "^17.4.2"
+        "happy-dom": "^17.4.3"
       },
       "peerDependencies": {
         "@signaldb/core": "1.x",
diff --git a/package.json b/package.json
index 4b4853e..e646d15 100644
--- a/package.json
+++ b/package.json
@@ -45,7 +45,7 @@
     "eslint-plugin-tsdoc": "^0.4.0",
     "eslint-plugin-unicorn": "^57.0.0",
     "fast-glob": "^3.3.3",
-    "happy-dom": "17.4.2",
+    "happy-dom": "17.4.3",
     "husky": "9.1.7",
     "mermaid": "^11.4.1",
     "rimraf": "6.0.1",
diff --git a/packages/base/sync/src/SyncManager.ts b/packages/base/sync/src/SyncManager.ts
index 4c46caa..1e205a4 100644
--- a/packages/base/sync/src/SyncManager.ts
+++ b/packages/base/sync/src/SyncManager.ts
@@ -50,6 +50,7 @@ interface Options<
   onError?: (collectionOptions: SyncOptions<CollectionOptions>, error: Error) => void,
 
   autostart?: boolean,
+  debounceTime?: number,
 }
 
 /**
@@ -95,12 +96,14 @@ export default class SyncManager<
   protected changes: Collection<Change<ItemType>, string>
   protected snapshots: Collection<Snapshot<ItemType>, string>
   protected syncOperations: Collection<SyncOperation, string>
+  protected scheduledPushes: Set<string> = new Set()
   protected remoteChanges: Omit<Change, 'id' | 'time'>[] = []
   protected syncQueues: Map<string, PromiseQueue> = new Map()
   protected persistenceReady: Promise<void>
   protected isDisposed = false
   protected instanceId = randomId()
   protected id: string
+  protected debouncedFlush: () => void
 
   /**
    * @param options Collection options
@@ -111,6 +114,8 @@ export default class SyncManager<
    * @param [options.persistenceAdapter] Persistence adapter to use for storing changes, snapshots and sync operations.
    * @param [options.reactivity] Reactivity adapter to use for reactivity.
    * @param [options.onError] Function to handle errors that occur async during syncing.
+   * @param [options.autostart] Whether to automatically start syncing new collections.
+   * @param [options.debounceTime] The time in milliseconds to debounce push operations.
    */
   constructor(options: Options<CollectionOptions, ItemType, IdType>) {
     this.options = {
@@ -155,6 +160,8 @@ export default class SyncManager<
     this.changes.setMaxListeners(1000)
     this.snapshots.setMaxListeners(1000)
     this.syncOperations.setMaxListeners(1000)
+
+    this.debouncedFlush = debounce(this.flushScheduledPushes, this.options.debounceTime ?? 100)
   }
 
   protected createPersistenceAdapter(name: string) {
@@ -311,12 +318,16 @@ export default class SyncManager<
     }
   }
 
-  protected deboucedPush = debounce((name: string) => {
-    this.pushChanges(name).catch(() => { /* error handler is called in sync */ })
-  }, 100)
+  protected flushScheduledPushes() {
+    this.scheduledPushes.forEach((name) => {
+      this.pushChanges(name).catch(() => { /* error handler is called in sync */ })
+    })
+    this.scheduledPushes.clear()
+  }
 
   protected schedulePush(name: string) {
-    this.deboucedPush(name)
+    this.scheduledPushes.add(name)
+    this.debouncedFlush()
   }
 
   /**


Test patch:
diff --git a/packages/base/sync/__tests__/SyncManager.spec.ts b/packages/base/sync/__tests__/SyncManager.spec.ts
index 9e7752b..0aab91b 100644
--- a/packages/base/sync/__tests__/SyncManager.spec.ts
+++ b/packages/base/sync/__tests__/SyncManager.spec.ts
@@ -300,6 +300,7 @@ it('should debounce push requests', async () => {
     persistenceAdapter: () => memoryPersistenceAdapter([]),
     pull: mockPull,
     push: mockPush,
+    debounceTime: 25,
   })
 
   const mockCollection = new Collection<TestItem, string, any>()
@@ -310,13 +311,52 @@ it('should debounce push requests', async () => {
   mockCollection.insert({ id: '3', name: 'Second Item' })
 
   await new Promise((resolve) => {
-    setTimeout(resolve, 110)
+    setTimeout(resolve, 50)
   })
 
   expect(onError).not.toHaveBeenCalled()
   expect(mockPush).toHaveBeenCalledTimes(1)
 })
 
+it('should debounce push requests for multiple collections', async () => {
+  const mockPull = vi.fn<() => Promise<LoadResponse<TestItem>>>().mockResolvedValue({
+    items: [],
+  })
+
+  const mockPush = vi.fn<(options: any, pushParameters: any) => Promise<void>>()
+    .mockResolvedValue()
+  const onError = vi.fn()
+
+  const syncManager = new SyncManager({
+    onError,
+    persistenceAdapter: () => memoryPersistenceAdapter([]),
+    pull: mockPull,
+    push: mockPush,
+    debounceTime: 25,
+  })
+
+  const collection1 = new Collection<TestItem, string, any>()
+  const collection2 = new Collection<TestItem, string, any>()
+
+  syncManager.addCollection(collection1, { name: 'test1' })
+  syncManager.addCollection(collection2, { name: 'test2' })
+
+  collection1.insert({ id: '1', name: 'Collection 1 Item' })
+  collection2.insert({ id: '2', name: 'Collection 2 Item' })
+
+  // Wait for debounce period to complete
+  await new Promise((resolve) => {
+    setTimeout(resolve, 50)
+  })
+
+  expect(onError).not.toHaveBeenCalled()
+  expect(mockPush).toHaveBeenCalledTimes(2)
+
+  const calls = mockPush.mock.calls
+  expect(calls.some(call => call[0].name === 'test1')).toBe(true)
+  expect(calls.some(call => call[0].name === 'test2')).toBe(true)
+})
+
 it('should handle sync errors and update sync operation status', async () => {
   const mockPull = vi.fn<() => Promise<LoadResponse<TestItem>>>().mockRejectedValue(new Error('Sync failed'))