From 2829aea54532ef1e7d2c38b05b227b7cc6166a54 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Thu, 25 Jun 2026 12:09:20 +0200 Subject: [PATCH 1/2] refactor(bindx-react): give EntityHandle a stable identity across data changes EntityHandle is a stateless live view over the store, yet `useEntity` and `` recreated it on every snapshot/version change (by feeding `snapshot`/`version` into the handle's useMemo deps) purely to hand memoized children a fresh reference. That defeated the store's fine-grained reactivity: any field change threw away the whole handle subtree and its caches, re-rendered the entire entity subtree, churned `useEffect([handle])` deps, and created the "superseded handle" concept the (now-removed) dispose lifecycle existed to clean up. The blessed leaf components (``, ``, ``) already subscribe to the store themselves (useField/useAccessor -> useSyncExternalStore), so they re-render on data changes without needing a changing handle reference. Drop `snapshot`/`version` from the handle memo deps so the handle keeps a stable identity. The host still subscribes and re-renders (keeping inline `.value` reads in `children` fresh); memoized leaves now re-render only via their own subscription, so a field edit re-renders just the leaves that read it. Adds regression tests: stable accessor identity across a data-driven re-render (both `` and `useEntity` paths), a memoized child holding the accessor no longer re-rendering on an unrelated data change, and `` staying live. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01EAKx7JbV7Z7EVXofqRgwop --- packages/bindx-react/src/hooks/useEntity.ts | 11 +- .../bindx-react/src/jsx/components/Entity.tsx | 24 +-- tests/react/jsx/stableHandleIdentity.test.tsx | 149 ++++++++++++++++++ 3 files changed, 166 insertions(+), 18 deletions(-) create mode 100644 tests/react/jsx/stableHandleIdentity.test.tsx diff --git a/packages/bindx-react/src/hooks/useEntity.ts b/packages/bindx-react/src/hooks/useEntity.ts index 5331ef2..4e58da3 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) 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..c05b51f --- /dev/null +++ b/tests/react/jsx/stableHandleIdentity.test.tsx @@ -0,0 +1,149 @@ +import '../../setup' +import { describe, test, expect, afterEach } from 'bun:test' +import { render, waitFor, act, fireEvent, cleanup } from '@testing-library/react' +import React, { memo } 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 field accessor keeps a stable identity', async () => { + const adapter = new MockAdapter(createMockData(), { delay: 0 }) + 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 + } + 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) + // ... but the field accessor was the same instance every time (handle cache survived). + expect(new Set(seenTitle).size).toBe(1) + }) +}) From c507b53f0dd8ee048c5800fb87da62ef112d3a8f Mon Sep 17 00:00:00 2001 From: David Matejka Date: Wed, 1 Jul 2026 16:34:51 +0200 Subject: [PATCH 2/2] fix(bindx-react): keep useEntity ready result stable --- packages/bindx-react/src/hooks/useEntity.ts | 59 ++++++++++++----- tests/react/jsx/stableHandleIdentity.test.tsx | 66 ++++++++++++++++++- 2 files changed, 107 insertions(+), 18 deletions(-) diff --git a/packages/bindx-react/src/hooks/useEntity.ts b/packages/bindx-react/src/hooks/useEntity.ts index 4e58da3..bc1af43 100644 --- a/packages/bindx-react/src/hooks/useEntity.ts +++ b/packages/bindx-react/src/hooks/useEntity.ts @@ -346,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')) { @@ -361,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 @@ -372,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/tests/react/jsx/stableHandleIdentity.test.tsx b/tests/react/jsx/stableHandleIdentity.test.tsx index c05b51f..ef0bbf5 100644 --- a/tests/react/jsx/stableHandleIdentity.test.tsx +++ b/tests/react/jsx/stableHandleIdentity.test.tsx @@ -1,7 +1,7 @@ import '../../setup' import { describe, test, expect, afterEach } from 'bun:test' import { render, waitFor, act, fireEvent, cleanup } from '@testing-library/react' -import React, { memo } from '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' @@ -105,8 +105,9 @@ describe(' handle identity across data-driven re-render', () => { }) describe('useEntity handle identity across data-driven re-render', () => { - test('the returned field accessor keeps a stable identity', async () => { + 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 { @@ -114,6 +115,7 @@ describe('useEntity handle identity across data-driven re-render', () => { if (article.$status !== 'ready') { return loading } + seenArticles.push(article) seenTitle.push(article.title) return (