refactor(bindx-react): stable EntityHandle identity (fine-grained reactivity)

packages/bindx-react/src/hooks/useEntity.ts

@@ -323,21 +323,20 @@ 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<Record<string, object>>),
-		[id, entityType, store, dispatcher, schemaRegistry, snapshot],
+		[id, entityType, store, dispatcher, schemaRegistry],
 	)
 
 	const handle = useMemo(
 		() => EntityHandle.wrapProxy(rawHandle),
 		[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<ReadyResultMeta>({
+		$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<object, object> => {
 		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<T> at runtime via field access delegation
 	return result as UseEntityResult<any, any>
@@ -373,32 +399,32 @@ export function useEntity(
 // Result factories
 // ============================================================================
 
+interface ReadyResultMeta extends Record<string, unknown> {
+	readonly $status: 'ready'
+	readonly $isLoading: false
+	readonly $isRefetching: boolean
+	readonly $isError: false
+	readonly $isNotFound: false
+	readonly $error: null
+	$persist(): Promise<void>
+	$reset(): void
+}
+
 function createReadyResult(
 	handle: EntityAccessor<object, object>,
-	persist: () => Promise<void>,
-	reset: () => void,
-	isRefetching: boolean,
+	metaRef: { readonly current: ReadyResultMeta },
 ): UseEntityResult<object, object> {
-	const meta: Record<string, unknown> = {
-		$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
 			}

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 (<Field>, <HasOne>, <HasMany>) 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,25 +312,21 @@ 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(
 		() => EntityHandle.wrapProxy(rawHandle),
 		[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<unknown>)
 	return <>{annotateElement(result, { 'data-entity': entityType, 'data-entity-id': entityId })}</>
 }

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 (<Field>/useField), not through a churning handle
+ * reference. These tests lock in both halves of that contract.
+ */
+describe('<Entity> 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<string>[] = []
+		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<string> }): 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 <span data-testid="title">{title.value}</span>
+		}
+
+		const { container } = render(
+			<BindxProvider adapter={adapter} schema={testSchema}>
+				<Entity entity={schema.Article} by={{ id: 'article-1' }}>
+					{article => <Capture title={article.title} />}
+				</Entity>
+			</BindxProvider>,
+		)
+
+		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 <Field> 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<Article> }): React.ReactElement {
+			memoRenders++
+			return <span data-testid="static">static</span>
+		})
+
+		function Controls({ article }: { article: EntityRef<Article> }): null {
+			const title = useField(article.title)
+			bump = () => title.setValue(`${title.value}!`)
+			return null
+		}
+
+		const { container } = render(
+			<BindxProvider adapter={adapter} schema={testSchema}>
+				<Entity entity={schema.Article} by={{ id: 'article-1' }}>
+					{article => (
+						<>
+							<StaticChild article={article} />
+							<span data-testid="field-value"><Field field={article.title} /></span>
+							<Controls article={article} />
+						</>
+					)}
+				</Entity>
+			</BindxProvider>,
+		)
+
+		await waitFor(() => {
+			expect(queryByTestId(container, 'static')).not.toBeNull()
+		})
+
+		const memoRendersBefore = memoRenders
+		act(() => {
+			bump!()
+		})
+
+		// <Field> 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<string>[] = []
+
+		function Probe(): React.ReactElement {
+			const article = useEntity(schema.Article, { by: { id: 'article-1' } }, e => e.title())
+			if (article.$status !== 'ready') {
+				return <span data-testid="loading">loading</span>
+			}
+			seenArticles.push(article)
+			seenTitle.push(article.title)
+			return (
+				<button
+					data-testid="title"
+					onClick={() => article.title.setValue(`${article.title.value}!`)}
+				>
+					{article.title.value}
+				</button>
+			)
+		}
+
+		const { container } = render(
+			<BindxProvider adapter={adapter} schema={testSchema}>
+				<Probe />
+			</BindxProvider>,
+		)
+
+		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 <span data-testid="loading">loading</span>
+			}
+
+			seenArticles.push(article)
+			seenRefetching.push(article.$isRefetching)
+			return (
+				<div>
+					<span data-testid="title">{article.title.value}</span>
+					<span data-testid="refetching">{article.$isRefetching ? 'yes' : 'no'}</span>
+				</div>
+			)
+		}
+
+		const { container } = render(
+			<BindxProvider adapter={adapter} schema={testSchema}>
+				<Probe />
+			</BindxProvider>,
+		)
+
+		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)
+	})
+})