From 5cd02243bb24f9b39929d9a4e91312590a2f71a4 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 11:25:57 +0000 Subject: [PATCH 1/4] fix(plugin-form,fields): a superseded load or save commits nothing in the line-items panel; `useRecordQuery` resets when it can no longer query (objectui#10712) `LineItemsPanel`: a load a newer one has superseded commits neither its rows nor the end of the loading state (the run number `loadSeqRef` now guards the rows commit and the `finally`). The save reloads through the panel's latest `load` only while the parent it saved is still on screen, and writes its failure only then; a save whose parent has left the screen re-reads nothing and reports nothing. `useRecordQuery`: the reset is keyed on `canQuery` (`enabled`, the data source and the object together) and is the same `reset()` a consumer calls. It moves the run number, so a read in flight commits nothing, ends `loading`, drops a pending debounced search and clears the query state. A pending debounced search is also dropped whenever `runQuery` changes, since the fetch effect re-runs the current query with the typed term then. Pins: `LineItemsPanel.supersededRun-10712.test.tsx` and `useRecordQuery.cannotQuery-10712.test.tsx`; one `patch` changeset per package. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_014mXUNuFomfj24w7s1pZzhN --- ...-fields-record-query-cannot-query-reset.md | 25 ++ ...2-plugin-form-line-items-superseded-run.md | 25 ++ .../useRecordQuery.cannotQuery-10712.test.tsx | 288 +++++++++++++ packages/fields/src/widgets/useRecordQuery.ts | 76 ++-- ...ineItemsPanel.supersededRun-10712.test.tsx | 381 ++++++++++++++++++ packages/plugin-form/src/LineItemsPanel.tsx | 50 ++- 6 files changed, 810 insertions(+), 35 deletions(-) create mode 100644 .changeset/10712-fields-record-query-cannot-query-reset.md create mode 100644 .changeset/10712-plugin-form-line-items-superseded-run.md create mode 100644 packages/fields/src/widgets/useRecordQuery.cannotQuery-10712.test.tsx create mode 100644 packages/plugin-form/src/LineItemsPanel.supersededRun-10712.test.tsx diff --git a/.changeset/10712-fields-record-query-cannot-query-reset.md b/.changeset/10712-fields-record-query-cannot-query-reset.md new file mode 100644 index 0000000000..f308da9005 --- /dev/null +++ b/.changeset/10712-fields-record-query-cannot-query-reset.md @@ -0,0 +1,25 @@ +--- +'@object-ui/fields': patch +--- + +fix(fields): `useRecordQuery` commits nothing once it can no longer query, and a pending debounced search never runs an older query (objectui#10712) + +`useRecordQuery`, the query kernel behind `LookupField`, the people picker and +`RecordPickerDialog`, cleared its records when `enabled` went false (a dialog +closing), but a read still in flight at that moment was the latest run: its +answer committed over the cleared state, and the loading state stayed on until +it landed. `reset()` had the same gap. When `objectName` or `dataSource` went +null nothing was cleared at all, so the previous object's records stayed and a +read in flight committed. + +Now the reset applies whenever the hook can no longer query (`enabled` false, +or no data source or object name) and on `reset()`: it supersedes any read in +flight, so that read's answer commits nothing, ends the loading state, drops a +pending debounced search and clears the query state. + +The debounced `setSearch` timer also ran the query it was armed with. A filter, +object, data-source or page-size change inside the debounce window already +re-ran the query with the typed term; the timer then ran the older query too, +and as the latest call its answer won, so the results shown were for the +previous filter. Now a pending debounced search is dropped when the query +changes. No prop, export or option changes. diff --git a/.changeset/10712-plugin-form-line-items-superseded-run.md b/.changeset/10712-plugin-form-line-items-superseded-run.md new file mode 100644 index 0000000000..951a4247ea --- /dev/null +++ b/.changeset/10712-plugin-form-line-items-superseded-run.md @@ -0,0 +1,25 @@ +--- +'@object-ui/plugin-form': patch +--- + +fix(plugin-form): the line-items panel commits only the answer to its current load, and a save only to the parent it was issued for (objectui#10712) + +The `record:line_items` panel (`LineItemsPanel`) loads its rows again whenever +the parent record or a load input changes. When a load was superseded by a +newer one while it was in flight (the host moved the panel to another parent, +say), its answer still landed. Landing last, it replaced the current parent's +lines (since objectui#10740 the rows it brought were refused as another +parent's, so the current lines were gone either way); landing first, it ended +the loading state and drew the grid while the current load was still pending. +Now a superseded load commits nothing: not the rows, not the end of the loading +state. + +The panel's save reloaded the rows through the `load` it captured when Save was +clicked. A save that landed after the panel had moved to another parent +re-read the old parent, its lines were then committed into the new parent's +panel (and, since objectui#10740, refused there behind the placeholder), and +its failure was written over the new parent's panel. Now a save reloads only +while the parent it saved is still on screen, reading the panel's current +inputs (sort, limit, filter) rather than the ones captured at the click, and a +save whose parent has left the screen neither re-reads nor reports. No prop, +export or schema key changes. diff --git a/packages/fields/src/widgets/useRecordQuery.cannotQuery-10712.test.tsx b/packages/fields/src/widgets/useRecordQuery.cannotQuery-10712.test.tsx new file mode 100644 index 0000000000..5b4215cf7f --- /dev/null +++ b/packages/fields/src/widgets/useRecordQuery.cannotQuery-10712.test.tsx @@ -0,0 +1,288 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#10712 — `useRecordQuery` commits nothing once it can no longer + * query, and a pending debounced search never runs an older query. + * + * PR objectui#10723 numbered every `runQuery` call that issues a read + * (`runSeqRef`), so only the latest call commits. Three ways a read in flight + * is superseded were left outside that number: + * + * - `enabled` goes false (a dialog closes). The reset effect cleared the + * records, but the read in flight was still the latest run: its answer + * then committed over the cleared state, and until it landed `loading` + * stayed true — against the hook's own header, which says it clears + * itself when `enabled` goes false. `reset()` had the same gap. + * - `objectName` or `dataSource` goes null. Nothing reset at all: the + * previous object's records stayed, and a read in flight committed. + * - The debounced `setSearch` timer closes over the `runQuery` in scope when + * it was armed. A filter, object or data-source change inside the debounce + * window re-ran the query through the fetch effect (with the typed term), + * and the timer then ran the OLDER closure — the previous filter — as a + * call numbered latest, so its answer won. + * + * Now the reset is keyed on the hook's ability to query (`enabled`, the data + * source and the object together): it bumps the run number, ends `loading`, + * clears a pending debounced search and clears the query state; `reset()` + * does the same. And a pending debounced search is dropped whenever `runQuery` + * changes, since the fetch effect issues the current query with the typed term + * then. + * + * Every `find` returns a promise the test settles by hand, so each ordering + * below is the ordering the answers really land in. The one timer here is the + * hook's own debounce, given a real window the test outwaits. + */ +import { renderHook, act, waitFor } from '@testing-library/react'; +import { describe, it, expect, vi } from 'vitest'; +import { useRecordQuery } from './useRecordQuery'; + +interface Deferred { + promise: Promise; + resolve: (value: T) => void; + reject: (reason: unknown) => void; +} + +function deferred(): Deferred { + let resolve!: (value: T) => void; + let reject!: (reason: unknown) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +} + +type FindCall = Deferred & { objectName: string; params: Record }; + +/** Every `find` returns a promise the test settles by hand. */ +function makeDeferredDataSource() { + const calls: FindCall[] = []; + const dataSource = { + find: vi.fn((objectName: string, params: Record) => { + const d = deferred(); + calls.push({ ...d, objectName, params }); + return d.promise; + }), + } as any; + return { dataSource, calls }; +} + +/** The `n`-th `find` call (1-based) once it has been issued. */ +async function nth(calls: FindCall[], n: number): Promise { + await waitFor(() => { + if (calls.length < n) throw new Error(`find ${n} not issued yet (${calls.length} so far)`); + }); + return calls[n - 1]; +} + +const answer = (call: FindCall, label: string, total: number) => + act(async () => { + call.resolve({ data: [{ id: label, name: label }], total }); + await Promise.resolve(); + }); + +const names = (records: any[]) => records.map((r) => r.name); + +/** Outwaits the hook's debounce window (`DEBOUNCE_MS`) with real timers. */ +const DEBOUNCE_MS = 30; +const outwaitDebounce = () => + act(async () => { + await new Promise((r) => setTimeout(r, DEBOUNCE_MS * 3)); + }); + +describe('useRecordQuery commits nothing once it can no longer query (objectui#10712)', () => { + it('control: an enabled query commits its records and ends loading', async () => { + const { dataSource, calls } = makeDeferredDataSource(); + const { result } = renderHook(() => useRecordQuery({ dataSource, objectName: 'o' })); + + const first = await nth(calls, 1); + expect(result.current.loading).toBe(true); + await answer(first, 'only', 1); + + expect(names(result.current.records)).toEqual(['only']); + expect(result.current.loading).toBe(false); + expect(dataSource.find).toHaveBeenCalledTimes(1); + }); + + it('`enabled` goes false while a read is in flight: loading ends at once, and the late answer commits nothing', async () => { + const { dataSource, calls } = makeDeferredDataSource(); + const { result, rerender } = renderHook( + ({ enabled }) => useRecordQuery({ dataSource, objectName: 'o', enabled }), + { initialProps: { enabled: true } }, + ); + const inFlight = await nth(calls, 1); + expect(result.current.loading).toBe(true); + + rerender({ enabled: false }); + expect(result.current.loading, 'the disable reset left the loading state of a read it superseded').toBe(false); + expect(result.current.records).toEqual([]); + + await answer(inFlight, 'late', 1); + + expect(names(result.current.records), 'a read superseded by the disable committed after the reset').toEqual([]); + expect(result.current.loading).toBe(false); + expect(dataSource.find).toHaveBeenCalledTimes(1); + }); + + it('control: re-enabled while the superseded read is still in flight, the new query is the one that commits (its run number already supersedes the earlier read)', async () => { + const { dataSource, calls } = makeDeferredDataSource(); + const { result, rerender } = renderHook( + ({ enabled }) => useRecordQuery({ dataSource, objectName: 'o', enabled }), + { initialProps: { enabled: true } }, + ); + const superseded = await nth(calls, 1); + + rerender({ enabled: false }); + rerender({ enabled: true }); + const current = await nth(calls, 2); + expect(result.current.loading).toBe(true); + + await answer(superseded, 'late', 9); + expect(names(result.current.records), 'the superseded read committed over the re-enabled query').toEqual([]); + expect(result.current.loading, 'the superseded read ended the loading state of the re-enabled query').toBe(true); + + await answer(current, 'fresh', 1); + expect(names(result.current.records)).toEqual(['fresh']); + expect(result.current.total).toBe(1); + expect(result.current.loading).toBe(false); + }); + + it('`reset()` while a read is in flight: loading ends at once, and the late answer commits nothing', async () => { + const { dataSource, calls } = makeDeferredDataSource(); + const { result } = renderHook(() => useRecordQuery({ dataSource, objectName: 'o' })); + const inFlight = await nth(calls, 1); + expect(result.current.loading).toBe(true); + + act(() => result.current.reset()); + expect(result.current.loading, '`reset()` left the loading state of a read it superseded').toBe(false); + + await answer(inFlight, 'late', 1); + + expect(names(result.current.records), 'a read superseded by `reset()` committed after it').toEqual([]); + expect(result.current.loading).toBe(false); + }); + + it('`objectName` goes null: the previous object’s records are cleared, and a read in flight commits nothing', async () => { + const { dataSource, calls } = makeDeferredDataSource(); + const { result, rerender } = renderHook( + ({ objectName }) => useRecordQuery({ dataSource, objectName }), + { initialProps: { objectName: 'o' as string | null } }, + ); + await answer(await nth(calls, 1), 'first', 1); + expect(names(result.current.records)).toEqual(['first']); + + act(() => result.current.refetch()); + const inFlight = await nth(calls, 2); + expect(result.current.loading).toBe(true); + + rerender({ objectName: null }); + expect(names(result.current.records), 'the previous object’s records stayed after the object went null').toEqual([]); + expect(result.current.loading, 'loading stayed on for a read of an object the hook no longer has').toBe(false); + + await answer(inFlight, 'late', 1); + + expect(names(result.current.records), 'a read for the previous object committed after it went null').toEqual([]); + expect(result.current.loading).toBe(false); + expect(dataSource.find).toHaveBeenCalledTimes(2); + }); + + it('`dataSource` goes null: the same reset, and the read in flight commits nothing', async () => { + const { dataSource, calls } = makeDeferredDataSource(); + const { result, rerender } = renderHook( + ({ ds }) => useRecordQuery({ dataSource: ds, objectName: 'o' }), + { initialProps: { ds: dataSource as any | null } }, + ); + const inFlight = await nth(calls, 1); + expect(result.current.loading).toBe(true); + + rerender({ ds: null }); + expect(result.current.loading, 'loading stayed on for a read through a data source the hook no longer has').toBe(false); + + await answer(inFlight, 'late', 1); + + expect(names(result.current.records), 'a read through the previous data source committed after it went null').toEqual([]); + expect(result.current.loading).toBe(false); + }); +}); + +describe('useRecordQuery drops a pending debounced search when the query it was armed for changes (objectui#10712)', () => { + it('control: with nothing else changing, the debounced search fires once with its term', async () => { + const { dataSource, calls } = makeDeferredDataSource(); + const { result } = renderHook(() => + useRecordQuery({ dataSource, objectName: 'o', filter: { status: 'open' }, debounceMs: DEBOUNCE_MS }), + ); + await answer(await nth(calls, 1), 'initial', 1); + + act(() => result.current.setSearch('a')); + await outwaitDebounce(); + + expect(dataSource.find).toHaveBeenCalledTimes(2); + const searched = calls[1]; + expect(searched.params.$search).toBe('a'); + expect(searched.params.$filter).toEqual({ status: 'open' }); + await answer(searched, 'a', 1); + expect(names(result.current.records)).toEqual(['a']); + }); + + it('a filter change inside the debounce window: the fetch effect carries the typed term, and the armed timer runs no older query', async () => { + const { dataSource, calls } = makeDeferredDataSource(); + const { result, rerender } = renderHook( + ({ filter }) => useRecordQuery({ dataSource, objectName: 'o', filter, debounceMs: DEBOUNCE_MS }), + { initialProps: { filter: { status: 'open' } as Record } }, + ); + await answer(await nth(calls, 1), 'initial', 1); + + act(() => result.current.setSearch('a')); + // The filter changes before the debounce fires. The fetch effect re-runs + // the query at once, with the current filter and the typed term. + rerender({ filter: { status: 'closed' } }); + const current = await nth(calls, 2); + expect(current.params.$filter).toEqual({ status: 'closed' }); + expect(current.params.$search).toBe('a'); + + await outwaitDebounce(); + + expect(dataSource.find, 'the debounced timer ran the query it was armed with, for the previous filter').toHaveBeenCalledTimes(2); + + await answer(current, 'closed a', 1); + expect(names(result.current.records)).toEqual(['closed a']); + expect(result.current.loading).toBe(false); + }); + + it('control: `reset()` inside the debounce window drops the pending search, as before', async () => { + const { dataSource, calls } = makeDeferredDataSource(); + const { result } = renderHook(() => useRecordQuery({ dataSource, objectName: 'o', debounceMs: DEBOUNCE_MS })); + await answer(await nth(calls, 1), 'initial', 1); + + act(() => result.current.setSearch('a')); + act(() => result.current.reset()); + await outwaitDebounce(); + + expect(dataSource.find, 'a pending search ran after `reset()`').toHaveBeenCalledTimes(1); + expect(result.current.search).toBe(''); + expect(result.current.records).toEqual([]); + }); + + it('`enabled` goes false inside the debounce window: the pending search never runs', async () => { + const { dataSource, calls } = makeDeferredDataSource(); + const { result, rerender } = renderHook( + ({ enabled }) => useRecordQuery({ dataSource, objectName: 'o', enabled, debounceMs: DEBOUNCE_MS }), + { initialProps: { enabled: true } }, + ); + await answer(await nth(calls, 1), 'initial', 1); + + act(() => result.current.setSearch('a')); + rerender({ enabled: false }); + await outwaitDebounce(); + + expect(dataSource.find, 'a pending search ran after the hook was disabled').toHaveBeenCalledTimes(1); + expect(result.current.records).toEqual([]); + expect(result.current.loading).toBe(false); + }); +}); diff --git a/packages/fields/src/widgets/useRecordQuery.ts b/packages/fields/src/widgets/useRecordQuery.ts index edbc46ff27..d010cbf927 100644 --- a/packages/fields/src/widgets/useRecordQuery.ts +++ b/packages/fields/src/widgets/useRecordQuery.ts @@ -22,7 +22,9 @@ import type { DataSource, QueryParams } from '@object-ui/types'; * (`{ data, total }`, tolerating a bare array), * - owns `records`/`loading`/`error`/`total` plus the `page`/`search`/`sort` * controls, with a debounced search path, - * - clears itself when `enabled` goes false (e.g. a dialog closing). + * - clears itself when it can no longer query — `enabled` false (e.g. a dialog + * closing), or no data source or object name — superseding any read still in + * flight, so that read's answer commits nothing (objectui#10712). * * The caller keeps ownership of *selection* state and of record→option mapping; * this hook only answers "what records match the current query". It is the @@ -146,9 +148,17 @@ export function useRecordQuery(options: UseRecordQueryOptions): UseRecordQueryRe const debounceTimer = useRef | null>(null); // objectui#10712 (objectui#10713) — the number of the latest `runQuery` call - // that issued a read. Held in a ref: nothing renders from it. + // that issued a read. Held in a ref: nothing renders from it. The reset + // below moves it too, so a read in flight at the reset is superseded. const runSeqRef = useRef(0); + const clearDebounce = useCallback(() => { + if (debounceTimer.current) { + clearTimeout(debounceTimer.current); + debounceTimer.current = null; + } + }, []); + // Stable signatures for object/array inputs so the fetch effect keys on their // *value*, not a fresh reference every render. const filterSignature = useMemo( @@ -229,36 +239,51 @@ export function useRecordQuery(options: UseRecordQueryOptions): UseRecordQueryRe // eslint-disable-next-line react-hooks/exhaustive-deps }, [canQuery, page, sort, runQuery]); - // Reset query state when disabled — separate from the fetch effect so resets - // don't re-trigger a fetch (React #185). + // Clear the query state, superseding any read in flight (objectui#10712). + // The run number moves, so that read's answer commits nothing: it was the + // latest run, and before this card it committed over the cleared state. + // `loading` is ended here, since that run no longer may end it, and a pending + // debounced search is dropped, since it would query what the hook is + // clearing. This is both the consumer's `reset()` and the reset the effect + // below applies when the hook can no longer query. + const reset = useCallback(() => { + runSeqRef.current += 1; + clearDebounce(); + setLoading(false); + setRecords([]); + setError(null); + setSearchState(''); + setPageState(1); + setSortState(null); + }, [clearDebounce]); + + // Reset query state when the hook can no longer query — separate from the + // fetch effect so resets don't re-trigger a fetch (React #185). Keyed on + // `canQuery`, not `enabled` alone (objectui#10712): `objectName` or + // `dataSource` going null had no reset, so the previous object's records + // stayed and a read in flight committed. useEffect(() => { - if (!enabled) { - setRecords([]); - setError(null); - setSearchState(''); - setPageState(1); - setSortState(null); - } - }, [enabled]); + if (!canQuery) reset(); + }, [canQuery, reset]); - // Clean up the debounce timer on unmount. - useEffect( - () => () => { - if (debounceTimer.current) clearTimeout(debounceTimer.current); - }, - [], - ); + // Drop a pending debounced search whenever `runQuery` changes, and on unmount + // (objectui#10712). The timer closes over the `runQuery` in scope when it was + // armed; a filter, object, data-source or page-size change inside the + // debounce window re-runs the query through the fetch effect above, with the + // current `search`, and the timer would then run the OLDER closure as a call + // numbered latest. + useEffect(() => () => clearDebounce(), [runQuery, clearDebounce]); const setSearch = useCallback( (query: string) => { setSearchState(query); setPageState(1); - if (debounceTimer.current) clearTimeout(debounceTimer.current); + clearDebounce(); debounceTimer.current = setTimeout(() => { runQuery(query, 1, sort); }, debounceMs); }, - [runQuery, sort, debounceMs], + [runQuery, sort, debounceMs, clearDebounce], ); const setPage = useCallback((next: number) => { @@ -279,15 +304,6 @@ export function useRecordQuery(options: UseRecordQueryOptions): UseRecordQueryRe setPageState(1); }, []); - const reset = useCallback(() => { - if (debounceTimer.current) clearTimeout(debounceTimer.current); - setRecords([]); - setError(null); - setSearchState(''); - setPageState(1); - setSortState(null); - }, []); - const refetch = useCallback(() => { runQuery(search, page, sort); }, [runQuery, search, page, sort]); diff --git a/packages/plugin-form/src/LineItemsPanel.supersededRun-10712.test.tsx b/packages/plugin-form/src/LineItemsPanel.supersededRun-10712.test.tsx new file mode 100644 index 0000000000..785218d26e --- /dev/null +++ b/packages/plugin-form/src/LineItemsPanel.supersededRun-10712.test.tsx @@ -0,0 +1,381 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#10712 — the `record:line_items` panel (`LineItemsPanel`) commits + * only the answer to its CURRENT load, and its save commits only to the + * parent it was issued for. + * + * The panel's `load` is numbered (`loadSeqRef`, PR objectui#10738), and since + * that PR only the current run writes the failure banner. This pin is about + * the rest of what a run writes — the rows commit (`setRows` / `setOriginal` / + * `setDirty`, with `rowsHeldFor` beside them since objectui#10740) and the + * `loading` release — and about the save, which called the `load` it captured + * at the click: + * + * - A load a newer one has superseded (another `parentId` while it was in + * flight) commits nothing. It neither replaces the current parent's lines + * nor ends the loading state the current load is still in. Before this + * card a superseded answer landing last replaced the current lines (since + * objectui#10740 the held-for-another-parent placeholder stood where the + * current lines had been; either way the current answer was gone), and + * one landing first ended the loading state, and drew the grid, while the + * current load was still pending. + * - A save that lands after the panel has moved to another parent re-reads + * nothing and reports nothing: its reload and its failure belong to the + * parent it saved, which is no longer on screen. Before this card it + * re-read the OLD parent through the captured `load` — a run numbered + * latest, so it committed over the new parent's panel — and wrote its + * failure there. A save whose parent is still on screen reloads it with the + * panel's CURRENT inputs (sort, limit, filter), not the ones captured at + * the click. + * + * Rendered through the real `SchemaRenderer` and this package's own + * registration of `record:line_items`. Every `find` and every + * `batchTransaction` returns a promise the test settles by hand, so each + * ordering below is the ordering the answers really land in; no timer decides + * it. The loading state is read through the grid's absence together with the + * panel's own loading paragraph; the refused state through its test id; the + * banner through the message the adapter threw. + */ +import React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, act, cleanup, waitFor, fireEvent } from '@testing-library/react'; +import { SchemaRenderer, SchemaRendererProvider } from '@object-ui/react'; +// Registers `record:line_items` through this package's own entry. +import './index'; + +interface Deferred { + promise: Promise; + resolve: (value: T) => void; + reject: (reason: unknown) => void; +} + +function deferred(): Deferred { + let resolve!: (value: T) => void; + let reject!: (reason: unknown) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +} + +type LinesRead = Deferred & { parentId: unknown; top: unknown }; +type Save = Deferred & { ops: any[] }; + +/** + * Every row load (`find`) and every save (`batchTransaction`) returns a promise + * the test settles by hand. The child schema read is answered at once with no + * schema, so child payloads are persisted as the grid holds them. + */ +function makeLinesDataSource() { + const lineReads: LinesRead[] = []; + const saves: Save[] = []; + const dataSource = { + getObjectSchema: vi.fn(async () => null), + find: vi.fn((_objectName: string, query: any) => { + const d = deferred(); + lineReads.push({ ...d, parentId: query?.$filter?.po, top: query?.$top }); + return d.promise; + }), + batchTransaction: vi.fn((ops: any[]) => { + const d = deferred(); + saves.push({ ...d, ops }); + return d.promise; + }), + }; + return { dataSource, lineReads, saves }; +} + +/** A panel on an existing parent record, over one text column. */ +const linesBlock = (over: Record = {}) => ({ + type: 'record:line_items', + childObject: 'po_line', + relationshipField: 'po', + parentObject: 'po', + parentId: 'p1', + columns: [{ name: 'label', label: 'Line', type: 'text' }], + ...over, +}); + +function mount(dataSource: unknown, block: Record) { + const tree = (b: Record) => ( + + + + ); + const view = render(tree(block)); + return { rerender: (next: Record) => view.rerender(tree(next)) }; +} + +/** The `n`-th read or save (1-based) once it has been issued. */ +async function nth(items: T[], n: number): Promise { + await waitFor(() => { + if (items.length < n) throw new Error(`item ${n} not issued yet (${items.length} so far)`); + }); + return items[n - 1]; +} + +async function settle(fn: () => void) { + await act(async () => { + fn(); + await Promise.resolve(); + }); +} + +const answerLines = (read: LinesRead, ...lines: Array<{ id: string; label: string }>) => + settle(() => read.resolve({ data: lines })); +const fail = (read: Deferred, message: string) => settle(() => read.reject(new Error(message))); +const landSave = (save: Save) => settle(() => save.resolve({ results: [] })); + +/** The grid's line inputs; the grid always trails one blank entry row. */ +const lineInputs = () => + Array.from(document.body.querySelectorAll('input[aria-label="Line"]')) as HTMLInputElement[]; +/** The lines the grid draws (its trailing blank row is not a line). */ +const shownLines = () => lineInputs().map((input) => input.value).filter((value) => value !== ''); +const saveButton = () => + Array.from(document.body.querySelectorAll('button')).find((b) => /^(Save|Saving…)$/.test(b.textContent ?? '')) as + | HTMLButtonElement + | undefined; +/** Whether the panel's banner reports `message` (the text the adapter threw). */ +const bannerShows = (message: string) => + Array.from(document.body.querySelectorAll('p')).some((p) => p.textContent === message); +const placeholder = () => document.body.querySelector('[data-testid="line-items-held-for-another-parent"]'); +/** The panel's own loading paragraph, drawn while no grid is. */ +const loadingShown = () => Array.from(document.body.querySelectorAll('p')).some((p) => p.textContent === 'Loading…'); + +async function editFirstLine(to: string) { + await act(async () => { + fireEvent.change(lineInputs()[0], { target: { value: to } }); + }); +} + +async function clickSave() { + const button = saveButton(); + expect(button, 'no Save button').toBeTruthy(); + await act(async () => { + fireEvent.click(button!); + }); +} + +/** Mounts a panel on `p1` whose first read is still in flight. */ +async function mountP1Pending() { + const ds = makeLinesDataSource(); + const view = mount(ds.dataSource, linesBlock()); + const first = await nth(ds.lineReads, 1); + expect(first.parentId).toBe('p1'); + return { ...ds, view, first }; +} + +/** Mounts a panel on `p1`, lands `l1`, and edits it; Save is then enabled. */ +async function mountP1Edited() { + const ds = makeLinesDataSource(); + const view = mount(ds.dataSource, linesBlock()); + const first = await nth(ds.lineReads, 1); + expect(first.parentId).toBe('p1'); + await answerLines(first, { id: 'l1', label: 'p1 line' }); + await waitFor(() => expect(shownLines()).toEqual(['p1 line'])); + await editFirstLine('p1 line edited'); + await waitFor(() => expect(saveButton()?.disabled).toBe(false)); + return { ...ds, view }; +} + +/** `mountP1Edited`, then Save with its batch left in flight. */ +async function mountP1Saving() { + const held = await mountP1Edited(); + await clickSave(); + const save = await nth(held.saves, 1); + expect(save.ops).toEqual([ + { object: 'po_line', action: 'update', id: 'l1', data: { label: 'p1 line edited', po: 'p1' } }, + ]); + expect(held.dataSource.find).toHaveBeenCalledTimes(1); + return { ...held, save }; +} + +beforeEach(() => { + // Each failure below is logged by the panel; keep the run readable. + vi.spyOn(console, 'error').mockImplementation(() => {}); +}); +afterEach(() => { + vi.restoreAllMocks(); + cleanup(); +}); + +describe('LineItemsPanel commits only the answer to its CURRENT load (objectui#10712)', () => { + it('control: a single load draws its lines, ends loading, and leaves Save off', async () => { + const { dataSource, first } = await mountP1Pending(); + expect(loadingShown()).toBe(true); + expect(lineInputs()).toHaveLength(0); + + await answerLines(first, { id: 'l1', label: 'p1 line' }); + + await waitFor(() => expect(shownLines()).toEqual(['p1 line'])); + expect(loadingShown()).toBe(false); + expect(placeholder()).toBeNull(); + expect(saveButton()?.disabled).toBe(true); + expect(dataSource.find).toHaveBeenCalledTimes(1); + }); + + it('values: a SUPERSEDED load that lands LAST leaves the current parent’s lines on screen', async () => { + const { lineReads, view, first } = await mountP1Pending(); + + // Another parent is asked for while the first load is still in flight. + view.rerender(linesBlock({ parentId: 'p2' })); + const second = await nth(lineReads, 2); + expect(second.parentId).toBe('p2'); + await answerLines(second, { id: 'l2', label: 'p2 line' }); + await waitFor(() => expect(shownLines()).toEqual(['p2 line'])); + + await answerLines(first, { id: 'l1', label: 'p1 line (stale)' }); + await settle(() => {}); + + expect(shownLines(), 'the superseded load for p1 replaced the lines of p2').toEqual(['p2 line']); + expect(placeholder(), 'the superseded rows were held for p1 and refused in p2’s panel').toBeNull(); + expect(saveButton()?.disabled).toBe(true); + }); + + it('loading: a SUPERSEDED load that FAILS first does not end the loading state the current load is in', async () => { + const { lineReads, view, first } = await mountP1Pending(); + + view.rerender(linesBlock({ parentId: 'p2' })); + const second = await nth(lineReads, 2); + await fail(first, 'superseded load failed'); + await settle(() => {}); + + expect(loadingShown(), 'a superseded failure ended the loading state the current load was in').toBe(true); + expect(lineInputs(), 'a superseded failure drew the grid while the current load was pending').toHaveLength(0); + expect(bannerShows('superseded load failed')).toBe(false); + + await answerLines(second, { id: 'l2', label: 'p2 line' }); + await waitFor(() => expect(shownLines(), 'the current load did not end the loading state').toEqual(['p2 line'])); + expect(loadingShown()).toBe(false); + }); + + it('loading: a SUPERSEDED load that SUCCEEDS first neither draws nor ends the loading state the current load is in', async () => { + const { lineReads, view, first } = await mountP1Pending(); + + view.rerender(linesBlock({ parentId: 'p2' })); + const second = await nth(lineReads, 2); + await answerLines(first, { id: 'l1', label: 'p1 line (stale)' }); + await settle(() => {}); + + expect(loadingShown(), 'a superseded answer ended the loading state the current load was in').toBe(true); + expect(lineInputs()).toHaveLength(0); + expect(placeholder(), 'a superseded answer was committed and then refused as another parent’s').toBeNull(); + + await answerLines(second, { id: 'l2', label: 'p2 line' }); + await waitFor(() => expect(shownLines()).toEqual(['p2 line'])); + expect(loadingShown()).toBe(false); + }); +}); + +describe('LineItemsPanel’s save commits only to the parent it was issued for (objectui#10712)', () => { + it('control: a save whose parent is still on screen reloads it and draws the reloaded lines', async () => { + const { dataSource, lineReads, save } = await mountP1Saving(); + expect(lineInputs().every((input) => input.disabled)).toBe(true); + + await landSave(save); + const reload = await nth(lineReads, 2); + expect(reload.parentId).toBe('p1'); + await answerLines(reload, { id: 'l1', label: 'p1 line saved' }); + + await waitFor(() => expect(shownLines()).toEqual(['p1 line saved'])); + expect(lineInputs().every((input) => !input.disabled)).toBe(true); + expect(saveButton()?.disabled).toBe(true); + expect(dataSource.find).toHaveBeenCalledTimes(2); + }); + + it('a save started on p1 that LANDS after the swap to p2 re-reads nothing and leaves p2’s lines on screen', async () => { + const { dataSource, lineReads, view, save } = await mountP1Saving(); + + // The host moves the panel to p2 while p1's batch is in flight; p2 loads. + view.rerender(linesBlock({ parentId: 'p2' })); + const second = await nth(lineReads, 2); + expect(second.parentId).toBe('p2'); + await answerLines(second, { id: 'l2', label: 'p2 line' }); + await waitFor(() => expect(shownLines()).toEqual(['p2 line'])); + + await landSave(save); + await settle(() => {}); + + expect(dataSource.find, 'the landed save re-read the OLD parent through the load it captured').toHaveBeenCalledTimes(2); + expect(shownLines()).toEqual(['p2 line']); + expect(placeholder()).toBeNull(); + await waitFor(() => expect(lineInputs().every((input) => !input.disabled), 'the save never ended').toBe(true)); + expect(saveButton()?.disabled).toBe(true); + }); + + it('a save started on p1 that FAILS after the swap to p2 does not raise its failure over p2’s panel', async () => { + const { dataSource, lineReads, view, save } = await mountP1Saving(); + + view.rerender(linesBlock({ parentId: 'p2' })); + const second = await nth(lineReads, 2); + await answerLines(second, { id: 'l2', label: 'p2 line' }); + await waitFor(() => expect(shownLines()).toEqual(['p2 line'])); + + await settle(() => save.reject(new Error('p1 save failed'))); + await settle(() => {}); + + expect(bannerShows('p1 save failed'), 'a save failure for p1 was raised over p2’s panel').toBe(false); + expect(shownLines()).toEqual(['p2 line']); + expect(dataSource.find).toHaveBeenCalledTimes(2); + await waitFor(() => expect(lineInputs().every((input) => !input.disabled)).toBe(true)); + }); + + it('a save started on p1 that lands after a swap to p2 whose load FAILED re-reads nothing, and p2’s failure banner stays', async () => { + const { dataSource, lineReads, view, save } = await mountP1Saving(); + + view.rerender(linesBlock({ parentId: 'p2' })); + const second = await nth(lineReads, 2); + await fail(second, 'p2 load failed'); + await waitFor(() => expect(bannerShows('p2 load failed')).toBe(true)); + // The rows are still p1's, held for p1: the objectui#10740 placeholder. + expect(placeholder()).toBeTruthy(); + + await landSave(save); + await settle(() => {}); + + expect(dataSource.find, 'the landed save re-read p1 into p2’s panel').toHaveBeenCalledTimes(2); + expect(bannerShows('p2 load failed'), 'the save’s re-read of p1 cleared p2’s failure').toBe(true); + expect(placeholder()).toBeTruthy(); + expect(shownLines()).toEqual([]); + }); + + it('control: a save whose parent is still on screen FAILS: its failure is shown over the edited lines', async () => { + const { dataSource, save } = await mountP1Saving(); + + await settle(() => save.reject(new Error('p1 save failed'))); + + await waitFor(() => expect(bannerShows('p1 save failed')).toBe(true)); + expect(shownLines()).toEqual(['p1 line edited']); + expect(dataSource.find).toHaveBeenCalledTimes(1); + await waitFor(() => expect(saveButton()?.disabled).toBe(false)); + }); + + it('the reload after a save reads the panel’s CURRENT inputs, not the ones captured at the click', async () => { + const { lineReads, view, save } = await mountP1Saving(); + + // A load input other than the parent moves while the batch is in flight + // (the row cap); the panel re-reads p1 with it at once. + view.rerender(linesBlock({ limit: 200 })); + const reload = await nth(lineReads, 2); + expect(reload.parentId).toBe('p1'); + expect(reload.top).toBe(200); + await answerLines(reload, { id: 'l1', label: 'p1 line' }); + + await landSave(save); + const afterSave = await nth(lineReads, 3); + expect(afterSave.parentId).toBe('p1'); + expect(afterSave.top, 'the save reloaded with the row cap captured at the click').toBe(200); + + await answerLines(afterSave, { id: 'l1', label: 'p1 line saved' }); + await waitFor(() => expect(shownLines()).toEqual(['p1 line saved'])); + expect(saveButton()?.disabled).toBe(true); + }); +}); diff --git a/packages/plugin-form/src/LineItemsPanel.tsx b/packages/plugin-form/src/LineItemsPanel.tsx index 42ebd9123f..9681ab8e6a 100644 --- a/packages/plugin-form/src/LineItemsPanel.tsx +++ b/packages/plugin-form/src/LineItemsPanel.tsx @@ -365,6 +365,16 @@ export const LineItemsPanel: React.FC<{ schema: LineItemsPanelSchema }> = ({ sch ...(orderBy ? { $orderby: orderBy } : {}), $top: resolveRowLimit(schema.limit, DEFAULT_LINE_ITEMS_LIMIT), }); + // objectui#10712 — a run a newer one has superseded (another `parentId`, + // say, while this read was in flight) commits nothing: not the rows, not + // their owner, not the banner, and not the end of the loading state the + // current run is still in (the `finally` below reads the same run + // number). Before this card the answer landed either way: last, it + // replaced the current parent's lines (since objectui#10740 the rows it + // brought were refused as another parent's, the current lines gone all + // the same); first, it ended the loading state and drew the grid while + // the current read was pending. + if (!isCurrent()) return; const data = (res?.data ?? []) as Record[]; setRows(data.map((r) => ({ ...r }))); setOriginal(data.map((r) => ({ ...r }))); @@ -376,7 +386,7 @@ export const LineItemsPanel: React.FC<{ schema: LineItemsPanelSchema }> = ({ sch // asks for now, so no earlier failure describes it: not a failed load, // and not a failed save, whose edits these rows replace (objectui#10682, // the objectui#10578 rule: cleared on a commit, never when a load starts). - if (isCurrent()) setError(null); + setError(null); } catch (e: any) { if (isCurrent()) { setError(e?.message || 'Failed to load line items'); @@ -387,7 +397,11 @@ export const LineItemsPanel: React.FC<{ schema: LineItemsPanelSchema }> = ({ sch setRowsHeldFor((held) => held ?? parentId); } } finally { - setLoading(false); + // Only the current run ends the loading state (objectui#10712): a + // superseded run's release would draw the grid over the empty rows while + // the current read is still pending. Every decline above releases it + // before any await, so a run that returns early is never superseded here. + if (isCurrent()) setLoading(false); } }, [ dataSource, @@ -400,6 +414,17 @@ export const LineItemsPanel: React.FC<{ schema: LineItemsPanelSchema }> = ({ sch orderBy, ]); + // objectui#10712 — the panel's latest `load` and the parent it reads, for the + // save's continuation. `save` closes over the render it was clicked in, and + // its batch lands after any number of renders. The reload that follows is the + // PANEL's read, not the click's: it reads the inputs on screen then (sort, + // limit, filter), and it does not run at all once the panel has moved to + // another parent. Written from an effect, never during render. + const latestLoadRef = useRef({ load, parentId }); + useEffect(() => { + latestLoadRef.current = { load, parentId }; + }, [load, parentId]); + useEffect(() => { void load(); }, [load]); @@ -445,6 +470,21 @@ export const LineItemsPanel: React.FC<{ schema: LineItemsPanelSchema }> = ({ sch // The Save button is off in this state; this is the write contract's own // guard, on the function every caller of it reaches. if (heldForAnotherParent) return; + // objectui#10712 — the parent this save is for. Its batch may land after + // the host has moved the panel to another parent, whose own load has by + // then committed (or failed) over these rows. The save's reload and its + // failure belong to the parent it saved. While that parent is still on + // screen, the reload is the panel's current read (`latestLoadRef`), not + // the `load` captured at the click, which would re-read with that render's + // inputs. Once it is not, the save commits nothing: no re-read of the old + // parent into the new parent's panel, and no banner about lines that are + // no longer on screen. Before this card the captured `load` ran as a new + // run numbered latest, so it committed the OLD parent's lines into the new + // parent's panel (refused there as another parent's since objectui#10740, + // its commit clearing the new parent's own failure all the same), and the + // save's failure was written wherever the panel had moved. + const savedParent = parentId; + const parentStillShown = () => latestLoadRef.current.parentId === savedParent; setSaving(true); setError(null); try { @@ -472,13 +512,13 @@ export const LineItemsPanel: React.FC<{ schema: LineItemsPanelSchema }> = ({ sch // directly (no $ref), so slicing off the parent op shifts no references. if (!canRollup) ops = ops.slice(1); if (ops.length) await runBatchTransaction(dataSource, ops); - await load(); + if (parentStillShown()) await latestLoadRef.current.load(); } catch (e: any) { - setError(e?.message || 'Failed to save line items'); + if (parentStillShown()) setError(e?.message || 'Failed to save line items'); } finally { setSaving(false); } - }, [dataSource, parentId, heldForAnotherParent, rows, original, schema, parentObject, load, childSchema]); + }, [dataSource, parentId, heldForAnotherParent, rows, original, schema, parentObject, childSchema]); const gridField = useMemo( () => From bef026bdc5c910aed4fbaf073804422daa29c587 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 12:07:27 +0000 Subject: [PATCH 2/4] fix(plugin-form): the line-items panel holds a same-parent reload while it has unsaved edits (objectui#10712 R3) A change to a load input other than the parent (sort, limit, filter) is held while the panel holds unsaved edits for the current parent: dirty, the rows held for `parentId`, and the last run's parent (and adapter, child object, relationship field) equal to the current ones. A held change issues no read and is not numbered. The next read that runs carries it: the post-save reload, a parent move, or the commit of a same-parent read that was in flight at the hold, which replays one read. The precedent is the default form's bus hold (`formDirtyRef` / `heldChangeRef` in ObjectForm.tsx). The objectui#10740 control row "a RE-load of the SAME parent that fails keeps the held edits" keeps its assertions and changes its trigger to a post-save reload that fails, since a limit change no longer reads while edits are held. Pin: `LineItemsPanel.dirtyReloadHold-10712.test.tsx`; the plugin-form changeset covers R3, frontmatter unchanged. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_014mXUNuFomfj24w7s1pZzhN --- ...2-plugin-form-line-items-superseded-run.md | 17 +- ...eItemsPanel.dirtyReloadHold-10712.test.tsx | 372 ++++++++++++++++++ ...sPanel.heldForAnotherParent-10740.test.tsx | 25 +- ...ineItemsPanel.supersededRun-10712.test.tsx | 14 +- packages/plugin-form/src/LineItemsPanel.tsx | 80 +++- 5 files changed, 487 insertions(+), 21 deletions(-) create mode 100644 packages/plugin-form/src/LineItemsPanel.dirtyReloadHold-10712.test.tsx diff --git a/.changeset/10712-plugin-form-line-items-superseded-run.md b/.changeset/10712-plugin-form-line-items-superseded-run.md index 951a4247ea..20eaeb43ec 100644 --- a/.changeset/10712-plugin-form-line-items-superseded-run.md +++ b/.changeset/10712-plugin-form-line-items-superseded-run.md @@ -2,7 +2,7 @@ '@object-ui/plugin-form': patch --- -fix(plugin-form): the line-items panel commits only the answer to its current load, and a save only to the parent it was issued for (objectui#10712) +fix(plugin-form): the line-items panel commits only the answer to its current load, a save only to the parent it was issued for, and holds a same-parent reload while it has unsaved edits (objectui#10712) The `record:line_items` panel (`LineItemsPanel`) loads its rows again whenever the parent record or a load input changes. When a load was superseded by a @@ -21,5 +21,16 @@ panel (and, since objectui#10740, refused there behind the placeholder), and its failure was written over the new parent's panel. Now a save reloads only while the parent it saved is still on screen, reading the panel's current inputs (sort, limit, filter) rather than the ones captured at the click, and a -save whose parent has left the screen neither re-reads nor reports. No prop, -export or schema key changes. +save whose parent has left the screen neither re-reads nor reports. + +A change to a load input other than the parent (sort, limit or filter) re-read +the rows while the author had unsaved lines, and the read's commit discarded +those lines with no signal. Now, as the default record form already does for +its own background re-read, the panel holds such a change while it has unsaved +edits for the current parent: no read is issued, the edited lines stay drawn +and saveable, and the next read that runs (the post-save reload, a parent move, +or the commit of a same-parent read that was in flight) carries the change. +While a change is held, the grid shows the rows under the previous sort, filter +or limit until the panel saves, with no signal. A change while the panel is +clean, a parent move, and a change of adapter or child object still read as +before. No prop, export or schema key changes. diff --git a/packages/plugin-form/src/LineItemsPanel.dirtyReloadHold-10712.test.tsx b/packages/plugin-form/src/LineItemsPanel.dirtyReloadHold-10712.test.tsx new file mode 100644 index 0000000000..bd3a4363bc --- /dev/null +++ b/packages/plugin-form/src/LineItemsPanel.dirtyReloadHold-10712.test.tsx @@ -0,0 +1,372 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#10712 (R3) — the `record:line_items` panel (`LineItemsPanel`) holds + * a same-parent reload while it has unsaved edits. + * + * The panel re-reads its rows whenever a load input moves: the parent, and the + * query's shape (sort, limit, filter). A read's commit replaces the rows and + * clears `dirty`. So when the host re-rendered the node with another sort, + * limit or filter while the author had unsaved lines, the re-read discarded + * those lines with no signal: the grid drew the server's rows, Save went off, + * and nothing said an edit had been lost. + * + * Now the rule the default form already applies to its own background re-read + * (`ObjectForm.tsx`, `formDirtyRef` / `heldChangeRef`: unsaved input holds the + * re-read; one is replayed when the form is pristine again or its save lands) + * applies here. A change to a load input other than the parent is HELD while + * the panel holds unsaved edits for the CURRENT parent — `dirty`, the rows + * held for `parentId`, and the last run's parent (and adapter, child object + * and relationship field) equal to the current ones. A held change issues no + * read; the next read that runs reads the panel's current inputs, so it + * replays the held change: the post-save reload (which since objectui#10712 + * reads the current inputs, not the click's), a parent move, or — when a + * same-parent read was in flight at the hold — that read's commit, which + * replays one read. + * + * What the rule leaves alone, pinned as controls: a change while the panel is + * clean reloads as before; a parent move while dirty behaves as objectui#10740 + * set (the new parent is read, and the held rows are refused there); a swap + * back to the edited parent after a move is read (the last run's parent was + * the other one); a change of adapter or child object is read (the held rows + * are not that adapter's or that object's). + * + * The cost, stated: while a change is held, the grid shows the rows under the + * previous sort, filter or limit until the panel saves, with no signal. + * + * Rendered through the real `SchemaRenderer` and this package's own + * registration of `record:line_items`. Every `find` and every + * `batchTransaction` returns a promise the test settles by hand; no timer + * decides an ordering. + */ +import React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, act, cleanup, waitFor, fireEvent } from '@testing-library/react'; +import { SchemaRenderer, SchemaRendererProvider } from '@object-ui/react'; +// Registers `record:line_items` through this package's own entry. +import './index'; + +interface Deferred { + promise: Promise; + resolve: (value: T) => void; + reject: (reason: unknown) => void; +} + +function deferred(): Deferred { + let resolve!: (value: T) => void; + let reject!: (reason: unknown) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +} + +type LinesRead = Deferred & { objectName: string; query: any; parentId: unknown }; +type Save = Deferred & { ops: any[] }; + +/** + * Every row load (`find`) and every save (`batchTransaction`) returns a promise + * the test settles by hand. The child schema read is answered at once with no + * schema, so child payloads are persisted as the grid holds them. + */ +function makeLinesDataSource() { + const lineReads: LinesRead[] = []; + const saves: Save[] = []; + const dataSource = { + getObjectSchema: vi.fn(async () => null), + find: vi.fn((objectName: string, query: any) => { + const d = deferred(); + lineReads.push({ ...d, objectName, query, parentId: parentOf(query) }); + return d.promise; + }), + batchTransaction: vi.fn((ops: any[]) => { + const d = deferred(); + saves.push({ ...d, ops }); + return d.promise; + }), + }; + return { dataSource, lineReads, saves }; +} + +/** The parent a query is scoped to: the plain `po` key, or the first leg of the AND-merged node. */ +function parentOf(query: any): unknown { + const f = query?.$filter; + if (f && !Array.isArray(f)) return f.po; + if (Array.isArray(f) && f[0] === 'and') { + const leg = f.find((x: any) => Array.isArray(x) && x[0] === 'po' && x[1] === '='); + return leg?.[2]; + } + return undefined; +} + +/** A panel on an existing parent record, over one text column. */ +const linesBlock = (over: Record = {}) => ({ + type: 'record:line_items', + childObject: 'po_line', + relationshipField: 'po', + parentObject: 'po', + parentId: 'p1', + columns: [{ name: 'label', label: 'Line', type: 'text' }], + ...over, +}); + +const tree = (b: Record, ds: unknown) => ( + + + +); + +function mount(dataSource: unknown, block: Record) { + const view = render(tree(block, dataSource)); + return { + rerender: (next: Record, ds: unknown = dataSource) => view.rerender(tree(next, ds)), + }; +} + +/** The `n`-th read or save (1-based) once it has been issued. */ +async function nth(items: T[], n: number): Promise { + await waitFor(() => { + if (items.length < n) throw new Error(`item ${n} not issued yet (${items.length} so far)`); + }); + return items[n - 1]; +} + +async function settle(fn: () => void) { + await act(async () => { + fn(); + await Promise.resolve(); + }); +} + +const answerLines = (read: LinesRead, ...lines: Array<{ id: string; label: string }>) => + settle(() => read.resolve({ data: lines })); +const fail = (read: Deferred, message: string) => settle(() => read.reject(new Error(message))); +const landSave = (save: Save) => settle(() => save.resolve({ results: [] })); + +/** The grid's line inputs; the grid always trails one blank entry row. */ +const lineInputs = () => + Array.from(document.body.querySelectorAll('input[aria-label="Line"]')) as HTMLInputElement[]; +/** The lines the grid draws (its trailing blank row is not a line). */ +const shownLines = () => lineInputs().map((input) => input.value).filter((value) => value !== ''); +const saveButton = () => + Array.from(document.body.querySelectorAll('button')).find((b) => /^(Save|Saving…)$/.test(b.textContent ?? '')) as + | HTMLButtonElement + | undefined; +const bannerShows = (message: string) => + Array.from(document.body.querySelectorAll('p')).some((p) => p.textContent === message); +const placeholder = () => document.body.querySelector('[data-testid="line-items-held-for-another-parent"]'); +const loadingShown = () => Array.from(document.body.querySelectorAll('p')).some((p) => p.textContent === 'Loading…'); + +async function editFirstLine(to: string) { + await act(async () => { + fireEvent.change(lineInputs()[0], { target: { value: to } }); + }); +} + +async function clickSave() { + const button = saveButton(); + expect(button, 'no Save button').toBeTruthy(); + await act(async () => { + fireEvent.click(button!); + }); +} + +/** Mounts a panel on `p1` and lands `l1`; the panel is clean. */ +async function mountP1Clean() { + const ds = makeLinesDataSource(); + const view = mount(ds.dataSource, linesBlock()); + const first = await nth(ds.lineReads, 1); + expect(first.parentId).toBe('p1'); + await answerLines(first, { id: 'l1', label: 'p1 line' }); + await waitFor(() => expect(shownLines()).toEqual(['p1 line'])); + expect(saveButton()?.disabled).toBe(true); + return { ...ds, view }; +} + +/** `mountP1Clean`, then edits `l1`; Save is then enabled. */ +async function mountP1Edited() { + const held = await mountP1Clean(); + await editFirstLine('p1 line edited'); + await waitFor(() => expect(saveButton()?.disabled).toBe(false)); + return held; +} + +/** The three load inputs other than the parent, each as a re-render of the same node. */ +const INPUT_CHANGES: ReadonlyArray<{ input: string; over: Record; carried: (q: any) => void }> = [ + { input: 'sort', over: { sort: [{ field: 'label', order: 'asc' }] }, carried: (q) => expect(q.$orderby).toEqual({ label: 'asc' }) }, + { input: 'limit', over: { limit: 200 }, carried: (q) => expect(q.$top).toBe(200) }, + { + input: 'filter', + over: { filter: { note: 'x' } }, + carried: (q) => expect(q.$filter).toEqual(['and', ['po', '=', 'p1'], ['note', '=', 'x']]), + }, +]; + +/** Holds the edits still: the edited line drawn editable, Save on, no read pending, no placeholder. */ +function expectEditsHeld() { + expect(shownLines(), 'the unsaved edit was discarded').toEqual(['p1 line edited']); + expect(lineInputs().every((input) => !input.disabled)).toBe(true); + expect(loadingShown()).toBe(false); + expect(placeholder()).toBeNull(); + expect(saveButton()?.disabled, 'Save went off over an unsaved edit').toBe(false); +} + +beforeEach(() => { + // Each failure below is logged by the panel; keep the run readable. + vi.spyOn(console, 'error').mockImplementation(() => {}); +}); +afterEach(() => { + vi.restoreAllMocks(); + cleanup(); +}); + +describe.each(INPUT_CHANGES)('LineItemsPanel holds a $input change while it has unsaved edits (objectui#10712 R3)', ({ over, carried }) => { + it('control: the change while the panel is CLEAN reloads with it, as before', async () => { + const { dataSource, lineReads, view } = await mountP1Clean(); + + view.rerender(linesBlock(over)); + const reload = await nth(lineReads, 2); + expect(reload.parentId).toBe('p1'); + carried(reload.query); + await answerLines(reload, { id: 'l1', label: 'p1 line again' }); + + await waitFor(() => expect(shownLines()).toEqual(['p1 line again'])); + expect(dataSource.find).toHaveBeenCalledTimes(2); + expect(saveButton()?.disabled).toBe(true); + }); + + it('while DIRTY: the change issues no read, keeps the edited line drawn editable, and leaves Save on', async () => { + const { dataSource, view } = await mountP1Edited(); + + view.rerender(linesBlock(over)); + await settle(() => {}); + + expect(dataSource.find, 'a same-parent reload ran over unsaved edits').toHaveBeenCalledTimes(1); + expectEditsHeld(); + }); + + it('the post-save reload then reads the CURRENT inputs, and the held change is replayed by it', async () => { + const { dataSource, lineReads, saves, view } = await mountP1Edited(); + view.rerender(linesBlock(over)); + await settle(() => {}); + expect(dataSource.find).toHaveBeenCalledTimes(1); + + await clickSave(); + const save = await nth(saves, 1); + expect(save.ops).toEqual([ + { object: 'po_line', action: 'update', id: 'l1', data: { label: 'p1 line edited', po: 'p1' } }, + ]); + await landSave(save); + + const reload = await nth(lineReads, 2); + expect(reload.parentId).toBe('p1'); + carried(reload.query); + await answerLines(reload, { id: 'l1', label: 'p1 line saved' }); + + await waitFor(() => expect(shownLines()).toEqual(['p1 line saved'])); + expect(dataSource.find, 'the held change was replayed twice').toHaveBeenCalledTimes(2); + expect(saveButton()?.disabled).toBe(true); + }); +}); + +describe('what the hold leaves alone (objectui#10712 R3 controls)', () => { + it('a parent move while dirty is read, and the held rows are refused there (objectui#10740): p2 is read, its failure draws the placeholder, Save is off', async () => { + const { dataSource, lineReads, view } = await mountP1Edited(); + view.rerender(linesBlock({ sort: [{ field: 'label', order: 'asc' }] })); + await settle(() => {}); + expect(dataSource.find).toHaveBeenCalledTimes(1); + + view.rerender(linesBlock({ parentId: 'p2', sort: [{ field: 'label', order: 'asc' }] })); + const second = await nth(lineReads, 2); + expect(second.parentId).toBe('p2'); + expect(second.query.$orderby).toEqual({ label: 'asc' }); + await fail(second, 'p2 load failed'); + + await waitFor(() => expect(bannerShows('p2 load failed')).toBe(true)); + expect(placeholder()).toBeTruthy(); + expect(shownLines()).toEqual([]); + expect(saveButton()?.disabled).toBe(true); + }); + + it('a swap BACK to the edited parent after a move is read: the last run’s parent was the other one', async () => { + const { dataSource, lineReads, view } = await mountP1Edited(); + + view.rerender(linesBlock({ parentId: 'p2' })); + await fail(await nth(lineReads, 2), 'p2 load failed'); + await waitFor(() => expect(bannerShows('p2 load failed')).toBe(true)); + + view.rerender(linesBlock({ parentId: 'p1' })); + const third = await nth(lineReads, 3); + expect(third.parentId, 'the swap back to p1 was held as a same-parent change').toBe('p1'); + expect(dataSource.find).toHaveBeenCalledTimes(3); + // objectui#10740's same-parent rule: its failure keeps p1's held edit drawn. + await fail(third, 'p1 load failed again'); + await waitFor(() => expect(bannerShows('p1 load failed again')).toBe(true)); + expect(shownLines()).toEqual(['p1 line edited']); + expect(saveButton()?.disabled).toBe(false); + }); + + it('a change held while a same-parent read was IN FLIGHT is replayed when that read commits', async () => { + const { dataSource, lineReads, view } = await mountP1Edited(); + + // The host moves away and back while p2's read is pending; p1's third read + // is in flight, its answer for the inputs it was issued with. + view.rerender(linesBlock({ parentId: 'p2' })); + await nth(lineReads, 2); + view.rerender(linesBlock({ parentId: 'p1' })); + const third = await nth(lineReads, 3); + expect(third.parentId).toBe('p1'); + expect(third.query.$top).toBe(500); + + // A limit change lands while that read is in flight: held (dirty, held for + // p1, last run p1), so it supersedes nothing. + view.rerender(linesBlock({ parentId: 'p1', limit: 200 })); + await settle(() => {}); + expect(dataSource.find).toHaveBeenCalledTimes(3); + + // The in-flight read commits; the held change is replayed by one read + // that carries the current limit. + await answerLines(third, { id: 'l1', label: 'p1 line' }); + const fourth = await nth(lineReads, 4); + expect(fourth.parentId).toBe('p1'); + expect(fourth.query.$top, 'the replayed read did not carry the held change').toBe(200); + await answerLines(fourth, { id: 'l1', label: 'p1 line at 200' }); + + await waitFor(() => expect(shownLines()).toEqual(['p1 line at 200'])); + expect(dataSource.find).toHaveBeenCalledTimes(4); + expect(saveButton()?.disabled).toBe(true); + }); + + it('a change of ADAPTER while dirty is read: the held rows are not the new adapter’s', async () => { + const { dataSource, view } = await mountP1Edited(); + const other = makeLinesDataSource(); + + view.rerender(linesBlock(), other.dataSource); + const read = await nth(other.lineReads, 1); + expect(read.parentId).toBe('p1'); + expect(dataSource.find).toHaveBeenCalledTimes(1); + await answerLines(read, { id: 'l1', label: 'p1 line from the other adapter' }); + + await waitFor(() => expect(shownLines()).toEqual(['p1 line from the other adapter'])); + }); + + it('a change of CHILD OBJECT while dirty is read: the held rows are not that object’s', async () => { + const { dataSource, lineReads, view } = await mountP1Edited(); + + view.rerender(linesBlock({ childObject: 'po_fee' })); + const read = await nth(lineReads, 2); + expect(read.objectName).toBe('po_fee'); + expect(read.parentId).toBe('p1'); + await answerLines(read, { id: 'f1', label: 'p1 fee' }); + + await waitFor(() => expect(shownLines()).toEqual(['p1 fee'])); + expect(dataSource.find).toHaveBeenCalledTimes(2); + }); +}); diff --git a/packages/plugin-form/src/LineItemsPanel.heldForAnotherParent-10740.test.tsx b/packages/plugin-form/src/LineItemsPanel.heldForAnotherParent-10740.test.tsx index 5b57a3b691..41dda5b81c 100644 --- a/packages/plugin-form/src/LineItemsPanel.heldForAnotherParent-10740.test.tsx +++ b/packages/plugin-form/src/LineItemsPanel.heldForAnotherParent-10740.test.tsx @@ -33,7 +33,11 @@ * saves that parent's lines only; * - a RE-load of the SAME parent that fails (a transient failure on a refresh) * keeps the author's unsaved edits drawn, editable and saveable — under - * that parent — exactly as it did before this card. + * that parent — exactly as it did before this card. Since objectui#10712 + * the same-parent re-read that can run while edits are held is the + * post-save reload (a sort, limit or filter change is held while the + * panel has unsaved edits, pinned in `LineItemsPanel.dirtyReloadHold-10712`), + * so that row's trigger is a save whose reload fails. * * The owner is written wherever the rows are written. A grid offered with no * adapter (the load declines before any fetch) holds rows nobody loaded; a line @@ -322,11 +326,18 @@ describe('what the rule leaves alone (objectui#10740 controls)', () => { }); it('a RE-load of the SAME parent that fails keeps the held edits: the edited line is drawn editable, Save is on, and the save carries l1 under p1', async () => { - const { lineReads, saves, view } = await mountP1Edited(); - - // A load input other than the parent moves (the row cap), so the panel - // re-reads p1's lines while p1's edit is held; that read fails. - view.rerender(linesBlock({ limit: 200 })); + const { lineReads, saves } = await mountP1Edited(); + + // The same-parent re-read that can still run while p1's edit is held is + // the post-save reload: since objectui#10712 a change to a load input other + // than the parent (the row cap, say) is HELD while the panel has unsaved + // edits for the current parent, so it issues no read. The property this + // row keeps is unchanged: a same-parent re-read that fails leaves the + // author's lines drawn, editable and saveable under that parent. So Save + // is pressed, its batch lands, and the reload that follows fails. + await clickSave(); + const landed = await nth(saves, 1); + await settle(() => landed.resolve({ results: [] })); const reload = await nth(lineReads, 2); expect(reload.parentId).toBe('p1'); await fail(reload, 'p1 reload failed'); @@ -338,7 +349,7 @@ describe('what the rule leaves alone (objectui#10740 controls)', () => { expect(saveButton()?.disabled).toBe(false); await clickSave(); - const save = await nth(saves, 1); + const save = await nth(saves, 2); expect(save.ops).toEqual([ { object: 'po_line', action: 'update', id: 'l1', data: { label: 'p1 line edited', po: 'p1' } }, ]); diff --git a/packages/plugin-form/src/LineItemsPanel.supersededRun-10712.test.tsx b/packages/plugin-form/src/LineItemsPanel.supersededRun-10712.test.tsx index 785218d26e..621aa4f7e7 100644 --- a/packages/plugin-form/src/LineItemsPanel.supersededRun-10712.test.tsx +++ b/packages/plugin-form/src/LineItemsPanel.supersededRun-10712.test.tsx @@ -359,18 +359,18 @@ describe('LineItemsPanel’s save commits only to the parent it was issued for ( }); it('the reload after a save reads the panel’s CURRENT inputs, not the ones captured at the click', async () => { - const { lineReads, view, save } = await mountP1Saving(); + const { dataSource, lineReads, view, save } = await mountP1Saving(); // A load input other than the parent moves while the batch is in flight - // (the row cap); the panel re-reads p1 with it at once. + // (the row cap). The panel holds unsaved edits for p1, so the change is + // HELD (objectui#10712 R3, `LineItemsPanel.dirtyReloadHold-10712`): no read + // is issued for it, and the post-save reload is what carries it. view.rerender(linesBlock({ limit: 200 })); - const reload = await nth(lineReads, 2); - expect(reload.parentId).toBe('p1'); - expect(reload.top).toBe(200); - await answerLines(reload, { id: 'l1', label: 'p1 line' }); + await settle(() => {}); + expect(dataSource.find).toHaveBeenCalledTimes(1); await landSave(save); - const afterSave = await nth(lineReads, 3); + const afterSave = await nth(lineReads, 2); expect(afterSave.parentId).toBe('p1'); expect(afterSave.top, 'the save reloaded with the row cap captured at the click').toBe(200); diff --git a/packages/plugin-form/src/LineItemsPanel.tsx b/packages/plugin-form/src/LineItemsPanel.tsx index 9681ab8e6a..bbeb098240 100644 --- a/packages/plugin-form/src/LineItemsPanel.tsx +++ b/packages/plugin-form/src/LineItemsPanel.tsx @@ -220,6 +220,34 @@ export const LineItemsPanel: React.FC<{ schema: LineItemsPanelSchema }> = ({ sch const [rowsHeldFor, setRowsHeldFor] = useState(undefined); const heldForAnotherParent = rowsHeldFor !== undefined && rowsHeldFor !== parentId; + // objectui#10712 (R3) — the same rule the default form applies to its own + // background re-read (`ObjectForm.tsx`, `formDirtyRef` / `heldChangeRef`: + // unsaved input holds the re-read, and one is replayed when the form is + // pristine again or its save lands). A change to a load input other than the + // parent (sort, limit, filter) is HELD while this panel holds unsaved edits + // for the CURRENT parent: `dirty`, the rows held for `parentId`, and the last + // run's parent, adapter, child object and relationship field equal to the + // current ones. Before this card the re-read committed over the author's + // lines with no signal. What `load` reads to decide this lives in refs, so + // an edit does not move `load` and re-run the fetch effect. `heldEditsRef` + // mirrors `dirty` / `rowsHeldFor` from an effect declared ahead of `load`; + // `lastRunRef` is written where a run is numbered; `heldReloadRef` is the + // pending held change, and `heldReplay` replays it once when a same-parent + // read that was in flight at the hold commits (the fetch effect is keyed on + // it, and the panel is clean after that commit, so the replay is not held). + const heldEditsRef = useRef({ dirty: false, rowsHeldFor: undefined as string | undefined }); + useEffect(() => { + heldEditsRef.current = { dirty, rowsHeldFor }; + }, [dirty, rowsHeldFor]); + const lastRunRef = useRef<{ + parentId: string | null | undefined; + dataSource: unknown; + childObject: string | undefined; + relationshipField: string | undefined; + } | null>(null); + const heldReloadRef = useRef(false); + const [heldReplay, setHeldReplay] = useState(0); + // Child object schema — used to strip computed / read-only columns from each // row before persisting (parity with the parent form's sanitize). Rows are // loaded from a full read, so an edit would otherwise round-trip formula / @@ -306,11 +334,46 @@ export const LineItemsPanel: React.FC<{ schema: LineItemsPanelSchema }> = ({ sch [sortKey], ); - const load = useCallback(async () => { + /** + * `origin` says who asked for the read: `'inputs'` is the fetch effect (a + * load input moved, or a held change is replayed), `'save'` the post-save + * reload. Only an `'inputs'` read can be HELD (objectui#10712 R3, above): the + * post-save reload reads the panel's current inputs, which is what replays a + * held change, so it is never held itself. + */ + const load = useCallback(async (origin: 'inputs' | 'save' = 'save') => { + if (origin === 'inputs') { + const held = heldEditsRef.current; + const last = lastRunRef.current; + const editsHeldForCurrentParent = + held.dirty && held.rowsHeldFor !== undefined && held.rowsHeldFor === parentId; + const sameRowsAsLastRun = + last !== null + && last.parentId === parentId + && last.dataSource === dataSource + && last.childObject === schema.childObject + && last.relationshipField === schema.relationshipField; + if (editsHeldForCurrentParent && sameRowsAsLastRun) { + // Held, and NOT numbered: a held change is not a run, so it supersedes + // nothing. A same-parent read still in flight commits, and its commit + // replays this change (the panel is clean then). + heldReloadRef.current = true; + return; + } + } + // Every run reads the panel's current inputs, so whatever change was held + // is carried by this one. + heldReloadRef.current = false; // Numbered before any early return, so a run that declines still // supersedes one in flight. const seq = ++loadSeqRef.current; const isCurrent = () => loadSeqRef.current === seq; + lastRunRef.current = { + parentId, + dataSource, + childObject: schema.childObject, + relationshipField: schema.relationshipField, + }; if (!dataSource || !parentId) { setLoading(false); return; @@ -387,6 +450,13 @@ export const LineItemsPanel: React.FC<{ schema: LineItemsPanelSchema }> = ({ sch // and not a failed save, whose edits these rows replace (objectui#10682, // the objectui#10578 rule: cleared on a commit, never when a load starts). setError(null); + // A change held while THIS read was in flight is replayed once, now that + // the panel is clean (objectui#10712 R3): this read was issued before the + // change, so its answer is for the inputs it was issued with. + if (heldReloadRef.current) { + heldReloadRef.current = false; + setHeldReplay((n) => n + 1); + } } catch (e: any) { if (isCurrent()) { setError(e?.message || 'Failed to load line items'); @@ -425,9 +495,11 @@ export const LineItemsPanel: React.FC<{ schema: LineItemsPanelSchema }> = ({ sch latestLoadRef.current = { load, parentId }; }, [load, parentId]); + // A load input moved, or a held change is replayed (`heldReplay`): the read + // the panel's inputs ask for, which `load` may hold (objectui#10712 R3). useEffect(() => { - void load(); - }, [load]); + void load('inputs'); + }, [load, heldReplay]); const onChange = useCallback((next: Record[]) => { setRows(next); @@ -512,7 +584,7 @@ export const LineItemsPanel: React.FC<{ schema: LineItemsPanelSchema }> = ({ sch // directly (no $ref), so slicing off the parent op shifts no references. if (!canRollup) ops = ops.slice(1); if (ops.length) await runBatchTransaction(dataSource, ops); - if (parentStillShown()) await latestLoadRef.current.load(); + if (parentStillShown()) await latestLoadRef.current.load('save'); } catch (e: any) { if (parentStillShown()) setError(e?.message || 'Failed to save line items'); } finally { From 8451dc9709383f4f0ea31f53a96829c75e1af7e2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 12:58:45 +0000 Subject: [PATCH 3/4] fix(fields): `useRecordQuery` drops a pending debounced search on a sort or page change too (objectui#10712) The debounce cleanup effect was keyed on `runQuery`, whose identity does not move on a `sort` or `page` change, so a `setSearch` timer armed before such a change stayed live and ran last with the older sort or page: the records shown were unsorted while `sort` read `name asc`, or page 1 while `page` read 2. The cleanup is now keyed on what the fetch effect is keyed on: `runQuery`, `page` and `sort`. Two pin rows (sort-in-window, page-in-window), each red at the previous head on its `find` count. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_014mXUNuFomfj24w7s1pZzhN --- .../useRecordQuery.cannotQuery-10712.test.tsx | 49 +++++++++++++++++++ packages/fields/src/widgets/useRecordQuery.ts | 20 +++++--- 2 files changed, 62 insertions(+), 7 deletions(-) diff --git a/packages/fields/src/widgets/useRecordQuery.cannotQuery-10712.test.tsx b/packages/fields/src/widgets/useRecordQuery.cannotQuery-10712.test.tsx index 5b4215cf7f..edd73102b6 100644 --- a/packages/fields/src/widgets/useRecordQuery.cannotQuery-10712.test.tsx +++ b/packages/fields/src/widgets/useRecordQuery.cannotQuery-10712.test.tsx @@ -255,6 +255,55 @@ describe('useRecordQuery drops a pending debounced search when the query it was expect(result.current.loading).toBe(false); }); + it('a sort change inside the debounce window: the fetch effect carries the typed term with the new sort, and the armed timer runs no older query', async () => { + const { dataSource, calls } = makeDeferredDataSource(); + const { result } = renderHook(() => useRecordQuery({ dataSource, objectName: 'o', debounceMs: DEBOUNCE_MS })); + await answer(await nth(calls, 1), 'initial', 1); + + act(() => result.current.setSearch('a')); + // The sort changes before the debounce fires. `runQuery`'s identity does + // not move on a sort change, so the timer's closure is the current one; + // what it lacks is the sort, which the fetch effect already carried. + act(() => result.current.toggleSort('name')); + const current = await nth(calls, 2); + expect(current.params.$orderby).toEqual({ name: 'asc' }); + expect(current.params.$search).toBe('a'); + + await outwaitDebounce(); + + expect(dataSource.find, 'the debounced timer ran the query it was armed with, without the sort').toHaveBeenCalledTimes(2); + + await answer(current, 'sorted a', 1); + expect(names(result.current.records)).toEqual(['sorted a']); + expect(result.current.sort).toEqual({ field: 'name', direction: 'asc' }); + expect(result.current.loading).toBe(false); + }); + + it('a page change inside the debounce window (paginate): the fetch effect carries the typed term on the new page, and the armed timer runs no older query', async () => { + const { dataSource, calls } = makeDeferredDataSource(); + const { result } = renderHook(() => + useRecordQuery({ dataSource, objectName: 'o', paginate: true, pageSize: 10, debounceMs: DEBOUNCE_MS }), + ); + const first = await nth(calls, 1); + expect(first.params.$skip).toBe(0); + await answer(first, 'initial', 30); + + act(() => result.current.setSearch('a')); + act(() => result.current.setPage(2)); + const current = await nth(calls, 2); + expect(current.params.$skip).toBe(10); + expect(current.params.$search).toBe('a'); + + await outwaitDebounce(); + + expect(dataSource.find, 'the debounced timer ran the query it was armed with, for page 1').toHaveBeenCalledTimes(2); + + await answer(current, 'page 2 a', 30); + expect(names(result.current.records)).toEqual(['page 2 a']); + expect(result.current.page).toBe(2); + expect(result.current.loading).toBe(false); + }); + it('control: `reset()` inside the debounce window drops the pending search, as before', async () => { const { dataSource, calls } = makeDeferredDataSource(); const { result } = renderHook(() => useRecordQuery({ dataSource, objectName: 'o', debounceMs: DEBOUNCE_MS })); diff --git a/packages/fields/src/widgets/useRecordQuery.ts b/packages/fields/src/widgets/useRecordQuery.ts index d010cbf927..910e8d78a8 100644 --- a/packages/fields/src/widgets/useRecordQuery.ts +++ b/packages/fields/src/widgets/useRecordQuery.ts @@ -266,13 +266,19 @@ export function useRecordQuery(options: UseRecordQueryOptions): UseRecordQueryRe if (!canQuery) reset(); }, [canQuery, reset]); - // Drop a pending debounced search whenever `runQuery` changes, and on unmount - // (objectui#10712). The timer closes over the `runQuery` in scope when it was - // armed; a filter, object, data-source or page-size change inside the - // debounce window re-runs the query through the fetch effect above, with the - // current `search`, and the timer would then run the OLDER closure as a call - // numbered latest. - useEffect(() => () => clearDebounce(), [runQuery, clearDebounce]); + // Drop a pending debounced search whenever the fetch effect above is about to + // run a query, and on unmount (objectui#10712). Keyed on what that effect is + // keyed on: `runQuery` (a filter, object, data-source, page-size or `enabled` + // change moves it) AND `page` and `sort`, which move without moving + // `runQuery`. Any of these inside the debounce window re-runs the query + // through the fetch effect with the current `search`; the armed timer would + // then run as a call numbered latest, either the OLDER closure (the previous + // filter, object or data source) or the current closure with the OLDER page + // or sort (`setSearch` arms it with page 1 and the sort in scope), and its + // answer would replace the one the fetch effect asked for. React runs this + // cleanup before that effect's body in the same commit, so the timer is gone + // before the new query is numbered. + useEffect(() => () => clearDebounce(), [runQuery, page, sort, clearDebounce]); const setSearch = useCallback( (query: string) => { From ad6629e7043b9e4891856742b17a469795933072 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 13:09:40 +0000 Subject: [PATCH 4/4] test(fields): the R6 pin header names the sort and page triggers (objectui#10712) The pin's header described the debounce defect through a filter, object or data-source change only, and the fix as keyed on `runQuery`. It now names the sort and page triggers the two round-3 rows pin, and the fix as keyed on what the fetch effect is keyed on. Comment only. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_014mXUNuFomfj24w7s1pZzhN --- .../widgets/useRecordQuery.cannotQuery-10712.test.tsx | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/fields/src/widgets/useRecordQuery.cannotQuery-10712.test.tsx b/packages/fields/src/widgets/useRecordQuery.cannotQuery-10712.test.tsx index edd73102b6..9ed9c2f995 100644 --- a/packages/fields/src/widgets/useRecordQuery.cannotQuery-10712.test.tsx +++ b/packages/fields/src/widgets/useRecordQuery.cannotQuery-10712.test.tsx @@ -25,14 +25,17 @@ * it was armed. A filter, object or data-source change inside the debounce * window re-ran the query through the fetch effect (with the typed term), * and the timer then ran the OLDER closure — the previous filter — as a - * call numbered latest, so its answer won. + * call numbered latest, so its answer won. A sort or page change inside + * the window did the same without moving `runQuery`: `setSearch` arms the + * timer with page 1 and the sort in scope, so it ran the older sort or + * page. * * Now the reset is keyed on the hook's ability to query (`enabled`, the data * source and the object together): it bumps the run number, ends `loading`, * clears a pending debounced search and clears the query state; `reset()` - * does the same. And a pending debounced search is dropped whenever `runQuery` - * changes, since the fetch effect issues the current query with the typed term - * then. + * does the same. And a pending debounced search is dropped whenever the fetch + * effect is about to run a query (`runQuery`, `page` or `sort` moves), since + * that query carries the typed term. * * Every `find` returns a promise the test settles by hand, so each ordering * below is the ordering the answers really land in. The one timer here is the