Reimlement gcManager

Author: MrFlashAccount

packages/query-core/src/__tests__/gcManager.test.tsx

@@ -40,6 +40,10 @@ describe('gcManager', () => {
 
     // Query exists and GC should not be running yet (query is active)
     expect(queryClient.getQueryCache().find({ queryKey: key })).toBeDefined()
+    console.log(
+      'gcManager.getEligibleItemCount()',
+      gcManager.getEligibleItemCount(),
+    )
     expect(gcManager.getEligibleItemCount()).toBe(0)
 
     // Unsubscribe - this should mark the query for GC

packages/query-core/src/__tests__/mutationCache.test.tsx

@@ -342,7 +342,6 @@ describe('mutationCache', () => {
         },
         1,
       )
-
       await vi.advanceTimersByTimeAsync(10)
 
       expect(testCache.getAll()).toHaveLength(1)
@@ -414,13 +413,10 @@ describe('mutationCache', () => {
       const unsubscribe = observer.subscribe(() => undefined)
 
       observer.mutate(1)
-      await vi.advanceTimersByTimeAsync(10)
-      expect(queryClient.getMutationCache().getAll()).toHaveLength(1)
 
       unsubscribe()
 
-      await vi.advanceTimersByTimeAsync(10)
-
+      await vi.advanceTimersByTimeAsync(11)
       expect(queryClient.getMutationCache().getAll()).toHaveLength(0)
       expect(onSuccess).toHaveBeenCalledTimes(1)
     })

packages/query-core/src/__tests__/mutationObserver.test.tsx

@@ -58,8 +58,7 @@ describe('mutationObserver', () => {
 
     unsubscribe()
 
-    await vi.advanceTimersByTimeAsync(20)
-
+    await vi.advanceTimersByTimeAsync(10)
     expect(queryClient.getMutationCache().findAll()).toHaveLength(0)
   })
 
@@ -80,7 +79,7 @@ describe('mutationObserver', () => {
 
     mutation.reset()
 
-    await vi.advanceTimersByTimeAsync(20)
+    await vi.advanceTimersByTimeAsync(10)
     expect(queryClient.getMutationCache().findAll()).toHaveLength(0)
 
     unsubscribe()

packages/query-core/src/__tests__/mutations.test.tsx

@@ -781,6 +781,7 @@ describe('mutations', () => {
 
       // Verify mutation returns its own result, not callback returns
       expect(mutationResult).toBe('actual-result')
+      console.log(results)
       expect(results).toEqual([
         'sync-onMutate',
         'async-onSuccess',

packages/query-core/src/__tests__/query.test.tsx

@@ -557,7 +557,9 @@ describe('query', () => {
     const unsubscribe1 = observer.subscribe(() => undefined)
     unsubscribe1()
 
-    await vi.advanceTimersByTimeAsync(1)
+    await vi.advanceTimersByTimeAsync(0)
+    await vi.advanceTimersByTimeAsync(0)
+
     expect(queryCache.find({ queryKey: key })).toBeUndefined()
     const unsubscribe2 = observer.subscribe(() => undefined)
     unsubscribe2()
@@ -579,7 +581,7 @@ describe('query', () => {
     expect(queryCache.find({ queryKey: key })).toBeDefined()
     unsubscribe()
 
-    await vi.advanceTimersByTimeAsync(1)
+    await vi.advanceTimersByTimeAsync(0)
     expect(queryCache.find({ queryKey: key })).toBeUndefined()
   })
 

packages/query-core/src/__tests__/queryClient.test.tsx

@@ -700,7 +700,7 @@ describe('queryClient', () => {
       })
       await vi.advanceTimersByTimeAsync(10)
       await expect(promise).resolves.toEqual(1)
-      await vi.advanceTimersByTimeAsync(10)
+      await vi.advanceTimersByTimeAsync(1)
       expect(queryClient.getQueryData(key1)).toEqual(undefined)
     })
 

packages/query-core/src/gcManager.ts

@@ -40,43 +40,30 @@ export class GCManager {
   #forceDisable = false
   #eligibleItems = new Set<Removable>()
   #scheduledScanTimeoutId: ManagedTimerId | null = null
-  #scheduledScanTimeout: number | null = null
-  #scheduledScanIdleCallbackId: ManagedTimerId | null = null
+  #isScheduledScan = false
 
   constructor(config: GCManagerConfig = {}) {
     this.#forceDisable = config.forceDisable ?? false
   }
 
   #scheduleScan(): void {
-    if (this.#forceDisable) {
+    if (this.#forceDisable || this.#isScheduledScan) {
       return
     }
 
-    if (this.#scheduledScanIdleCallbackId !== null) {
-      return
-    }
-
-    if (this.#isScanning && this.#scheduledScanTimeoutId !== null) {
-      timeoutManager.clearTimeout(this.#scheduledScanTimeoutId)
-      this.#isScanning = false
-      this.#scheduledScanTimeoutId = null
-      this.#scheduledScanTimeout = null
-      this.#scheduledScanIdleCallbackId = null
-    }
+    this.#isScheduledScan = true
 
-    this.#scheduledScanIdleCallbackId = timeoutManager.setTimeout(() => {
-      this.#scheduledScanIdleCallbackId = null
+    queueMicrotask(() => {
+      if (!this.#isScheduledScan) {
+        return
+      }
 
-      const now = Date.now()
+      this.#isScheduledScan = false
 
       let minTimeUntilGc = Infinity
 
       for (const item of this.#eligibleItems) {
-        const gcAt = item.getGcAtTimestamp()
-        if (gcAt === null) {
-          continue
-        }
-        const timeUntilGc = Math.max(0, gcAt - now)
+        const timeUntilGc = getTimeUntilGc(item)
 
         if (timeUntilGc < minTimeUntilGc) {
           minTimeUntilGc = timeUntilGc
@@ -87,20 +74,14 @@ export class GCManager {
         return
       }
 
-      if (this.#scheduledScanTimeout === minTimeUntilGc) {
-        return
-      }
-
       if (this.#scheduledScanTimeoutId !== null) {
         timeoutManager.clearTimeout(this.#scheduledScanTimeoutId)
-        this.#scheduledScanTimeoutId = null
       }
 
+      this.#isScanning = true
       this.#scheduledScanTimeoutId = timeoutManager.setTimeout(() => {
         this.#isScanning = false
         this.#scheduledScanTimeoutId = null
-        this.#scheduledScanTimeout = null
-        this.#scheduledScanIdleCallbackId = null
 
         this.#performScan()
 
@@ -109,26 +90,23 @@ export class GCManager {
           this.#scheduleScan()
         }
       }, minTimeUntilGc)
-
-      this.#isScanning = true
-      this.#scheduledScanTimeout = minTimeUntilGc
-    }, 0)
+    })
   }
 
   /**
    * Stop periodic scanning. Safe to call multiple times.
    */
   stopScanning(): void {
+    this.#isScanning = false
+    this.#isScheduledScan = false
+
     if (this.#scheduledScanTimeoutId === null) {
       return
     }
 
     timeoutManager.clearTimeout(this.#scheduledScanTimeoutId)
 
-    this.#isScanning = false
     this.#scheduledScanTimeoutId = null
-    this.#scheduledScanTimeout = null
-    this.#scheduledScanIdleCallbackId = null
   }
 
   /**
@@ -145,12 +123,17 @@ export class GCManager {
    * @param item - The query or mutation marked for GC
    */
   trackEligibleItem(item: Removable): void {
-    this.#eligibleItems.add(item)
+    if (this.#forceDisable) {
+      return
+    }
 
-    // Start scanning if we have eligible items and aren't already scanning
-    if (!this.#isScanning) {
-      this.#scheduleScan()
+    if (this.#eligibleItems.has(item)) {
+      return
     }
+
+    this.#eligibleItems.add(item)
+
+    this.#scheduleScan()
   }
 
   /**
@@ -160,11 +143,22 @@ export class GCManager {
    * @param item - The query or mutation no longer eligible for GC
    */
   untrackEligibleItem(item: Removable): void {
+    if (this.#forceDisable) {
+      return
+    }
+
+    if (!this.#eligibleItems.has(item)) {
+      return
+    }
+
     this.#eligibleItems.delete(item)
 
-    // Stop scanning if no items are eligible
-    if (this.getEligibleItemCount() === 0 && this.#isScanning) {
-      this.stopScanning()
+    if (this.isScanning()) {
+      if (this.getEligibleItemCount() === 0) {
+        this.stopScanning()
+      } else {
+        this.#scheduleScan()
+      }
     }
   }
 
@@ -183,7 +177,7 @@ export class GCManager {
           const wasCollected = item.optionalRemove()
 
           if (wasCollected) {
-            this.untrackEligibleItem(item)
+            this.#eligibleItems.delete(item)
           }
         }
       } catch (error) {
@@ -194,4 +188,17 @@ export class GCManager {
       }
     }
   }
+
+  clear(): void {
+    this.#eligibleItems.clear()
+    this.stopScanning()
+  }
+}
+
+function getTimeUntilGc(item: Removable): number {
+  const gcAt = item.getGcAtTimestamp()
+  if (gcAt === null) {
+    return Infinity
+  }
+  return Math.max(0, gcAt - Date.now())
 }

packages/query-core/src/mutation.ts

@@ -98,20 +98,17 @@ export class Mutation<
   >
   #mutationCache: MutationCache
   #retryer?: Retryer<TData>
-  #gcManager: GCManager
 
   constructor(
     config: MutationConfig<TData, TError, TVariables, TOnMutateResult>,
   ) {
     super()
 
     this.#client = config.client
-    this.#gcManager = config.client.getGcManager()
     this.mutationId = config.mutationId
     this.#mutationCache = config.mutationCache
     this.#observers = []
     this.state = config.state || getDefaultState()
-
     this.setOptions(config.options)
     this.markForGc()
   }
@@ -129,7 +126,7 @@ export class Mutation<
   }
 
   protected getGcManager(): GCManager {
-    return this.#gcManager
+    return this.#client.getGcManager()
   }
 
   addObserver(observer: MutationObserver<any, any, any, any>): void {
@@ -161,13 +158,21 @@ export class Mutation<
     })
   }
 
+  private isSafeToRemove(): boolean {
+    return this.state.status !== 'pending' && this.#observers.length === 0
+  }
+
   optionalRemove(): boolean {
-    if (!this.isSafeToRemove()) {
-      return false
+    if (!this.#observers.length) {
+      if (this.state.status === 'pending') {
+        this.markForGc()
+      } else {
+        this.#mutationCache.remove(this)
+        return true
+      }
     }
 
-    this.#mutationCache.remove(this)
-    return true
+    return false
   }
 
   continue(): Promise<unknown> {
@@ -378,7 +383,6 @@ export class Mutation<
     }
     this.state = reducer(this.state)
 
-    // Check for immediate removal after state change
     if (this.isSafeToRemove()) {
       this.markForGc()
     }
@@ -394,10 +398,6 @@ export class Mutation<
       })
     })
   }
-
-  isSafeToRemove(): boolean {
-    return this.state.status !== 'pending' && this.#observers.length === 0
-  }
 }
 
 export function getDefaultState<

packages/query-core/src/mutationCache.ts

@@ -237,14 +237,6 @@ export class MutationCache extends Subscribable<MutationCacheListener> {
       ),
     )
   }
-
-  /**
-   * Destroy the cache and clean up resources
-   */
-  destroy(): void {
-    // Clean up all mutations
-    this.clear()
-  }
 }
 
 function scopeFor(mutation: Mutation<any, any, any, any>) {