fix(query): keep the previous result when .suspend() changes key with placeholderData

core/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@comwit/state",
-  "version": "2.5.1",
+  "version": "2.5.2",
   "private": false,
   "homepage": "https://library.comwit.io/state",
   "repository": {

core/src/core/query/accessor.ts

@@ -7,6 +7,7 @@ import {
   RESOURCE_HYDRATE,
   RESOURCE_QUERY_OPTION_KEYS,
   RESOURCE_SUSPEND_COMMIT,
+  RESOURCE_SUSPEND_PLACEHOLDER,
   RESOURCE_SUSPEND_PREPARE,
   RESOURCE_SUSPEND_RESTORE,
   RESOURCE_SUSPEND_STREAM,
@@ -839,6 +840,34 @@ export function createResourceAccessor(
     runInternal(() => applyCachedState(state, entry))
   }
 
+  /**
+   * With `placeholderData`, a `.suspend()` key that has no result yet does not suspend while the
+   * resource already shows another key's result: the selector keeps that data visible and the key
+   * loads after commit like `.load()`. Returns the placeholder option in that case.
+   */
+  const readSuspendPlaceholder = (key: QueryCacheKey) => {
+    if (runtime.activeKey === undefined || runtime.activeKey === key) return
+    if (!getActiveEntry()?.hasQueried || runtime.cacheEntries.get(key)?.hasQueried) return
+    const { placeholderData } = normalizeDescriptorOptions<unknown, unknown>(
+      descriptor,
+      defaults,
+      undefined
+    )
+    return placeholderData === undefined ? undefined : { placeholderData }
+  }
+
+  /** True while a key loads behind the previous result instead of suspending. */
+  const keepsPreviousResult = (key: QueryCacheKey) => {
+    if (readSuspendPlaceholder(key)) return true
+    // After commit the key is active and fetching, with the placeholder still visible.
+    return (
+      runtime.activeKey === key &&
+      !runtime.cacheEntries.get(key)?.hasQueried &&
+      state.isFetching === true &&
+      state.isLoading === false
+    )
+  }
+
   /**
    * Prepare an initial query for selector `.suspend()` without touching the
    * observable resource proxy. Cache entries live outside the proxy, so a
@@ -863,6 +892,7 @@ export function createResourceAccessor(
     if (entry.hasQueried) return
     if (entry.suspendError) throw entry.suspendError
     if (entry.suspendPromise) return entry.suspendPromise
+    if (keepsPreviousResult(key)) return
 
     // Reuse an effect/direct query that was already started for this exact
     // controller and key. It began after a previous commit, so it is safe to
@@ -1064,7 +1094,12 @@ export function createResourceAccessor(
   ): Promise<unknown> => {
     const queryArg = hasArg ? arg : undefined
     const entry = runtime.cacheEntries.get(key)
-    if (!entry?.hasQueried) return Promise.resolve()
+    if (!entry?.hasQueried) {
+      // A key kept behind placeholder data loads now, keeping the previous result visible.
+      return readSuspendPlaceholder(key)
+        ? executeQuery(queryArg, undefined, hasArg, 'replace', false)
+        : Promise.resolve()
+    }
 
     if (!entry.suspendNeedsCommit) {
       // A previously committed cache hit remains visible while its normal
@@ -1359,6 +1394,7 @@ export function createResourceAccessor(
       if (prop === RESOURCE_SUSPEND_COMMIT) return commitSuspend
       if (prop === RESOURCE_SUSPEND_RESTORE) return restoreSuspend
       if (prop === RESOURCE_SUSPEND_STREAM) return readSuspendStream
+      if (prop === RESOURCE_SUSPEND_PLACEHOLDER) return readSuspendPlaceholder
       if (prop === 'query') return queryFn
       if (descriptor.selectorMethod && prop === descriptor.selectorMethod) return queryFn
       if (prop === 'refetch') return refetch

core/src/core/query/select.ts

@@ -1,6 +1,7 @@
 import { serializeResourceArg } from './accessor'
 import type {
   AnyResourceDescriptor,
+  PlaceholderData,
   QueryBindingRegistry,
   QueryCacheKey,
   ResourceDataLike,
@@ -9,6 +10,7 @@ import type {
 } from './types'
 import {
   RESOURCE_SUSPEND_COMMIT,
+  RESOURCE_SUSPEND_PLACEHOLDER,
   RESOURCE_SUSPEND_PREPARE,
   RESOURCE_SUSPEND_RESTORE,
   RESOURCE_SUSPEND_STREAM,
@@ -62,13 +64,45 @@ function pendingState(descriptor: AnyResourceDescriptor, isFetching: boolean) {
   }) as ResourceDataLike
 }
 
+/** Previous data shown while a `.suspend()` key with `placeholderData` loads after commit. */
+function placeholderState(
+  state: ResourceDataLike,
+  controller: ResourceDataLike,
+  key: QueryCacheKey,
+  arg: unknown
+): ResourceDataLike | undefined {
+  const read = Reflect.get(controller, RESOURCE_SUSPEND_PLACEHOLDER)
+  if (typeof read !== 'function') return
+  const placeholder = read.call(controller, key) as
+    | { placeholderData: PlaceholderData<unknown, unknown> }
+    | undefined
+  if (!placeholder) return
+  const { placeholderData } = placeholder
+  return Object.freeze({
+    ...state,
+    data:
+      typeof placeholderData === 'function'
+        ? (placeholderData as (queryArg: unknown, previousData: unknown) => unknown)(
+            arg,
+            state.data
+          )
+        : placeholderData,
+    isLoading: false,
+    isFetching: true,
+    isError: false,
+    error: null,
+  }) as ResourceDataLike
+}
+
 function cachedState(
   state: ResourceDataLike,
   runtime: ResourceRuntimeState | undefined,
   controller: ResourceDataLike,
   descriptor: AnyResourceDescriptor,
   key: QueryCacheKey,
-  registry: QueryBindingRegistry
+  registry: QueryBindingRegistry,
+  arg: unknown,
+  mode: 'load' | 'suspend'
 ): ResourceDataLike {
   const activeEntry = runtime?.activeKey === key ? runtime.cacheEntries.get(key) : undefined
   // A first `.suspend()` result is already the proxy value, but until the selecting tree commits
@@ -93,6 +127,11 @@ function cachedState(
     return entryState
   }
 
+  if (mode === 'suspend') {
+    const placeholder = placeholderState(state, controller, key, arg)
+    if (placeholder) return placeholder
+  }
+
   return pendingState(descriptor, pending)
 }
 
@@ -141,7 +180,7 @@ function createResourceSelector(
         }
 
         const runtime = registry.boundResourceRuntime.get(controller)
-        return cachedState(state, runtime, controller, descriptor, key, registry)
+        return cachedState(state, runtime, controller, descriptor, key, registry, arg, load.mode)
       }
     },
   })

