diff --git a/packages/bindx-react/src/hooks/useEntity.ts b/packages/bindx-react/src/hooks/useEntity.ts index 5331ef2..bc1af43 100644 --- a/packages/bindx-react/src/hooks/useEntity.ts +++ b/packages/bindx-react/src/hooks/useEntity.ts @@ -323,9 +323,13 @@ export function useEntity( }, [entityType, id, byKey, effectiveQueryKey, options.cache, batcher, store, dispatcher, selectionMeta]) // --- EntityHandle --- + // The handle keeps a stable identity across data changes — it is a stateless live view over the + // store (see BaseHandle), so `snapshot` is intentionally NOT a dependency here. The host already + // re-renders on data changes via useStoreSubscription above (keeping inline `accessor.value` + // reads fresh), and memoized children re-render through their own subscription (useField/useAccessor). const rawHandle = useMemo( () => EntityHandle.createRaw(id, entityType, store, dispatcher, schemaRegistry as SchemaRegistry>), - [id, entityType, store, dispatcher, schemaRegistry, snapshot], + [id, entityType, store, dispatcher, schemaRegistry], ) const handle = useMemo( @@ -333,11 +337,6 @@ export function useEntity( [rawHandle], ) - // `snapshot` is a useMemo dep, so `rawHandle` is recreated on every entity data change (to give - // memoized children a fresh reference). Superseded handles need no cleanup: EntityHandle is a - // stateless live view that owns no resources (see BaseHandle), so it is reclaimed by GC once - // unreferenced and a handle a consumer still holds stays usable for late reads/writes. - // --- Persist & reset callbacks --- const persist = useCallback(async () => { await batchPersister.persist(entityType, id) @@ -347,6 +346,33 @@ export function useEntity( rawHandle.reset() }, [rawHandle]) + // Keep the public ready proxy stable while serving fresh metadata from each render. + const readyMetaRef = useRef({ + $status: 'ready', + $isLoading: false, + $isRefetching: false, + $isError: false, + $isNotFound: false, + $error: null, + $persist: persist, + $reset: reset, + }) + readyMetaRef.current = { + $status: 'ready', + $isLoading: false, + $isRefetching: isRefetching, + $isError: false, + $isNotFound: false, + $error: null, + $persist: persist, + $reset: reset, + } + + const readyResult = useMemo( + () => createReadyResult(handle, readyMetaRef), + [handle], + ) + // --- Build result --- const result = useMemo((): UseEntityResult => { if (!loadState || loadState.status === 'loading' || (!snapshot && loadState.status === 'success')) { @@ -362,8 +388,8 @@ export function useEntity( } // Ready — layer status metadata on top of EntityHandle proxy - return createReadyResult(handle, persist, reset, isRefetching) - }, [snapshot, loadState, isPersisting, id, handle, persist, reset, isRefetching]) + return readyResult + }, [snapshot, loadState, isPersisting, id, persist, reset, readyResult]) // Proxy-based result satisfies the full UseEntityResult at runtime via field access delegation return result as UseEntityResult @@ -373,32 +399,32 @@ export function useEntity( // Result factories // ============================================================================ +interface ReadyResultMeta extends Record { + readonly $status: 'ready' + readonly $isLoading: false + readonly $isRefetching: boolean + readonly $isError: false + readonly $isNotFound: false + readonly $error: null + $persist(): Promise + $reset(): void +} + function createReadyResult( handle: EntityAccessor, - persist: () => Promise, - reset: () => void, - isRefetching: boolean, + metaRef: { readonly current: ReadyResultMeta }, ): UseEntityResult { - const meta: Record = { - $status: 'ready' as const, - $isLoading: false as const, - $isRefetching: isRefetching, - $isError: false as const, - $isNotFound: false as const, - $error: null, - $persist: persist, - $reset: reset, - } - // Proxy merges status metadata onto EntityAccessor — satisfies ReadyEntityResult at runtime return new Proxy(handle as object, { get(target, prop, receiver) { + const meta = metaRef.current if (typeof prop === 'string' && prop in meta) { return meta[prop] } return Reflect.get(target, prop, receiver) }, has(target, prop) { + const meta = metaRef.current if (typeof prop === 'string' && prop in meta) { return true } diff --git a/packages/bindx-react/src/jsx/components/Entity.tsx b/packages/bindx-react/src/jsx/components/Entity.tsx index acdee09..a334d25 100644 --- a/packages/bindx-react/src/jsx/components/Entity.tsx +++ b/packages/bindx-react/src/jsx/components/Entity.tsx @@ -285,9 +285,13 @@ interface EntityHandleRendererProps { * Shared component that creates an EntityHandle and renders children with it. * Used by both EntityByMode and EntityCreateMode. * - * Subscribes to entity snapshot changes so that the handle reference changes - * when entity data changes. This is necessary because children may be wrapped - * in React.memo and need a new handle reference to trigger re-renders. + * Subscribes to the entity snapshot so this host re-renders on data changes, + * which keeps inline reads in `children` fresh. The EntityHandle itself has a + * stable identity — it is intentionally NOT recreated on data changes — because + * it is a stateless live view over the store (see BaseHandle). Memoized leaf + * components (, , ) re-render through their own store + * subscription (useField/useAccessor), not through a changing handle reference, + * so a data change re-renders only the leaves that read the changed data. */ function EntityHandleRenderer({ entityId, @@ -308,11 +312,14 @@ function EntityHandleRenderer({ [store, entityType, entityId], ) - const version = useSyncExternalStore(subscribe, getSnapshot, getSnapshot) + // Subscribe so this host re-renders on data changes (keeps inline reads in `children` fresh). + // The version is deliberately NOT fed into the handle memo below: the handle is a stable live + // view and must keep a stable identity across data changes. + useSyncExternalStore(subscribe, getSnapshot, getSnapshot) const rawHandle = useMemo( () => EntityHandle.createRaw(entityId, entityType, store, dispatcher, schemaRegistry, undefined, selection), - [entityId, entityType, store, dispatcher, schemaRegistry, selection, version], + [entityId, entityType, store, dispatcher, schemaRegistry, selection], ) const handle = useMemo( @@ -320,13 +327,6 @@ function EntityHandleRenderer({ [rawHandle], ) - // `version` is a useMemo dep, so `rawHandle` is recreated on every entity data change (to give - // memoized children a fresh reference). Superseded handles need no cleanup: EntityHandle and its - // child Field/HasMany handles are stateless live views that own no resources (see BaseHandle), - // so they are reclaimed by GC once unreferenced — and a consumer that still holds an accessor - // from an earlier render (e.g. the Slate block editor dispatching `setValue` from an async - // onChange after the version bumped) can keep using it. - const result = children(handle as EntityAccessor) return <>{annotateElement(result, { 'data-entity': entityType, 'data-entity-id': entityId })} } diff --git a/tests/react/jsx/stableHandleIdentity.test.tsx b/tests/react/jsx/stableHandleIdentity.test.tsx new file mode 100644 index 0000000..ef0bbf5 --- /dev/null +++ b/tests/react/jsx/stableHandleIdentity.test.tsx @@ -0,0 +1,211 @@ +import '../../setup' +import { describe, test, expect, afterEach } from 'bun:test' +import { render, waitFor, act, fireEvent, cleanup } from '@testing-library/react' +import React, { memo, useState } from 'react' +import { BindxProvider, MockAdapter, Entity, Field, useField, useEntity } from '@contember/bindx-react' +import type { EntityRef, FieldRef, FieldAccessor } from '@contember/bindx' +import type { Article } from '../../shared' +import { getByTestId, queryByTestId, createMockData, schema, testSchema } from '../../shared' + +afterEach(() => { + cleanup() +}) + +/** + * EntityHandle is a stateless live view over the store, so it keeps a *stable identity* + * across data changes — it is no longer recreated on every snapshot bump. The host still + * re-renders (so inline reads stay fresh), but reactivity for memoized leaves now flows + * through their own store subscription (/useField), not through a churning handle + * reference. These tests lock in both halves of that contract. + */ +describe(' handle identity across data-driven re-render', () => { + test('hands children the same accessor instance even though the host re-renders', async () => { + const adapter = new MockAdapter(createMockData(), { delay: 0 }) + const seen: FieldAccessor[] = [] + let bump: (() => void) | null = null + + // Passing the field ref down as a prop makes `article.title` collectable during the + // selection phase (a component body, where useField runs, is not executed then). + function Capture({ title: titleRef }: { title: FieldRef }): React.ReactElement { + // useAccessor returns the same ref it was given, so this is the field accessor identity. + const title = useField(titleRef) + seen.push(title) + bump = () => title.setValue(`${title.value}!`) + return {title.value} + } + + const { container } = render( + + + {article => } + + , + ) + + await waitFor(() => { + expect(queryByTestId(container, 'title')).not.toBeNull() + }) + + const rendersBefore = seen.length + act(() => { + bump!() + }) + + // The host re-rendered on the data change (inline reads stay fresh) ... + expect(getByTestId(container, 'title').textContent).toBe('Hello World!') + expect(seen.length).toBeGreaterThan(rendersBefore) + // ... yet every render received the exact same accessor instance (stable identity). + expect(new Set(seen).size).toBe(1) + }) + + test('a memoized child holding the accessor does not re-render on a data change, while stays live', async () => { + const adapter = new MockAdapter(createMockData(), { delay: 0 }) + let memoRenders = 0 + let bump: (() => void) | null = null + + const StaticChild = memo(function StaticChild(_props: { article: EntityRef
}): React.ReactElement { + memoRenders++ + return static + }) + + function Controls({ article }: { article: EntityRef
}): null { + const title = useField(article.title) + bump = () => title.setValue(`${title.value}!`) + return null + } + + const { container } = render( + + + {article => ( + <> + + + + + )} + + , + ) + + await waitFor(() => { + expect(queryByTestId(container, 'static')).not.toBeNull() + }) + + const memoRendersBefore = memoRenders + act(() => { + bump!() + }) + + // re-rendered from its own subscription — fine-grained reactivity preserved. + expect(getByTestId(container, 'field-value').textContent).toBe('Hello World!') + // The memoized child received a stable accessor prop — React.memo bailed, no re-render. + expect(memoRenders).toBe(memoRendersBefore) + }) +}) + +describe('useEntity handle identity across data-driven re-render', () => { + test('the returned ready accessor and field accessor keep stable identities', async () => { + const adapter = new MockAdapter(createMockData(), { delay: 0 }) + const seenArticles: object[] = [] + const seenTitle: FieldAccessor[] = [] + + function Probe(): React.ReactElement { + const article = useEntity(schema.Article, { by: { id: 'article-1' } }, e => e.title()) + if (article.$status !== 'ready') { + return loading + } + seenArticles.push(article) + seenTitle.push(article.title) + return ( + + ) + } + + const { container } = render( + + + , + ) + + await waitFor(() => { + expect(queryByTestId(container, 'title')).not.toBeNull() + }) + + const rendersBefore = seenTitle.length + act(() => { + fireEvent.click(getByTestId(container, 'title')) + }) + + expect(getByTestId(container, 'title').textContent).toBe('Hello World!') + // Re-rendered on the data change ... + expect(seenTitle.length).toBeGreaterThan(rendersBefore) + // The top-level ready accessor is also the same proxy, not just article.title. + expect(new Set(seenArticles).size).toBe(1) + // ... but the field accessor was the same instance every time (handle cache survived). + expect(new Set(seenTitle).size).toBe(1) + }) + + test('the stable ready accessor exposes fresh refetch metadata', async () => { + const mockData = createMockData() + const adapter = new MockAdapter(mockData, { delay: 20 }) + const seenArticles: object[] = [] + const seenRefetching: boolean[] = [] + let setKey: (key: string) => void = () => {} + + function Probe(): React.ReactElement { + const [key, setLocalKey] = useState('v1') + setKey = setLocalKey + + const article = useEntity( + schema.Article, + { by: { id: 'article-1' }, queryKey: key }, + e => e.title(), + ) + if (article.$status !== 'ready') { + return loading + } + + seenArticles.push(article) + seenRefetching.push(article.$isRefetching) + return ( +
+ {article.title.value} + {article.$isRefetching ? 'yes' : 'no'} +
+ ) + } + + const { container } = render( + + + , + ) + + await waitFor(() => { + expect(queryByTestId(container, 'title')).not.toBeNull() + }) + const firstReadyArticle = seenArticles[seenArticles.length - 1] + expect(getByTestId(container, 'refetching').textContent).toBe('no') + + mockData.Article['article-1']!.title = 'Updated By RPC' + act(() => { + setKey('v2') + }) + + expect(getByTestId(container, 'refetching').textContent).toBe('yes') + expect(seenArticles[seenArticles.length - 1]).toBe(firstReadyArticle) + + await waitFor(() => { + expect(getByTestId(container, 'title').textContent).toBe('Updated By RPC') + }) + expect(getByTestId(container, 'refetching').textContent).toBe('no') + expect(new Set(seenArticles).size).toBe(1) + expect(seenRefetching).toContain(true) + }) +})