address review

Author: samwillis

packages/db/src/collection/events.ts

@@ -35,17 +35,18 @@ export interface CollectionSubscribersChangeEvent {
 /**
  * Event emitted when the collection's loading more state changes
  */
-export interface CollectionLoadingMoreChangeEvent {
-  type: `loadingMore:change`
+export interface CollectionLoadingSubsetChangeEvent {
+  type: `loadingSubset:change`
   collection: Collection<any, any, any, any, any>
-  isLoadingMore: boolean
-  previousIsLoadingMore: boolean
+  isLoadingSubset: boolean
+  previousIsLoadingSubset: boolean
+  loadingSubsetTransition: `start` | `end`
 }
 
 export type AllCollectionEvents = {
   "status:change": CollectionStatusChangeEvent
   "subscribers:change": CollectionSubscribersChangeEvent
-  "loadingMore:change": CollectionLoadingMoreChangeEvent
+  "loadingSubset:change": CollectionLoadingSubsetChangeEvent
 } & {
   [K in CollectionStatus as `status:${K}`]: CollectionStatusEvent<K>
 }
@@ -54,7 +55,7 @@ export type CollectionEvent =
   | AllCollectionEvents[keyof AllCollectionEvents]
   | CollectionStatusChangeEvent
   | CollectionSubscribersChangeEvent
-  | CollectionLoadingMoreChangeEvent
+  | CollectionLoadingSubsetChangeEvent
 
 export type CollectionEventHandler<T extends keyof AllCollectionEvents> = (
   event: AllCollectionEvents[T]

packages/db/src/collection/index.ts

@@ -359,8 +359,8 @@ export class CollectionImpl<
    * Check if the collection is currently loading more data
    * @returns true if the collection has pending load more operations, false otherwise
    */
-  public get isLoadingMore(): boolean {
-    return this._sync.isLoadingMore
+  public get isLoadingSubset(): boolean {
+    return this._sync.isLoadingSubset
   }
 
   /**
@@ -372,7 +372,7 @@ export class CollectionImpl<
   }
 
   /**
-   * Tracks a load promise for isLoadingMore state.
+   * Tracks a load promise for isLoadingSubset state.
    * @internal This is for internal coordination (e.g., live-query glue code), not for general use.
    */
   public trackLoadPromise(promise: Promise<void>): void {

packages/db/src/collection/subscription.ts

@@ -124,6 +124,24 @@ export class CollectionSubscription
     } as SubscriptionEvents[typeof eventKey])
   }
 
+  /**
+   * Track a loadSubset promise and manage loading status
+   */
+  private trackLoadSubsetPromise(syncResult: Promise<void> | true) {
+    // Track the promise if it's actually a promise (async work)
+    if (syncResult instanceof Promise) {
+      this.pendingLoadSubsetPromises.add(syncResult)
+      this.setStatus(`loadingSubset`)
+
+      syncResult.finally(() => {
+        this.pendingLoadSubsetPromises.delete(syncResult)
+        if (this.pendingLoadSubsetPromises.size === 0) {
+          this.setStatus(`ready`)
+        }
+      })
+    }
+  }
+
   hasLoadedInitialState() {
     return this.loadedInitialState
   }
@@ -179,18 +197,7 @@ export class CollectionSubscription
       subscription: this,
     })
 
-    // Track the promise if it's actually a promise (async work)
-    if (syncResult instanceof Promise) {
-      this.pendingLoadSubsetPromises.add(syncResult)
-      this.setStatus(`loadingMore`)
-
-      syncResult.finally(() => {
-        this.pendingLoadSubsetPromises.delete(syncResult)
-        if (this.pendingLoadSubsetPromises.size === 0) {
-          this.setStatus(`ready`)
-        }
-      })
-    }
+    this.trackLoadSubsetPromise(syncResult)
 
     // Also load data immediately from the collection
     const snapshot = this.collection.currentStateAsChanges(stateOpts)
@@ -289,18 +296,7 @@ export class CollectionSubscription
       subscription: this,
     })
 