core/src/core/query/types.ts

@@ -7,6 +7,7 @@ export const RESOURCE_SUSPEND_PREPARE = Symbol('comwit-resource-suspend-prepare'
 export const RESOURCE_SUSPEND_COMMIT = Symbol('comwit-resource-suspend-commit')
 export const RESOURCE_SUSPEND_RESTORE = Symbol('comwit-resource-suspend-restore')
 export const RESOURCE_SUSPEND_STREAM = Symbol('comwit-resource-suspend-stream')
+export const RESOURCE_SUSPEND_PLACEHOLDER = Symbol('comwit-resource-suspend-placeholder')
 export const RESOURCE_HYDRATE = Symbol('comwit-resource-hydrate')
 
 export type AsyncResult<T> = T | Promise<T> | AsyncIterable<T>

core/tests/query-suspend.test.tsx

@@ -2,7 +2,16 @@
 import React, { Component, Suspense, startTransition, type ReactNode } from 'react'
 import { act, fireEvent, render, screen, waitFor } from '@testing-library/react'
 import { describe, expect, test, vi } from 'vitest'
-import { action, ComwitProvider, local, model, query, useAction, useModel } from '../src'
+import {
+  action,
+  ComwitProvider,
+  keepPreviousData,
+  local,
+  model,
+  query,
+  useAction,
+  useModel,
+} from '../src'
 
 /** A render that suspends through React 19 `use()` must run inside an awaited act. */
 async function renderAsync(ui: React.ReactElement) {
@@ -427,4 +436,93 @@ describe('query selector suspend', () => {
     })
     await waitFor(() => expect(screen.getByTestId('value').textContent).toBe('loaded'))
   })
