diff --git a/.changeset/settled-schema-resolution-hook-6482.md b/.changeset/settled-schema-resolution-hook-6482.md new file mode 100644 index 0000000000..7f3f4345fe --- /dev/null +++ b/.changeset/settled-schema-resolution-hook-6482.md @@ -0,0 +1,18 @@ +--- +'@object-ui/react': minor +--- + +New export: `useSettledSchema` — the settled-schema RESOLUTION half shared by +`ObjectKanban` / `ObjectView` / `ObjectCalendar`'s fetch-gate hand copies +(objectui#6482, maintainer ruling Option A). It tracks whether an object's +definition has finished resolving FOR THE KEY THE CURRENT RENDER IS ASKING +ABOUT, returning `{ ready, def }` from one piece of internal state so `ready` +and `def` can never be observed inconsistently and a stale key can never read +as ready — the structural fix for the `ObjectTree` defect (objectui#6481) +where a definition and a separate, one-way-latched "settled" boolean could +disagree for a render after the object changed. + +Gate PLACEMENT — which effect branch actually waits on `ready` — stays a +per-component decision and is not part of this hook; see the hook's own doc +comment. Existing hand copies are migrated on their own subsequent cards, not +by this change. diff --git a/packages/react/README.md b/packages/react/README.md index ed324471ca..83084505ff 100644 --- a/packages/react/README.md +++ b/packages/react/README.md @@ -158,6 +158,34 @@ falling back to the object's full scope. Use `useElementDataSourceSchema` (plus the exported `ElementDataSourceErrorPanel` / `ElementDataSourceLoadingPanel`) when a block cannot be wrapped — a renderer whose hooks must run before the panels. +### useSettledSchema + +The settled-schema RESOLUTION half shared by `ObjectKanban` / `ObjectView` / +`ObjectCalendar`'s fetch-gate hand copies (objectui#6482). Tracks whether an +object's definition has finished resolving FOR THE KEY THE CURRENT RENDER IS +ASKING ABOUT — `ready` and `def` are two views of one piece of state, so a +stale key can never read as ready. GATE PLACEMENT — which effect actually +waits on `ready` — stays a per-component decision; this hook only owns the +resolution. + +```tsx +import { useSettledSchema } from '@object-ui/react' + +function ObjectSomething({ schema, dataSource }) { + const key = schema.objectName ?? '' + const { ready, def } = useSettledSchema(key, dataSource) + + useEffect(() => { + if (!ready) return // gate placement is local to this component + // issue the record query, e.g. buildExpandFields(def?.fields) + }, [ready, def]) +} +``` + +Pass `dataSource: undefined` for a render that should settle immediately with +no definition (e.g. a provider that issues no metadata read at all) instead of +adding a separate enable flag. + ### ComponentRegistry There is no registry hook: the registry is a process-level singleton exported diff --git a/packages/react/src/hooks/__tests__/useSettledSchema.test.ts b/packages/react/src/hooks/__tests__/useSettledSchema.test.ts new file mode 100644 index 0000000000..758fb016c7 --- /dev/null +++ b/packages/react/src/hooks/__tests__/useSettledSchema.test.ts @@ -0,0 +1,220 @@ +/** + * ObjectUI — useSettledSchema Tests + * Copyright (c) 2024-present ObjectStack Inc. + * + * objectui#6482: the settled-schema RESOLUTION half shared across + * ObjectKanban / ObjectView / ObjectCalendar's hand copies, extracted so the + * shape those three already got right (and objectui#6014/#6481's ObjectTree + * did not) is structural rather than conventional. + * + * The acceptance bar this file exists to demonstrate (per the maintainer + * ruling): `ready` and `def` cannot be observed inconsistently, and a STALE + * key can never read as ready — the exact defect objectui#6481 shipped by + * carrying "settled" as a second, independent boolean. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { renderHook, act, waitFor } from '@testing-library/react'; +import { useSettledSchema } from '../useSettledSchema'; + +/** A deferred promise, so a test controls exactly when a fetch resolves. */ +function deferred() { + let resolve!: (value: T) => void; + let reject!: (err: unknown) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +} + +describe('useSettledSchema', () => { + it('settles immediately with ready=true, def=null when dataSource is undefined', async () => { + const { result } = renderHook(() => useSettledSchema('accounts', undefined)); + + await waitFor(() => expect(result.current.ready).toBe(true)); + expect(result.current.def).toBeNull(); + }); + + it('settles immediately with ready=true, def=null when the key is empty', async () => { + const ds: any = { getObjectSchema: vi.fn().mockResolvedValue({ fields: {} }) }; + const { result } = renderHook(() => useSettledSchema('', ds)); + + await waitFor(() => expect(result.current.ready).toBe(true)); + expect(result.current.def).toBeNull(); + expect(ds.getObjectSchema).not.toHaveBeenCalled(); + }); + + it('settles immediately with ready=true, def=null when dataSource has no getObjectSchema', async () => { + const ds: any = { find: vi.fn() }; + const { result } = renderHook(() => useSettledSchema('accounts', ds)); + + await waitFor(() => expect(result.current.ready).toBe(true)); + expect(result.current.def).toBeNull(); + }); + + it('is NOT ready while the fetch is in flight, then resolves with the definition', async () => { + const d = deferred(); + const ds: any = { getObjectSchema: vi.fn().mockReturnValue(d.promise) }; + + const { result } = renderHook(() => useSettledSchema('accounts', ds)); + + // Not ready yet — the fetch is still pending. + expect(result.current.ready).toBe(false); + expect(result.current.def).toBeNull(); + + await act(async () => { + d.resolve({ fields: { name: { type: 'text' } } }); + await d.promise; + }); + + await waitFor(() => expect(result.current.ready).toBe(true)); + expect(result.current.def).toEqual({ fields: { name: { type: 'text' } } }); + expect(ds.getObjectSchema).toHaveBeenCalledWith('accounts'); + }); + + it('settles with def=null (ready=true) when the fetch throws — "settled with nothing" is not "not ready"', async () => { + const d = deferred(); + const ds: any = { getObjectSchema: vi.fn().mockReturnValue(d.promise) }; + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}); + + const { result } = renderHook(() => useSettledSchema('accounts', ds)); + expect(result.current.ready).toBe(false); + + await act(async () => { + d.reject(new Error('boom')); + await d.promise.catch(() => {}); + }); + + await waitFor(() => expect(result.current.ready).toBe(true)); + expect(result.current.def).toBeNull(); + consoleError.mockRestore(); + }); + + // --------------------------------------------------------------------- + // The acceptance bar: a stale key can never read as ready. + // --------------------------------------------------------------------- + + it('objectui#6481 unwritable: switching keys while a fetch is in flight reads NOT ready in the SAME render — never the old key\'s def', async () => { + const accountsFetch = deferred(); + const contactsFetch = deferred(); + const ds: any = { + getObjectSchema: vi.fn((key: string) => + key === 'accounts' ? accountsFetch.promise : contactsFetch.promise, + ), + }; + + const { result, rerender } = renderHook( + ({ key }) => useSettledSchema(key, ds), + { initialProps: { key: 'accounts' } }, + ); + + // Settle the FIRST key while it is still current. + await act(async () => { + accountsFetch.resolve({ fields: { accountName: {} } }); + await accountsFetch.promise; + }); + await waitFor(() => expect(result.current.ready).toBe(true)); + expect(result.current.def).toEqual({ fields: { accountName: {} } }); + + // Switch keys. The new key's fetch has NOT resolved yet — this is the + // exact window objectui#6481's bare `schemaSettled` boolean got wrong: + // it stayed `true` from the 'accounts' settle, so a gated effect reading + // it would see "ready" while `objectSchema` still held ACCOUNTS' fields, + // for a key that is now CONTACTS. + rerender({ key: 'contacts' }); + + // Must read as NOT ready — synchronously, in the render right after the + // key changed, with no need for the new fetch to complete first — and + // `def` must NOT be leaking the previous ('accounts') definition. + expect(result.current.ready).toBe(false); + expect(result.current.def).toBeNull(); + + // Now settle the new key. `ready` flips true again, keyed to 'contacts'. + await act(async () => { + contactsFetch.resolve({ fields: { contactName: {} } }); + await contactsFetch.promise; + }); + await waitFor(() => expect(result.current.ready).toBe(true)); + expect(result.current.def).toEqual({ fields: { contactName: {} } }); + }); + + it('a late resolution for an ABANDONED key never lands — the current key\'s resolution is never clobbered by a stale one arriving out of order', async () => { + const accountsFetch = deferred(); + const contactsFetch = deferred(); + const ds: any = { + getObjectSchema: vi.fn((key: string) => + key === 'accounts' ? accountsFetch.promise : contactsFetch.promise, + ), + }; + + const { result, rerender } = renderHook( + ({ key }) => useSettledSchema(key, ds), + { initialProps: { key: 'accounts' } }, + ); + + // Switch away from 'accounts' before its fetch ever resolves. + rerender({ key: 'contacts' }); + expect(result.current.ready).toBe(false); + + // Settle 'contacts' FIRST. + await act(async () => { + contactsFetch.resolve({ fields: { contactName: {} } }); + await contactsFetch.promise; + }); + await waitFor(() => expect(result.current.ready).toBe(true)); + expect(result.current.def).toEqual({ fields: { contactName: {} } }); + + // The ABANDONED 'accounts' fetch now resolves, late. Its effect's + // cleanup already ran (key changed), so its `isMounted` closure is + // false — this write must be dropped, not overwrite 'contacts'. + await act(async () => { + accountsFetch.resolve({ fields: { accountName: {} } }); + await accountsFetch.promise.catch(() => {}); + }); + + expect(result.current.ready).toBe(true); + expect(result.current.def).toEqual({ fields: { contactName: {} } }); + }); + + it('does not call setState after unmount when a fetch resolves late', async () => { + const d = deferred(); + const ds: any = { getObjectSchema: vi.fn().mockReturnValue(d.promise) }; + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}); + + const { unmount } = renderHook(() => useSettledSchema('accounts', ds)); + unmount(); + + await act(async () => { + d.resolve({ fields: {} }); + await d.promise; + }); + + // No React "state update on an unmounted component" warning. + const reactWarning = consoleError.mock.calls.some((args) => + String(args[0] ?? '').includes('unmounted'), + ); + expect(reactWarning).toBe(false); + consoleError.mockRestore(); + }); + + it('re-fetches when dataSource identity changes even if the key does not', async () => { + const dsA: any = { getObjectSchema: vi.fn().mockResolvedValue({ fields: { a: {} } }) }; + const dsB: any = { getObjectSchema: vi.fn().mockResolvedValue({ fields: { b: {} } }) }; + + const { result, rerender } = renderHook( + ({ ds }) => useSettledSchema('accounts', ds), + { initialProps: { ds: dsA } }, + ); + + await waitFor(() => expect(result.current.ready).toBe(true)); + expect(result.current.def).toEqual({ fields: { a: {} } }); + + rerender({ ds: dsB }); + // Ready flips false the instant the source changes, same mechanism as a + // key change — there is only ever one comparison, `resolution.key === key`, + // gated additionally by the effect re-running on a NEW `dataSource`. + await waitFor(() => expect(result.current.def).toEqual({ fields: { b: {} } })); + expect(dsB.getObjectSchema).toHaveBeenCalledWith('accounts'); + }); +}); diff --git a/packages/react/src/hooks/index.ts b/packages/react/src/hooks/index.ts index b5dc626761..e6d52b85a8 100644 --- a/packages/react/src/hooks/index.ts +++ b/packages/react/src/hooks/index.ts @@ -49,3 +49,7 @@ export * from './useDatasetDimensionLabels.js'; export * from './useDataRefresh.js'; export * from './usePageAssignment.js'; export * from './useRecordSearch.js'; +// The settled-schema RESOLUTION half shared across ObjectKanban / ObjectView / +// ObjectCalendar's hand copies (objectui#6482, maintainer ruling Option A). +// Gate PLACEMENT stays per-component — see the hook's own doc comment. +export * from './useSettledSchema.js'; diff --git a/packages/react/src/hooks/useSettledSchema.ts b/packages/react/src/hooks/useSettledSchema.ts new file mode 100644 index 0000000000..1d89c21a1f --- /dev/null +++ b/packages/react/src/hooks/useSettledSchema.ts @@ -0,0 +1,158 @@ +/** + * 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. + */ + +import { useEffect, useState } from 'react'; +import type { DataSource } from '@object-ui/types'; + +/** + * The settled result of `{@link useSettledSchema}`: whether the object + * definition for the CURRENT key has finished resolving, and the definition + * itself once it has. + * + * `ready` is never a fact about "a fetch completed" in isolation — it is + * "a fetch completed FOR THE OBJECT THIS RENDER IS ASKING ABOUT". See the + * hook's own doc comment for why that distinction is the entire point. + */ +export interface SettledSchema { + /** + * `true` once the object definition for the key passed to this render has + * settled — successfully, with a thrown read, or because there was no + * source to read from. `false` while it is still in flight, and also + * `false` for exactly one render right after `key` changes, even if a + * PREVIOUS key's resolution is sitting in state (objectui#6481's defect, + * made unwritable — see the hook doc comment). + */ + ready: boolean; + /** + * The resolved definition once `ready` is `true`, otherwise `null`. + * `null` while `ready` is `true` is a legitimate, DISTINCT outcome from + * "not ready yet" — it means the resolution settled with nothing (no + * `dataSource`, no `getObjectSchema`, no key, or a read that threw), not + * that the read never happened. + */ + def: TDef | null; +} + +/** + * Resolves an object's schema/definition and tracks whether that resolution + * has SETTLED for the object CURRENTLY being asked about — one piece of + * state a caller cannot observe half of. + * + * ## The shape, and why it is exactly this one + * + * This is the RESOLUTION half of the settled-schema gate four views + * (`ObjectKanban`, `ObjectView`, `ObjectCalendar`, and — pre-fix — + * `ObjectTree`) each hand-wrote (objectui#6271, #6419, #6453, #6014). Ruled + * objectui#6482 (maintainer, 2026-08-27, Option A): extract the resolution + * half as a shared, published hook; leave GATE PLACEMENT — deciding which + * effect branch actually waits on `ready` — to each component, because that + * part is genuinely component-private (`ObjectCalendar` gates only its + * `object`-provider branch and keys on `dataConfig.object ?? schema.objectName` + * rather than `schema.objectName`, because an inline `value` data set issues + * no metadata read at all — a whole-effect gate would hold its query open on + * a resolution nothing was ever going to produce). + * + * `ready` is DERIVED at render time — `resolution !== null && resolution.key + * === key` — from a SINGLE piece of state, `{ key, def } | null`, rather than + * stored as a second, independent boolean. That is not a style choice; it is + * the fix for the defect this card exists to close. objectui#6481's + * `ObjectTree` carried the definition (`objectSchema`) and "has it settled" + * (`schemaSettled`) as two SEPARATE `useState`s. `schemaSettled` was a + * one-way latch — set `true` on first settle and never reset — so when the + * host swapped `objectName` mid-life, the fetch effect re-ran and started + * refetching the NEW object's schema, but `schemaSettled` stayed `true` from + * the OLD object's settle. The gated effect read `schemaSettled === true` + * and `objectSchema` still holding the OLD object's fields, and queried with + * the WRONG `$expand` — a stale key reading as ready. + * + * With one state value and a render-time key comparison, that failure mode + * is not merely fixed, it is UNREPRESENTABLE: the instant `key` changes, + * `resolution.key === key` is false in that very render (no effect needs to + * run first), so `ready` flips to `false` in the same commit the key + * changed — there is no window, and no second piece of state a caller could + * read out of sync with the first, because there is no second piece of + * state. A caller cannot spell "ready for the wrong object": `ready` and + * `def` are two views of one value, never independently settable. + * + * Every exit of the internal fetch effect settles the resolution — success, + * a thrown read, and "there is no source to read from" alike — because a + * caller's gated effect WAITS on `ready`. An exit that returned without + * settling would not merely skip the expansion; it would hold that gated + * query open forever. + * + * ## What this hook does NOT do + * + * It does not decide when to fetch beyond "when `key` or `dataSource` + * change", and it does not decide what a caller's OTHER effects should wait + * on. A caller that should not fetch at all for the current render (e.g. + * `ObjectCalendar`'s inline `value` provider, which issues no metadata read) + * passes `dataSource: undefined` for that render rather than a new "should + * fetch" flag — the hook already settles-with-`null` whenever `dataSource` + * is absent, which is the exact "no source to read from" outcome that case + * needs. + * + * @param key - The identity of the object THIS render is asking about (e.g. + * `dataConfig.object ?? schema.objectName ?? ''`). Computing the right key + * is the caller's job — it is the component-private half of the original + * hand copies, not something this hook can infer. + * @param dataSource - The data source to read the definition from. Pass + * `undefined`/`null` for a render that should settle immediately with no + * definition (no source, or a provider that needs none) rather than adding + * a separate enable flag. + * @returns `{ ready, def }` — see {@link SettledSchema}. + * + * @example + * ```tsx + * const schemaKey = dataConfig?.provider === 'object' ? dataConfig.object : schema.objectName; + * const { ready: objectSchemaReady, def: objectSchema } = + * useSettledSchema(schemaKey ?? '', hasInlineData ? undefined : dataSource); + * + * useEffect(() => { + * if (dataConfig?.provider === 'object' && !objectSchemaReady) return; // gate placement stays local + * // ...issue the record query, `buildExpandFields(objectSchema?.fields)` + * }, [objectSchemaReady, objectSchema, /* ... *\/]); + * ``` + */ +export function useSettledSchema( + key: string, + dataSource: DataSource | null | undefined, +): SettledSchema { + const [resolution, setResolution] = useState<{ key: string; def: TDef | null } | null>(null); + + useEffect(() => { + let isMounted = true; + const settleKey = key; + + const resolve = async () => { + if (!dataSource || !settleKey || typeof dataSource.getObjectSchema !== 'function') { + // No source for a definition: settle with none, so anything gated on + // `ready` still runs (unexpanded — with no schema there is no expand + // set to derive). + if (isMounted) setResolution({ key: settleKey, def: null }); + return; + } + try { + const def = await dataSource.getObjectSchema(settleKey); + if (isMounted) setResolution({ key: settleKey, def: def as TDef }); + } catch (err) { + console.error('[useSettledSchema] getObjectSchema failed for', settleKey, err); + if (isMounted) setResolution({ key: settleKey, def: null }); + } + }; + + resolve(); + return () => { + isMounted = false; + }; + }, [key, dataSource]); + + const ready = resolution !== null && resolution.key === key; + const def = ready ? (resolution as { key: string; def: TDef | null }).def : null; + + return { ready, def }; +} diff --git a/vitest.config.mts b/vitest.config.mts index b8dce7865b..6151f6f3f6 100644 --- a/vitest.config.mts +++ b/vitest.config.mts @@ -71,6 +71,7 @@ const domTsTests = [ 'packages/react/src/hooks/__tests__/useDataRefresh.test.ts', 'packages/react/src/hooks/__tests__/useExpression.test.ts', 'packages/react/src/hooks/__tests__/useRecordSearch.test.ts', + 'packages/react/src/hooks/__tests__/useSettledSchema.test.ts', ]; // Test files that render through the ComponentRegistry and therefore need the