-    // Track the promise if it's actually a promise (async work)
-    if (syncResult instanceof Promise) {
-      this.pendingLoadSubsetPromises.add(syncResult)
-      this.setStatus(`loadingMore`)
-
-      syncResult.finally(() => {
-        this.pendingLoadSubsetPromises.delete(syncResult)
-        if (this.pendingLoadSubsetPromises.size === 0) {
-          this.setStatus(`ready`)
-        }
-      })
-    }
+    this.trackLoadSubsetPromise(syncResult)
   }
 
   /**

packages/db/src/collection/sync.ts

@@ -259,40 +259,41 @@ export class CollectionSyncManager<
   /**
    * Gets whether the collection is currently loading more data
    */
-  public get isLoadingMore(): boolean {
+  public get isLoadingSubset(): boolean {
     return this.pendingLoadSubsetPromises.size > 0
   }
 
   /**
-   * Tracks a load promise for isLoadingMore state.
+   * Tracks a load promise for isLoadingSubset state.
    * @internal This is for internal coordination (e.g., live-query glue code), not for general use.
    */
   public trackLoadPromise(promise: Promise<void>): void {
-    const wasLoading = this.isLoadingMore
+    const loadingStarting = !this.isLoadingSubset
     this.pendingLoadSubsetPromises.add(promise)
-    const isLoadingNow = this.isLoadingMore
 
-    if (!wasLoading && isLoadingNow) {
-      this._events.emit(`loadingMore:change`, {
-        type: `loadingMore:change`,
+    if (loadingStarting) {
+      this._events.emit(`loadingSubset:change`, {
+        type: `loadingSubset:change`,
         collection: this.collection,
-        isLoadingMore: true,
-        previousIsLoadingMore: false,
+        isLoadingSubset: true,
+        previousIsLoadingSubset: false,
+        loadingSubsetTransition: `start`,
       })
     }
 
     promise.finally(() => {
-      // Check loading state BEFORE removing the promise
-      const wasLoadingBeforeRemoval = this.isLoadingMore
+      const loadingEnding =
+        this.pendingLoadSubsetPromises.size === 1 &&
+        this.pendingLoadSubsetPromises.has(promise)
       this.pendingLoadSubsetPromises.delete(promise)
-      const stillLoading = this.isLoadingMore
 
-      if (wasLoadingBeforeRemoval && !stillLoading) {
-        this._events.emit(`loadingMore:change`, {
-          type: `loadingMore:change`,
+      if (loadingEnding) {
+        this._events.emit(`loadingSubset:change`, {
+          type: `loadingSubset:change`,
           collection: this.collection,
-          isLoadingMore: false,
-          previousIsLoadingMore: true,
+          isLoadingSubset: false,
+          previousIsLoadingSubset: true,
+          loadingSubsetTransition: `end`,
         })
       }
     })

packages/db/src/query/live/collection-subscriber.ts

@@ -76,12 +76,12 @@ export class CollectionSubscriber<
     // Subscribe to subscription status changes to propagate loading state
     const statusUnsubscribe = subscription.on(`status:change`, (event) => {
       // TODO: For now we are setting this loading state whenever the subscription
-      // status changes to 'loadingMore'. But we have discussed it only happening
+      // status changes to 'loadingSubset'. But we have discussed it only happening
       // when the the live query has it's offset/limit changed, and that triggers the
       // subscription to request a snapshot. This will require more work to implement,
       // and builds on https://github.com/TanStack/db/pull/663 which this PR
       // does not yet depend on.
-      if (event.status === `loadingMore`) {
+      if (event.status === `loadingSubset`) {
         // Guard against duplicate transitions
         if (!this.subscriptionLoadingPromises.has(subscription)) {
           let resolve: () => void

packages/db/src/types.ts

@@ -154,7 +154,7 @@ export type OperationType = `insert` | `update` | `delete`
 /**
  * Subscription status values
  */
-export type SubscriptionStatus = `ready` | `loadingMore`
+export type SubscriptionStatus = `ready` | `loadingSubset`
 
 /**
  * Event emitted when subscription status changes
@@ -190,7 +190,7 @@ export interface SubscriptionUnsubscribedEvent {
 export type SubscriptionEvents = {
   "status:change": SubscriptionStatusChangeEvent
   "status:ready": SubscriptionStatusEvent<`ready`>
-  "status:loadingMore": SubscriptionStatusEvent<`loadingMore`>
+  "status:loadingSubset": SubscriptionStatusEvent<`loadingSubset`>
   unsubscribed: SubscriptionUnsubscribedEvent
 }
 
@@ -380,14 +380,6 @@ export type CollectionStatus =
   /** Collection has been cleaned up and resources freed */
   | `cleaned-up`
 
-/**
- * @default `eager`
- * @description
- * Collections have two modes of sync: eager and on-demand.
- * - eager: syncs all data immediately on preload
- * - on-demand: syncs data in incremental snapshots when the collection is queried
- * The exact implementation of the sync mode is up to the sync implementation.
- */
 export type SyncMode = `eager` | `on-demand`
 
 export interface BaseCollectionConfig<

packages/db/tests/collection-subscription.test.ts

@@ -20,7 +20,7 @@ describe(`CollectionSubscription status tracking`, () => {
     subscription.unsubscribe()
   })
 
-  it(`status changes to 'loadingMore' when requestSnapshot triggers a promise`, async () => {
+  it(`status changes to 'loadingSubset' when requestSnapshot triggers a promise`, async () => {
     let resolveLoadSubset: () => void
     const loadSubsetPromise = new Promise<void>((resolve) => {
       resolveLoadSubset = resolve
@@ -49,8 +49,8 @@ describe(`CollectionSubscription status tracking`, () => {
     // Trigger a snapshot request that will call loadSubset
     subscription.requestSnapshot({ optimizedOnly: false })
 
-    // Status should now be loadingMore
-    expect(subscription.status).toBe(`loadingMore`)
+    // Status should now be loadingSubset
+    expect(subscription.status).toBe(`loadingSubset`)
 
     // Resolve the load more promise
     resolveLoadSubset!()
@@ -87,7 +87,7 @@ describe(`CollectionSubscription status tracking`, () => {
     })
 
     subscription.requestSnapshot({ optimizedOnly: false })
-    expect(subscription.status).toBe(`loadingMore`)
+    expect(subscription.status).toBe(`loadingSubset`)
 
     resolveLoadSubset!()
     await flushPromises()
@@ -96,7 +96,7 @@ describe(`CollectionSubscription status tracking`, () => {
     subscription.unsubscribe()
   })
 
-  it(`concurrent promises keep status as 'loadingMore' until all resolve`, async () => {
+  it(`concurrent promises keep status as 'loadingSubset' until all resolve`, async () => {
     let resolveLoadSubset1: () => void
     let resolveLoadSubset2: () => void
     let callCount = 0
@@ -132,18 +132,18 @@ describe(`CollectionSubscription status tracking`, () => {
 
     // Trigger first load
     subscription.requestSnapshot({ optimizedOnly: false })
-    expect(subscription.status).toBe(`loadingMore`)
+    expect(subscription.status).toBe(`loadingSubset`)
 
     // Trigger second load
     subscription.requestSnapshot({ optimizedOnly: false })
-    expect(subscription.status).toBe(`loadingMore`)
+    expect(subscription.status).toBe(`loadingSubset`)
 
     // Resolve first promise
     resolveLoadSubset1!()
     await flushPromises()
 
     // Should still be loading because second promise is pending
-    expect(subscription.status).toBe(`loadingMore`)
+    expect(subscription.status).toBe(`loadingSubset`)
 
     // Resolve second promise
     resolveLoadSubset2!()
@@ -193,15 +193,15 @@ describe(`CollectionSubscription status tracking`, () => {
     expect(statusChanges).toHaveLength(1)
     expect(statusChanges[0]).toEqual({
       previous: `ready`,
-      current: `loadingMore`,
+      current: `loadingSubset`,
     })
 
     resolveLoadSubset!()
     await flushPromises()
 
     expect(statusChanges).toHaveLength(2)
     expect(statusChanges[1]).toEqual({
-      previous: `loadingMore`,
+      previous: `loadingSubset`,
       current: `ready`,
     })
 
@@ -235,7 +235,7 @@ describe(`CollectionSubscription status tracking`, () => {
     })
 
     subscription.requestSnapshot({ optimizedOnly: false })
-    expect(subscription.status).toBe(`loadingMore`)
+    expect(subscription.status).toBe(`loadingSubset`)
 
     // Reject the promise
     rejectLoadSubset!(new Error(`Load failed`))