+
+  describe('placeholderData on an argument change', () => {
+    function setup(placeholderData?: typeof keepPreviousData) {
+      const requests = new Map<number, ReturnType<typeof deferred<string[]>>>()
+      const request = (page: number) => {
+        if (!requests.has(page)) requests.set(page, deferred<string[]>())
+        return requests.get(page)!
+      }
+      const queryFn = vi.fn(({ page }: { page: number }) => request(page).promise)
+      const catalog = model({
+        page: 1,
+        items: query<string[], { page: number }>({ initialData: [], queryFn, placeholderData }),
+      })
+      const catalogActions = action(({ state }) => ({
+        setPage(page: number) {
+          state(catalog).page = page
+        },
+      }))
+
+      function View() {
+        const items = useModel(catalog, (state) => state.items.suspend({ page: state.page }))
+        const actions = useAction<{ setPage(page: number): void }>([catalogActions])
+        return (
+          <>
+            <div data-testid="items">{items.data.join(',')}</div>
+            <div data-testid="fetching">{String(items.isFetching)}</div>
+            <button onClick={() => actions.setPage(2)}>page 2</button>
+          </>
+        )
+      }
+
+      return { request, queryFn, View }
+    }
+
+    async function renderFirstPage(
+      View: () => React.ReactElement,
+      request: (page: number) => ReturnType<typeof deferred<string[]>>
+    ) {
+      await renderAsync(
+        <ComwitProvider>
+          <Suspense fallback={<div data-testid="fallback">loading</div>}>
+            <View />
+          </Suspense>
+        </ComwitProvider>
+      )
+      await act(async () => {
+        request(1).resolve(['a'])
+        await request(1).promise
+      })
+      await waitFor(() => expect(screen.getByTestId('items').textContent).toBe('a'))
+    }
+
+    test('keepPreviousData keeps the previous result visible instead of suspending', async () => {
+      const { request, queryFn, View } = setup(keepPreviousData)
+      await renderFirstPage(View, request)
+
+      await act(async () => {
+        fireEvent.click(screen.getByText('page 2'))
+      })
+      expect(screen.queryByTestId('fallback')).toBeNull()
+      expect(screen.getByTestId('items').textContent).toBe('a')
+      expect(screen.getByTestId('fetching').textContent).toBe('true')
+      expect(queryFn).toHaveBeenCalledTimes(2)
+
+      await act(async () => {
+        request(2).resolve(['b'])
+        await request(2).promise
+      })
+      await waitFor(() => expect(screen.getByTestId('items').textContent).toBe('b'))
+      expect(screen.getByTestId('fetching').textContent).toBe('false')
+      expect(queryFn).toHaveBeenCalledTimes(2)
+    })
+
+    test('without placeholderData a new argument still suspends', async () => {
+      const { request, View } = setup()
+      await renderFirstPage(View, request)
+
+      await act(async () => {
+        fireEvent.click(screen.getByText('page 2'))
+      })
+      expect(screen.getByTestId('fallback')).toBeDefined()
+
+      await act(async () => {
+        request(2).resolve(['b'])
+        await request(2).promise
+      })
+      await waitFor(() => expect(screen.getByTestId('items').textContent).toBe('b'))
+    })
+  })
 })