From 1b27e9ef4873ebbabf9eaaeddabe844fa6de306a Mon Sep 17 00:00:00 2001 From: David Matejka Date: Thu, 10 Sep 2026 16:45:33 +0200 Subject: [PATCH 1/4] perf(bindx-react): batch list refresh notifications --- .../bindx-react/src/hooks/useEntityList.ts | 25 ++-- packages/bindx/src/store/SnapshotStore.ts | 5 + .../bindx/src/store/SubscriptionManager.ts | 86 ++++++++---- .../useEntityList/batchNotifications.test.tsx | 48 +++++++ tests/unit/store/notificationBatch.test.ts | 123 ++++++++++++++++++ 5 files changed, 247 insertions(+), 40 deletions(-) create mode 100644 tests/react/hooks/useEntityList/batchNotifications.test.tsx create mode 100644 tests/unit/store/notificationBatch.test.ts diff --git a/packages/bindx-react/src/hooks/useEntityList.ts b/packages/bindx-react/src/hooks/useEntityList.ts index edbbd67..428d9ca 100644 --- a/packages/bindx-react/src/hooks/useEntityList.ts +++ b/packages/bindx-react/src/hooks/useEntityList.ts @@ -456,19 +456,20 @@ export function useEntityList( throw new Error('Unexpected query result type') } - const items = result.data.map((data: Record) => { - const id = data['id'] as string - // Revalidation: advance the server baseline but keep local dirty - // edits intact (see EntitySnapshotStore.refreshServerData). - dispatcher.dispatch( - refreshServerData(entityType, id, data), - ) - return { id, data: data as object } + store.batchNotifications(() => { + const items = result.data.map((data: Record) => { + const id = data['id'] as string + // Revalidation preserves local edits while advancing the server baseline. + dispatcher.dispatch( + refreshServerData(entityType, id, data), + ) + return { id, data: data as object } + }) + + listStateRef.current = { status: 'ready', items, isRefetching: false } + versionRef.current++ + store.notify() }) - - listStateRef.current = { status: 'ready', items, isRefetching: false } - versionRef.current++ - store.notify() } catch (error) { if (abortController.signal.aborted) return diff --git a/packages/bindx/src/store/SnapshotStore.ts b/packages/bindx/src/store/SnapshotStore.ts index eed7820..7d9146c 100644 --- a/packages/bindx/src/store/SnapshotStore.ts +++ b/packages/bindx/src/store/SnapshotStore.ts @@ -176,6 +176,11 @@ export class SnapshotStore implements SnapshotVersionBumper, JournalTarget { } } + /** Coalesces synchronous notifications, even on failure, without changing undo boundaries. */ + batchNotifications(fn: () => T): T { + return this.subscriptions.batchNotifications(fn) + } + // ==================== Key Generation ==================== private getEntityKey(entityType: string, id: string): string { diff --git a/packages/bindx/src/store/SubscriptionManager.ts b/packages/bindx/src/store/SubscriptionManager.ts index 09d4ef9..30dd435 100644 --- a/packages/bindx/src/store/SubscriptionManager.ts +++ b/packages/bindx/src/store/SubscriptionManager.ts @@ -45,6 +45,48 @@ export class SubscriptionManager implements Rekeyable { /** Global version number for change detection */ private globalVersion = 0 + private notificationBatchDepth = 0 + private pendingEntities = new Set() + private pendingRelations = new Set() + private pendingGlobal = false + + /** Versions advance immediately; callbacks observe the completed synchronous batch. */ + batchNotifications(fn: () => T): T { + this.notificationBatchDepth++ + try { + return fn() + } finally { + if (--this.notificationBatchDepth === 0) this.flushNotifications() + } + } + + private flushNotifications(): void { + const entities = this.pendingEntities + const relations = this.pendingRelations + const global = this.pendingGlobal + this.pendingEntities = new Set() + this.pendingRelations = new Set() + this.pendingGlobal = false + const notified = new Set() + const deliver = (subscribers: Set | undefined): void => { + for (const sub of subscribers ?? []) { + if (notified.has(sub)) continue + notified.add(sub) + sub() + } + } + for (const key of entities) deliver(this.entitySubscribers.get(this.resolveKey(key))) + for (const key of relations) deliver(this.relationSubscribers.get(this.resolveKey(key))) + if (global) deliver(this.globalSubscribers) + } + + private notifyGlobalSubscribers(): void { + if (this.notificationBatchDepth > 0) { + this.pendingGlobal = true + return + } + for (const sub of this.globalSubscribers) sub() + } /** * Resolves a child's parents from live relation edges. Injected after @@ -127,9 +169,7 @@ export class SubscriptionManager implements Rekeyable { */ notify(): void { this.globalVersion++ - for (const sub of this.globalSubscribers) { - sub() - } + this.notifyGlobalSubscribers() } /** @@ -145,13 +185,9 @@ export class SubscriptionManager implements Rekeyable { // Iterate the live sets, as the per-key paths do: a subscriber that unsubscribes a // not-yet-visited sibling removes it from the iteration, whereas a copied array would // still invoke it after its unsubscribe() returned. - for (const subs of this.entitySubscribers.values()) { - for (const sub of subs) sub() - } - for (const subs of this.relationSubscribers.values()) { - for (const sub of subs) sub() - } - for (const sub of this.globalSubscribers) sub() + for (const key of this.entitySubscribers.keys()) this.notifyEntityDirect(key) + for (const key of this.relationSubscribers.keys()) this.notifyRelationDirect(key) + this.notifyGlobalSubscribers() } // ==================== Parent-Child Relationships ==================== @@ -221,12 +257,7 @@ export class SubscriptionManager implements Rekeyable { notifiedKeys.add(key) // Notify entity-specific subscribers - const entitySubs = this.entitySubscribers.get(key) - if (entitySubs) { - for (const sub of entitySubs) { - sub() - } - } + this.notifyEntityDirect(key) // Notify parent entity subscribers (propagate change up the tree). // Parents are derived from the relation store's LIVE edges, so a @@ -243,9 +274,7 @@ export class SubscriptionManager implements Rekeyable { // Notify global subscribers (only once, from the root invocation — not // again for each parent reached via propagation) if (isRoot) { - for (const sub of this.globalSubscribers) { - sub() - } + this.notifyGlobalSubscribers() } } @@ -262,12 +291,7 @@ export class SubscriptionManager implements Rekeyable { this.globalVersion++ // Notify relation-specific subscribers - const relationSubs = this.relationSubscribers.get(key) - if (relationSubs) { - for (const sub of relationSubs) { - sub() - } - } + this.notifyRelationDirect(key) // Bump entity snapshot version so isEqual detects a change bumper.bumpEntitySnapshotVersion(entityKey) @@ -280,6 +304,10 @@ export class SubscriptionManager implements Rekeyable { * Used during batch imports (e.g., undo/redo). */ notifyEntityDirect(key: string): void { + if (this.notificationBatchDepth > 0) { + this.pendingEntities.add(key) + return + } const subs = this.entitySubscribers.get(key) if (subs) { for (const sub of subs) { @@ -293,6 +321,10 @@ export class SubscriptionManager implements Rekeyable { * Used during batch imports (e.g., undo/redo). */ notifyRelationDirect(key: string): void { + if (this.notificationBatchDepth > 0) { + this.pendingRelations.add(key) + return + } const subs = this.relationSubscribers.get(key) if (subs) { for (const sub of subs) { @@ -306,9 +338,7 @@ export class SubscriptionManager implements Rekeyable { */ notifyGlobal(): void { this.globalVersion++ - for (const sub of this.globalSubscribers) { - sub() - } + this.notifyGlobalSubscribers() } /** diff --git a/tests/react/hooks/useEntityList/batchNotifications.test.tsx b/tests/react/hooks/useEntityList/batchNotifications.test.tsx new file mode 100644 index 0000000..b2905d8 --- /dev/null +++ b/tests/react/hooks/useEntityList/batchNotifications.test.tsx @@ -0,0 +1,48 @@ +import '../../../setup' +import { afterEach, expect, test } from 'bun:test' +import { cleanup, render, waitFor } from '@testing-library/react' +import React, { useLayoutEffect } from 'react' +import { + BindxProvider, MockAdapter, defineSchema, entityDef, scalar, + useEntityList, useSnapshotStore, +} from '@contember/bindx-react' + +afterEach(cleanup) + +interface Article { + id: string + title: string +} + +const schema = defineSchema<{ Article: Article }>({ + entities: { Article: { fields: { id: scalar(), title: scalar() } } }, +}) +const articleDef = entityDef
('Article') + +test('a list response notifies global and entity subscribers only after all rows are loaded', async () => { + const count = 200 + const rows = Object.fromEntries(Array.from({ length: count }, (_, i) => [String(i), { id: String(i), title: `Row ${i}` }])) + const adapter = new MockAdapter({ Article: rows }, { delay: 0 }) + const observedCounts: number[] = [] + let entityCalls = 0 + + function List(): React.JSX.Element { + const store = useSnapshotStore() + useLayoutEffect(() => { + const readCount = (): number => Object.keys(rows).filter(id => store.hasEntity('Article', id)).length + const unsubscribe = store.subscribe(() => observedCounts.push(readCount())) + const unsubscribeEntity = store.subscribeToEntity('Article', '0', () => { + entityCalls++ + expect(readCount()).toBe(count) + }) + return () => { unsubscribe(); unsubscribeEntity() } + }, [store]) + const articles = useEntityList(articleDef, {}, a => a.id().title()) + return
{articles.$status === 'ready' ? `Loaded ${articles.items.length}` : 'Loading'}
+ } + + const view = render() + await waitFor(() => expect(view.getByText(`Loaded ${count}`)).toBeDefined()) + expect(observedCounts.filter(value => value > 0)).toEqual([count]) + expect(entityCalls).toBe(1) +}) diff --git a/tests/unit/store/notificationBatch.test.ts b/tests/unit/store/notificationBatch.test.ts new file mode 100644 index 0000000..ea125c8 --- /dev/null +++ b/tests/unit/store/notificationBatch.test.ts @@ -0,0 +1,123 @@ +import { describe, expect, test } from 'bun:test' +import { SnapshotStore } from '@contember/bindx' +import { SubscriptionManager } from '../../../packages/bindx/src/store/SubscriptionManager.js' + +describe('notification batches', () => { + test('nested refreshes expose final data and preserve local edits', () => { + const store = new SnapshotStore() + store.setEntityData('Article', 'a', { title: 'Original', count: 0 }, true) + store.setFieldValue('Article', 'a', ['title'], 'Local') + let calls = 0 + store.subscribe(() => { + calls++ + expect(store.getEntitySnapshot('Article', 'b')).toBeDefined() + expect(store.getEntitySnapshot('Article', 'a')?.data).toMatchObject({ title: 'Local', count: 2 }) + }) + store.batchNotifications(() => { + store.refreshServerData('Article', 'a', { title: 'Server', count: 1 }) + store.batchNotifications(() => { + store.refreshServerData('Article', 'a', { title: 'Server', count: 2 }) + }) + expect(calls).toBe(0) + store.refreshServerData('Article', 'b', { title: 'Other' }) + store.notify() + }) + expect(calls).toBe(1) + }) + + test('deduplicates entity, relation, ancestor and global callbacks with current versions', () => { + const manager = new SubscriptionManager() + const bumped: string[] = [] + const bumper = { bumpEntitySnapshotVersion: (key: string): void => { bumped.push(key) } } + manager.setParentKeyLookup({ getParentKeysForChild: id => new Set(id === 'child' ? ['Article:parent'] : []) }) + let calls = 0 + const subscriber = (): void => { + calls++ + expect(manager.getVersion()).toBe(3) + expect(bumped).toContain('Article:parent') + } + manager.subscribeToEntity('Article:child', subscriber) + manager.subscribeToEntity('Article:parent', subscriber) + manager.subscribeToRelation('Article:child:tags', subscriber) + manager.subscribe(subscriber) + manager.batchNotifications(() => { + manager.notifyEntitySubscribers('Article:child', bumper) + manager.notifyRelationSubscribers('Article:child:tags', 'Article:child', bumper) + manager.notify() + expect(calls).toBe(0) + }) + expect(calls).toBe(1) + }) + + test('honors unsubscribe before and during delivery', () => { + const manager = new SubscriptionManager() + let calls = 0 + let unsubscribe = (): void => {} + manager.subscribe(() => unsubscribe()) + unsubscribe = manager.subscribe(() => { calls++ }) + const unsubscribeEntity = manager.subscribeToEntity('Article:a', () => { calls++ }) + manager.batchNotifications(() => { + manager.notifyEntityDirect('Article:a') + manager.notify() + unsubscribeEntity() + }) + expect(calls).toBe(0) + }) + + test('flushes completed writes on failure and restores immediate notifications', () => { + const manager = new SubscriptionManager() + let calls = 0 + manager.subscribe(() => { calls++ }) + expect(() => manager.batchNotifications(() => { + manager.notify() + throw new Error('Failed write') + })).toThrow('Failed write') + expect(calls).toBe(1) + manager.notify() + expect(calls).toBe(2) + }) + + test('queued notifications follow persisted identities and unsubscribe redirects', () => { + const store = new SnapshotStore() + const tempId = store.createEntity('Article', { title: 'Draft' }) + let calls = 0 + let removedCalls = 0 + store.subscribeToEntity('Article', tempId, () => { calls++ }) + const unsubscribe = store.subscribeToEntity('Article', tempId, () => { removedCalls++ }) + store.batchNotifications(() => { + store.setFieldValue('Article', tempId, ['title'], 'Edited') + store.mapTempIdToPersistedId('Article', tempId, 'persisted') + unsubscribe() + }) + expect(calls).toBe(1) + expect(removedCalls).toBe(0) + }) + + test('clear notifies all subscription scopes after the batch', () => { + const store = new SnapshotStore() + store.setEntityData('Article', 'a', { title: 'Original' }, true) + let calls = 0 + const subscriber = (): void => { + calls++ + expect(store.hasEntity('Article', 'a')).toBe(false) + } + store.subscribeToEntity('Article', 'a', subscriber) + store.subscribe(subscriber) + store.batchNotifications(() => { + store.setFieldValue('Article', 'a', ['title'], 'Edited') + store.clear() + expect(calls).toBe(0) + }) + expect(calls).toBe(1) + }) + + test('delivers reentrant writes rather than losing them in the completed batch', () => { + const manager = new SubscriptionManager() + let calls = 0 + manager.subscribe(() => { + if (++calls === 1) manager.batchNotifications(() => manager.notify()) + }) + manager.batchNotifications(() => manager.notify()) + expect(calls).toBe(2) + }) +}) From 1aef15524225dae49c48284accec558482648165 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Fri, 11 Sep 2026 15:53:54 +0200 Subject: [PATCH 2/4] fix(ci): align Contember images with locked schema version --- .github/workflows/ci.yml | 4 ++-- docker-compose.yaml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fbb62a4..ca0fe36 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -61,7 +61,7 @@ jobs: - 9000:8000 contember-engine: - image: contember/engine:2.1.0-beta.2 + image: contember/engine:2.1.0-rc.3 env: NODE_ENV: development CONTEMBER_PORT: "4000" @@ -114,7 +114,7 @@ jobs: -e CONTEMBER_PROJECT_NAME=example \ -v ${{ github.workspace }}:/src \ -w /src/packages/example \ - contember/cli:2.1.0-beta.2 \ + contember/cli:2.1.0-rc.3 \ migrations:execute --yes CONTEMBER_API_URL=http://localhost:1581 CONTEMBER_API_TOKEN=0000000000000000000000000000000000000000 bun run packages/example/seed.ts diff --git a/docker-compose.yaml b/docker-compose.yaml index 5853cc7..a7f5d37 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -1,6 +1,6 @@ services: contember-engine: - image: contember/engine:2.1.0-beta.2 + image: contember/engine:2.1.0-rc.3 environment: NODE_ENV: 'development' @@ -43,7 +43,7 @@ services: condition: service_healthy contember-cli: - image: contember/cli:2.1.0-beta.2 + image: contember/cli:2.1.0-rc.3 user: '1000:1000' deploy: From 6c5f47d8595ff572301fa85efc1c2f69cb645baa Mon Sep 17 00:00:00 2001 From: David Matejka Date: Fri, 11 Sep 2026 16:40:28 +0200 Subject: [PATCH 3/4] fix(bindx): harden notification batches - A throwing subscriber no longer drops the rest of a flush. Delivery finishes, then the first subscriber error propagates and the rest are logged. When the batch callback itself throws, its error wins. - Queued keys move with their subscribers on rekey, so a flush no longer reads the redirect map, which clear() does not reset. - batchNotifications rejects async callbacks at compile time. - Rows under a shared ancestor walk it once per batch. The walked set is reset whenever a relation write changes the edges. - applyJournalImages and importPartialSnapshot run in a batch: an undo that removes K creates notifies once, after the restore completes. - notifyGlobal() is removed; it was identical to notify(). Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DQrj6jnat2NHRCAR4vgmWk --- packages/bindx/src/store/SnapshotStore.ts | 72 ++++---- .../bindx/src/store/SubscriptionManager.ts | 106 +++++++----- tests/unit/store/notificationBatch.test.ts | 156 +++++++++++++++++- 3 files changed, 254 insertions(+), 80 deletions(-) diff --git a/packages/bindx/src/store/SnapshotStore.ts b/packages/bindx/src/store/SnapshotStore.ts index 7d9146c..b5a2ca9 100644 --- a/packages/bindx/src/store/SnapshotStore.ts +++ b/packages/bindx/src/store/SnapshotStore.ts @@ -1,7 +1,7 @@ import type { EntitySnapshot, LoadStatus } from './snapshots.js' import { createEntitySnapshot } from './snapshots.js' import type { FieldError, FieldErrorFilter } from '../errors/types.js' -import { SubscriptionManager, type SnapshotVersionBumper } from './SubscriptionManager.js' +import { SubscriptionManager, type SnapshotVersionBumper, type SynchronousResult } from './SubscriptionManager.js' import { ErrorStore } from './ErrorStore.js' import { RelationStore, @@ -177,7 +177,7 @@ export class SnapshotStore implements SnapshotVersionBumper, JournalTarget { } /** Coalesces synchronous notifications, even on failure, without changing undo boundaries. */ - batchNotifications(fn: () => T): T { + batchNotifications(fn: () => SynchronousResult): T { return this.subscriptions.batchNotifications(fn) } @@ -1110,23 +1110,16 @@ export class SnapshotStore implements SnapshotVersionBumper, JournalTarget { hasManyStates: Map entityMetas: Map }): void { - const notifiedEntityKeys = this.entitySnapshots.importSnapshots(snapshot.entitySnapshots) + this.subscriptions.batchNotifications(() => { + const entityKeys = this.entitySnapshots.importSnapshots(snapshot.entitySnapshots) + this.meta.importMetas(snapshot.entityMetas) + const relationKeys = this.relations.importRelationStates(snapshot.relationStates) + const hasManyKeys = this.relations.importHasManyStates(snapshot.hasManyStates) - this.meta.importMetas(snapshot.entityMetas) - - const relationKeys = this.relations.importRelationStates(snapshot.relationStates) - const hasManyKeys = this.relations.importHasManyStates(snapshot.hasManyStates) - const notifiedRelationKeys = new Set([...relationKeys, ...hasManyKeys]) - - this.subscriptions.notifyGlobal() - - for (const key of notifiedEntityKeys) { - this.subscriptions.notifyEntityDirect(key) - } - - for (const key of notifiedRelationKeys) { - this.subscriptions.notifyRelationDirect(key) - } + this.subscriptions.notify() + for (const key of entityKeys) this.subscriptions.notifyEntityDirect(key) + for (const key of [...relationKeys, ...hasManyKeys]) this.subscriptions.notifyRelationDirect(key) + }) } // ==================== Undo Journal Target (cell capture / restore) ==================== @@ -1241,9 +1234,6 @@ export class SnapshotStore implements SnapshotVersionBumper, JournalTarget { * then present relations, then un-creates, then dropped relations. */ applyJournalImages(images: JournalCellImage[]): void { - const notifyEntities = new Set() - const notifyRelations = new Set() - const presentEntities: EntityCellImage[] = [] const presentRelations: Array = [] const absentEntities: EntityCellImage[] = [] @@ -1251,29 +1241,37 @@ export class SnapshotStore implements SnapshotVersionBumper, JournalTarget { for (const img of images) { if (img.kind === 'entity') { - notifyEntities.add(img.key) ;(img.present ? presentEntities : absentEntities).push(img) } else { - notifyRelations.add(img.key) - notifyEntities.add(entityKeyOfRelationKey(img.key)) ;(img.present ? presentRelations : absentRelations).push(img) } } - for (const img of presentEntities) this.applyEntityImage(img) - for (const img of presentRelations) this.applyRelationImage(img) - for (const img of absentEntities) { - const [type, id] = splitEntityKey(img.key) - this.removeEntity(type, id) - } - for (const img of absentRelations) { - if (img.kind === 'relation') this.relations.removeRelationState(img.key) - else this.relations.removeHasManyState(img.key) - } + // removeEntity notifies; the batch holds that back until the restore is complete. + this.subscriptions.batchNotifications(() => { + for (const img of presentEntities) this.applyEntityImage(img) + for (const img of presentRelations) this.applyRelationImage(img) + for (const img of absentEntities) { + const [type, id] = splitEntityKey(img.key) + this.removeEntity(type, id) + } + for (const img of absentRelations) { + if (img.kind === 'relation') this.relations.removeRelationState(img.key) + else this.relations.removeHasManyState(img.key) + } - this.subscriptions.notifyGlobal() - for (const key of notifyEntities) this.subscriptions.notifyEntityDirect(key) - for (const key of notifyRelations) this.subscriptions.notifyRelationDirect(key) + this.subscriptions.notify() + for (const img of images) this.notifyRestoredCell(img) + }) + } + + private notifyRestoredCell(img: JournalCellImage): void { + if (img.kind === 'entity') { + this.subscriptions.notifyEntityDirect(img.key) + return + } + this.subscriptions.notifyRelationDirect(img.key) + this.subscriptions.notifyEntityDirect(entityKeyOfRelationKey(img.key)) } private applyEntityImage(img: EntityCellImage): void { diff --git a/packages/bindx/src/store/SubscriptionManager.ts b/packages/bindx/src/store/SubscriptionManager.ts index 30dd435..2aa4b85 100644 --- a/packages/bindx/src/store/SubscriptionManager.ts +++ b/packages/bindx/src/store/SubscriptionManager.ts @@ -2,6 +2,9 @@ import type { RekeyContext, Rekeyable } from './RekeyOrchestrator.js' type Subscriber = () => void +/** Rejects async callbacks at compile time: a notification batch would end at the first `await`. */ +export type SynchronousResult = T extends PromiseLike ? never : T + /** * Callback interface for SubscriptionManager to request snapshot version bumps. * This decouples notification logic from entity snapshot storage. @@ -18,6 +21,8 @@ export interface SnapshotVersionBumper { */ export interface ParentKeyLookup { getParentKeysForChild(childId: string): Set + /** Increases on every relation write, so an unchanged value proves the edges are unchanged. */ + getMutationVersion(): number } /** @@ -49,35 +54,48 @@ export class SubscriptionManager implements Rekeyable { private pendingEntities = new Set() private pendingRelations = new Set() private pendingGlobal = false + /** Keys the current batch has already walked; valid only while the relation edges are unchanged. */ + private readonly batchWalkedKeys = new Set() + private batchWalkedEdgeVersion = -1 /** Versions advance immediately; callbacks observe the completed synchronous batch. */ - batchNotifications(fn: () => T): T { + batchNotifications(fn: () => SynchronousResult): T { this.notificationBatchDepth++ + let completed = false try { - return fn() + const result = fn() + completed = true + return result } finally { - if (--this.notificationBatchDepth === 0) this.flushNotifications() + if (--this.notificationBatchDepth === 0) this.flushNotifications(completed) } } - private flushNotifications(): void { + private flushNotifications(callbackCompleted: boolean): void { const entities = this.pendingEntities const relations = this.pendingRelations const global = this.pendingGlobal this.pendingEntities = new Set() this.pendingRelations = new Set() this.pendingGlobal = false + this.batchWalkedKeys.clear() const notified = new Set() + const errors: unknown[] = [] const deliver = (subscribers: Set | undefined): void => { for (const sub of subscribers ?? []) { if (notified.has(sub)) continue notified.add(sub) - sub() + try { + sub() + } catch (error) { + errors.push(error) + } } } - for (const key of entities) deliver(this.entitySubscribers.get(this.resolveKey(key))) - for (const key of relations) deliver(this.relationSubscribers.get(this.resolveKey(key))) + for (const key of entities) deliver(this.entitySubscribers.get(key)) + for (const key of relations) deliver(this.relationSubscribers.get(key)) if (global) deliver(this.globalSubscribers) + throwSubscriberErrors(errors, callbackCompleted) } private notifyGlobalSubscribers(): void { @@ -229,7 +247,19 @@ export class SubscriptionManager implements Rekeyable { bumper: SnapshotVersionBumper, ): void { this.globalVersion++ - this.notifyEntityAndParentSubscribers(key, bumper, new Set()) + this.notifyEntityAndParentSubscribers(key, bumper, this.getWalkedKeys()) + this.notifyGlobalSubscribers() + } + + /** A batch shares one walked set, so rows under a common hub walk it once instead of once per row. */ + private getWalkedKeys(): Set { + if (this.notificationBatchDepth === 0) return new Set() + const edgeVersion = this.parentKeyLookup?.getMutationVersion() ?? 0 + if (edgeVersion !== this.batchWalkedEdgeVersion) { + this.batchWalkedKeys.clear() + this.batchWalkedEdgeVersion = edgeVersion + } + return this.batchWalkedKeys } /** @@ -245,36 +275,21 @@ export class SubscriptionManager implements Rekeyable { private notifyEntityAndParentSubscribers( key: string, bumper: SnapshotVersionBumper, - notifiedKeys: Set, + walkedKeys: Set, ): void { - // Prevent infinite recursion - if (notifiedKeys.has(key)) return - // Capture "am I the root invocation" BEFORE adding/recursing — the parent - // propagation below grows the shared `notifiedKeys` set, so checking its - // size after recursion would misclassify any child-with-parent as non-root - // and skip the global notification (see issue #51). - const isRoot = notifiedKeys.size === 0 - notifiedKeys.add(key) - - // Notify entity-specific subscribers + if (walkedKeys.has(key)) return + walkedKeys.add(key) + this.notifyEntityDirect(key) - // Notify parent entity subscribers (propagate change up the tree). // Parents are derived from the relation store's LIVE edges, so a // disconnected child no longer reaches its former parent. - const parents = this.getParentKeys(key) - for (const parentKey of parents) { + for (const parentKey of this.getParentKeys(key)) { // An ancestor reachable through several edges is bumped and walked once. - if (notifiedKeys.has(parentKey)) continue + if (walkedKeys.has(parentKey)) continue // Bump parent snapshot version so useSyncExternalStore detects a change bumper.bumpEntitySnapshotVersion(parentKey) - this.notifyEntityAndParentSubscribers(parentKey, bumper, notifiedKeys) - } - - // Notify global subscribers (only once, from the root invocation — not - // again for each parent reached via propagation) - if (isRoot) { - this.notifyGlobalSubscribers() + this.notifyEntityAndParentSubscribers(parentKey, bumper, walkedKeys) } } @@ -296,7 +311,8 @@ export class SubscriptionManager implements Rekeyable { // Bump entity snapshot version so isEqual detects a change bumper.bumpEntitySnapshotVersion(entityKey) - this.notifyEntityAndParentSubscribers(entityKey, bumper, new Set()) + this.notifyEntityAndParentSubscribers(entityKey, bumper, this.getWalkedKeys()) + this.notifyGlobalSubscribers() } /** @@ -333,14 +349,6 @@ export class SubscriptionManager implements Rekeyable { } } - /** - * Bumps global version and notifies global subscribers. - */ - notifyGlobal(): void { - this.globalVersion++ - this.notifyGlobalSubscribers() - } - /** * Moves entity and relation subscriptions from oldKey to newKey. * Also rekeys relation subscribers under oldKeyPrefix to newKeyPrefix. @@ -358,6 +366,7 @@ export class SubscriptionManager implements Rekeyable { } } this.rekeyedKeys.set(oldKey, newKey) + this.movePendingNotifications(ctx) // Move entity subscribers, merging into anything already subscribed under the new key this.moveSubscribers(this.entitySubscribers, oldKey, newKey) @@ -384,6 +393,16 @@ export class SubscriptionManager implements Rekeyable { } } + /** Queued keys follow their subscribers, so a flush never reads the long-lived redirect map. */ + private movePendingNotifications({ oldKey, newKey, oldKeyPrefix, newKeyPrefix }: RekeyContext): void { + if (this.pendingEntities.delete(oldKey)) this.pendingEntities.add(newKey) + for (const key of [...this.pendingRelations]) { + if (!key.startsWith(oldKeyPrefix)) continue + this.pendingRelations.delete(key) + this.pendingRelations.add(newKeyPrefix + key.slice(oldKeyPrefix.length)) + } + } + /** * Re-homes a subscriber set under a new key. The destination may already hold * subscribers (a component mounted on the persisted id before the draft was rekeyed @@ -402,3 +421,12 @@ export class SubscriptionManager implements Rekeyable { for (const sub of moved) existing.add(sub) } } + +/** The first subscriber error propagates, as it would unbatched, and the rest are logged; a failed callback's own error wins. */ +function throwSubscriberErrors(errors: readonly unknown[], callbackCompleted: boolean): void { + const [first, ...rest] = errors + for (const error of callbackCompleted ? rest : errors) { + console.error('[Bindx SubscriptionManager] Subscriber error:', error) + } + if (callbackCompleted && errors.length > 0) throw first +} diff --git a/tests/unit/store/notificationBatch.test.ts b/tests/unit/store/notificationBatch.test.ts index ea125c8..c1ed1f8 100644 --- a/tests/unit/store/notificationBatch.test.ts +++ b/tests/unit/store/notificationBatch.test.ts @@ -1,6 +1,36 @@ -import { describe, expect, test } from 'bun:test' -import { SnapshotStore } from '@contember/bindx' -import { SubscriptionManager } from '../../../packages/bindx/src/store/SubscriptionManager.js' +import { describe, expect, spyOn, test, type Mock } from 'bun:test' +import { ActionDispatcher, SnapshotStore, UndoManager } from '@contember/bindx' +import { + SubscriptionManager, + type ParentKeyLookup, + type SynchronousResult, +} from '../../../packages/bindx/src/store/SubscriptionManager.js' + +class FakeParentLookup implements ParentKeyLookup { + calls = 0 + version = 0 + + constructor(private readonly parents: Map) {} + + getParentKeysForChild(childId: string): Set { + this.calls++ + return new Set(this.parents.get(childId) ?? []) + } + + getMutationVersion(): number { + return this.version + } +} + +type AssertEqual = [T] extends [U] ? ([U] extends [T] ? true : false) : false + +function assertType(): void { + // compile-time only +} + +function loggedMessages(logged: Mock): string[] { + return logged.mock.calls.map(([, error]) => error instanceof Error ? error.message : String(error)) +} describe('notification batches', () => { test('nested refreshes expose final data and preserve local edits', () => { @@ -29,7 +59,7 @@ describe('notification batches', () => { const manager = new SubscriptionManager() const bumped: string[] = [] const bumper = { bumpEntitySnapshotVersion: (key: string): void => { bumped.push(key) } } - manager.setParentKeyLookup({ getParentKeysForChild: id => new Set(id === 'child' ? ['Article:parent'] : []) }) + manager.setParentKeyLookup(new FakeParentLookup(new Map([['child', ['Article:parent']]]))) let calls = 0 const subscriber = (): void => { calls++ @@ -120,4 +150,122 @@ describe('notification batches', () => { manager.batchNotifications(() => manager.notify()) expect(calls).toBe(2) }) + + test('a throwing subscriber does not drop the rest of the batch', () => { + const store = new SnapshotStore() + store.setEntityData('Article', 'a', { title: 'A' }, true) + store.setEntityData('Article', 'b', { title: 'B' }, true) + let otherCalls = 0 + let globalCalls = 0 + store.subscribeToEntity('Article', 'a', () => { throw new Error('Subscriber a failed') }) + store.subscribeToEntity('Article', 'b', () => { otherCalls++ }) + store.subscribe(() => { globalCalls++ }) + expect(() => store.batchNotifications(() => { + store.refreshServerData('Article', 'a', { title: 'A2' }) + store.refreshServerData('Article', 'b', { title: 'B2' }) + })).toThrow('Subscriber a failed') + expect(otherCalls).toBe(1) + expect(globalCalls).toBe(1) + }) + + test('rethrows the first subscriber error and logs the rest', () => { + const manager = new SubscriptionManager() + const logged = spyOn(console, 'error').mockImplementation(() => {}) + try { + manager.subscribeToEntity('Article:a', () => { throw new Error('First') }) + manager.subscribe(() => { throw new Error('Second') }) + expect(() => manager.batchNotifications(() => { + manager.notifyEntityDirect('Article:a') + manager.notify() + })).toThrow('First') + expect(loggedMessages(logged)).toEqual(['Second']) + } finally { + logged.mockRestore() + } + }) + + test('a failing callback keeps its own error and logs subscriber errors', () => { + const manager = new SubscriptionManager() + const logged = spyOn(console, 'error').mockImplementation(() => {}) + try { + manager.subscribe(() => { throw new Error('Subscriber failed') }) + expect(() => manager.batchNotifications(() => { + manager.notify() + throw new Error('Write failed') + })).toThrow('Write failed') + expect(loggedMessages(logged)).toEqual(['Subscriber failed']) + } finally { + logged.mockRestore() + } + }) + + test('after clear(), a batched write reaches a reused id like an immediate write does', () => { + const store = new SnapshotStore() + const tempId = store.createEntity('Article', { title: 'Draft' }) + store.mapTempIdToPersistedId('Article', tempId, 'persisted') + store.clear() + store.createEntity('Article', { id: tempId, title: 'Again' }) + let calls = 0 + store.subscribeToEntity('Article', tempId, () => { calls++ }) + store.setFieldValue('Article', tempId, ['title'], 'Immediate') + store.batchNotifications(() => store.setFieldValue('Article', tempId, ['title'], 'Batched')) + expect(calls).toBe(2) + }) + + test('rows under a shared ancestor walk it once per batch', () => { + const rows = Array.from({ length: 50 }, (_, i) => `r${i}`) + const parents = new Map([['hub', rows.map(id => `Article:${id}`)]]) + for (const id of rows) parents.set(id, ['Category:hub']) + const lookup = new FakeParentLookup(parents) + const manager = new SubscriptionManager() + manager.setParentKeyLookup(lookup) + let bumps = 0 + let hubCalls = 0 + manager.subscribeToEntity('Category:hub', () => { hubCalls++ }) + manager.batchNotifications(() => { + for (const id of rows) manager.notifyEntitySubscribers(`Article:${id}`, { bumpEntitySnapshotVersion: () => { bumps++ } }) + }) + expect(lookup.calls).toBe(rows.length + 1) + expect(bumps).toBe(rows.length) + expect(hubCalls).toBe(1) + }) + + test('a relation write inside a batch re-walks ancestors through the new edge', () => { + const parents = new Map([['child', ['Article:a']]]) + const lookup = new FakeParentLookup(parents) + const manager = new SubscriptionManager() + manager.setParentKeyLookup(lookup) + const bumped: string[] = [] + const bumper = { bumpEntitySnapshotVersion: (key: string): void => { bumped.push(key) } } + let newParentCalls = 0 + manager.subscribeToEntity('Article:b', () => { newParentCalls++ }) + manager.batchNotifications(() => { + manager.notifyEntitySubscribers('Item:child', bumper) + parents.set('child', ['Article:a', 'Article:b']) + lookup.version++ + manager.notifyEntitySubscribers('Item:child', bumper) + }) + expect(bumped).toContain('Article:b') + expect(newParentCalls).toBe(1) + }) + + test('undoing several creates notifies once, after the restore is complete', () => { + const store = new SnapshotStore() + const dispatcher = new ActionDispatcher(store) + const undo = new UndoManager(store, { debounceMs: 0 }) + dispatcher.addMiddleware(undo.createMiddleware()) + const ids = ['c0', 'c1', 'c2', 'c3'] + store.transaction(() => { + for (const id of ids) store.createEntity('Article', { id, title: id }) + }) + const remaining: number[] = [] + store.subscribe(() => remaining.push(ids.filter(id => store.hasEntity('Article', id)).length)) + undo.undo() + expect(remaining).toEqual([0]) + }) + + test('async callbacks are rejected at compile time', () => { + assertType>, never>>() + assertType, number>>() + }) }) From 4aa2fab85012feefc7613db0f2ff2157e7a20ee0 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Fri, 11 Sep 2026 16:40:28 +0200 Subject: [PATCH 4/4] perf: batch notifications of multi-row store writers Each of these wrote row by row and sent one global notification per row, so every mounted usePersist ran a dirty scan per row: - HasManyDataGrid page loads - BatchPersister claim, post-persist reconciliation, and release + sweep - EntityLoader.loadMany - useEntity: refreshed data and the success state now arrive together List and grid rows are validated before any write, without `as` casts; a row without a string id now fails the load. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DQrj6jnat2NHRCAR4vgmWk --- .../bindx-dataview/src/HasManyDataGrid.tsx | 17 ++-- packages/bindx-react/src/hooks/useEntity.ts | 9 ++- .../bindx-react/src/hooks/useEntityList.ts | 18 +++-- packages/bindx/src/core/EntityLoader.ts | 12 +-- .../bindx/src/persistence/BatchPersister.ts | 26 +++--- .../hasManyDataGridNotificationBatch.test.tsx | 72 +++++++++++++++++ .../useEntity/batchNotifications.test.tsx | 42 ++++++++++ .../useEntityList/batchNotifications.test.tsx | 81 ++++++++++++++----- .../persistNotificationBatch.test.ts | 53 ++++++++++++ 9 files changed, 273 insertions(+), 57 deletions(-) create mode 100644 tests/react/dataview/hasManyDataGridNotificationBatch.test.tsx create mode 100644 tests/react/hooks/useEntity/batchNotifications.test.tsx create mode 100644 tests/unit/persistence/persistNotificationBatch.test.ts diff --git a/packages/bindx-dataview/src/HasManyDataGrid.tsx b/packages/bindx-dataview/src/HasManyDataGrid.tsx index 2c6188e..1227111 100644 --- a/packages/bindx-dataview/src/HasManyDataGrid.tsx +++ b/packages/bindx-dataview/src/HasManyDataGrid.tsx @@ -84,6 +84,12 @@ interface ListState { const INITIAL_LIST_STATE: ListState = { status: 'loading', items: [] } +function getRowId(entityType: string, row: Record): string { + const id = row['id'] + if (typeof id !== 'string') throw new Error(`${entityType} relation row has no string id`) + return id +} + // ============================================================================ // Implementation // ============================================================================ @@ -183,12 +189,11 @@ function HasManyDataGridImpl({ return } - const items = relation.rows.map((data: Record) => { - const id = data['id'] as string - dispatcher.dispatch( - setEntityData(targetEntityType, id, data, true), - ) - return { id, data: data as object } + const items = relation.rows.map(data => ({ id: getRowId(targetEntityType, data), data })) + store.batchNotifications(() => { + for (const item of items) { + dispatcher.dispatch(setEntityData(targetEntityType, item.id, item.data, true)) + } }) setListState({ status: 'ready', items, totalCount: relation.totalCount }) diff --git a/packages/bindx-react/src/hooks/useEntity.ts b/packages/bindx-react/src/hooks/useEntity.ts index b0352b5..97294e6 100644 --- a/packages/bindx-react/src/hooks/useEntity.ts +++ b/packages/bindx-react/src/hooks/useEntity.ts @@ -290,12 +290,13 @@ export function useEntity( if (result.type === 'get' && result.data === null) { dispatcher.dispatch(setLoadState(entityType, id, 'not_found')) } else if (result.type === 'get' && result.data) { + const data = result.data // Revalidation: advance the server baseline but keep local dirty // edits intact (see EntitySnapshotStore.refreshServerData). - dispatcher.dispatch( - refreshServerData(entityType, id, result.data), - ) - dispatcher.dispatch(setLoadState(entityType, id, 'success')) + store.batchNotifications(() => { + dispatcher.dispatch(refreshServerData(entityType, id, data)) + dispatcher.dispatch(setLoadState(entityType, id, 'success')) + }) } } catch (error) { if (abortController.signal.aborted) return diff --git a/packages/bindx-react/src/hooks/useEntityList.ts b/packages/bindx-react/src/hooks/useEntityList.ts index 428d9ca..7ec691f 100644 --- a/packages/bindx-react/src/hooks/useEntityList.ts +++ b/packages/bindx-react/src/hooks/useEntityList.ts @@ -96,6 +96,12 @@ function createLoadingListResult(): LoadingEntityListResult { } } +function getRowId(entityType: string, row: Record): string { + const id = row['id'] + if (typeof id !== 'string') throw new Error(`${entityType} list row has no string id`) + return id +} + function createErrorListResult(error: FieldError): ErrorEntityListResult { return { $status: 'error', @@ -456,16 +462,12 @@ export function useEntityList( throw new Error('Unexpected query result type') } + const items = result.data.map(data => ({ id: getRowId(entityType, data), data })) store.batchNotifications(() => { - const items = result.data.map((data: Record) => { - const id = data['id'] as string + for (const item of items) { // Revalidation preserves local edits while advancing the server baseline. - dispatcher.dispatch( - refreshServerData(entityType, id, data), - ) - return { id, data: data as object } - }) - + dispatcher.dispatch(refreshServerData(entityType, item.id, item.data)) + } listStateRef.current = { status: 'ready', items, isRefetching: false } versionRef.current++ store.notify() diff --git a/packages/bindx/src/core/EntityLoader.ts b/packages/bindx/src/core/EntityLoader.ts index 30057b8..7c758fa 100644 --- a/packages/bindx/src/core/EntityLoader.ts +++ b/packages/bindx/src/core/EntityLoader.ts @@ -124,13 +124,13 @@ export class EntityLoader { } } - // Store each entity in snapshot store - for (const item of result.data) { - const record = item as Record - if (typeof record['id'] === 'string') { - this.store.setEntityData(entityType, record['id'], record, true) + this.store.batchNotifications(() => { + for (const record of result.data) { + if (typeof record['id'] === 'string') { + this.store.setEntityData(entityType, record['id'], record, true) + } } - } + }) return { status: 'success', data: result.data as T[] } } catch (error) { diff --git a/packages/bindx/src/persistence/BatchPersister.ts b/packages/bindx/src/persistence/BatchPersister.ts index 56b04f5..cb54052 100644 --- a/packages/bindx/src/persistence/BatchPersister.ts +++ b/packages/bindx/src/persistence/BatchPersister.ts @@ -327,16 +327,18 @@ export class BatchPersister { } const transactionResult = await this.executeTransaction(execution, options?.signal) - result = this.processExecutionResult(execution, transactionResult, options) + result = this.store.batchNotifications(() => this.processExecutionResult(execution, transactionResult, options)) result = this.mergeCancelled(result, execution.vetoed) return result } finally { - this.releaseEntities([...claimed.values()]) - this.undoManager?.unblock() - if (result?.success) { - this.store.sweepUnreachableCreated() - } + this.store.batchNotifications(() => { + this.releaseEntities([...claimed.values()]) + this.undoManager?.unblock() + if (result?.success) { + this.store.sweepUnreachableCreated() + } + }) } // Unreachable, but keeps the return type explicit when control-flow analysis changes. @@ -991,11 +993,13 @@ export class BatchPersister { const fresh = entities.filter(entity => !claimed.has(entityIdentityKey(entity.entityType, entity.entityId))) if (fresh.length === 0) return this.changeRegistry.markInFlight(fresh) - for (const entity of fresh) { - claimed.set(entityIdentityKey(entity.entityType, entity.entityId), entity) - this.dispatcher.dispatch(setPersisting(entity.entityType, entity.entityId, true, updateMode === 'pessimistic')) - this.dispatcher.dispatch(clearAllServerErrors(entity.entityType, entity.entityId)) - } + this.store.batchNotifications(() => { + for (const entity of fresh) { + claimed.set(entityIdentityKey(entity.entityType, entity.entityId), entity) + this.dispatcher.dispatch(setPersisting(entity.entityType, entity.entityId, true, updateMode === 'pessimistic')) + this.dispatcher.dispatch(clearAllServerErrors(entity.entityType, entity.entityId)) + } + }) } private releaseEntities(entities: readonly DirtyEntity[]): void { diff --git a/tests/react/dataview/hasManyDataGridNotificationBatch.test.tsx b/tests/react/dataview/hasManyDataGridNotificationBatch.test.tsx new file mode 100644 index 0000000..5752893 --- /dev/null +++ b/tests/react/dataview/hasManyDataGridNotificationBatch.test.tsx @@ -0,0 +1,72 @@ +import '../../setup' +import { afterEach, expect, test } from 'bun:test' +import { cleanup, render, waitFor } from '@testing-library/react' +import React from 'react' +import { + BindxProvider, + Entity, + MockAdapter, + SnapshotStore, + defineSchema, + entityDef, + hasMany, + scalar, +} from '@contember/bindx-react' +import { DataGridTextColumn, HasManyDataGrid } from '@contember/bindx-dataview' +import { queryByTestId, TestTable } from './helpers.js' + +afterEach(cleanup) + +interface Article { + id: string + title: string +} + +interface Author { + id: string + articles: Article[] +} + +const schema = defineSchema<{ Article: Article; Author: Author }>({ + entities: { + Article: { fields: { id: scalar(), title: scalar() } }, + Author: { fields: { id: scalar(), articles: hasMany('Article') } }, + }, +}) +const authorDef = entityDef('Author') + +async function countNotificationsUntilLoaded(rowCount: number): Promise { + const articles = Array.from({ length: rowCount }, (_, i) => ({ id: `a${i}`, title: `Article ${i}` })) + const adapter = new MockAdapter({ + Article: Object.fromEntries(articles.map(article => [article.id, article])), + Author: { author: { id: 'author', articles } }, + }, { delay: 0 }) + const store = new SnapshotStore() + let notifications = 0 + store.subscribe(() => { notifications++ }) + + const { container } = render( + + + {author => ( + + {it => ( + <> + + + + )} + + )} + + , + ) + await waitFor(() => expect(queryByTestId(container, 'datagrid-table')).not.toBeNull()) + cleanup() + return notifications +} + +test('loading a has-many grid page notifies independently of its row count', async () => { + const single = await countNotificationsUntilLoaded(1) + expect(await countNotificationsUntilLoaded(30)).toBe(single) +}) diff --git a/tests/react/hooks/useEntity/batchNotifications.test.tsx b/tests/react/hooks/useEntity/batchNotifications.test.tsx new file mode 100644 index 0000000..63d067a --- /dev/null +++ b/tests/react/hooks/useEntity/batchNotifications.test.tsx @@ -0,0 +1,42 @@ +import '../../../setup' +import { afterEach, expect, test } from 'bun:test' +import { cleanup, render, waitFor } from '@testing-library/react' +import React from 'react' +import { + BindxProvider, MockAdapter, SnapshotStore, defineSchema, entityDef, scalar, + useEntity, +} from '@contember/bindx-react' + +afterEach(cleanup) + +interface Article { + id: string + title: string +} + +const schema = defineSchema<{ Article: Article }>({ + entities: { Article: { fields: { id: scalar(), title: scalar() } } }, +}) +const articleDef = entityDef
('Article') + +test('a loaded entity is published together with its success state', async () => { + const store = new SnapshotStore() + const observations: string[] = [] + store.subscribe(() => { + const title = store.getEntitySnapshot
('Article', 'a')?.data.title ?? '-' + observations.push(`${store.getLoadState('Article', 'a')?.status}:${title}`) + }) + + function Detail(): React.JSX.Element { + const article = useEntity(articleDef, { by: { id: 'a' } }, e => e.title()) + return
{article.$isLoading ? 'Loading' : `Title ${article.$isError || article.$isNotFound ? '-' : article.title.value}`}
+ } + + const view = render( + + + , + ) + await waitFor(() => expect(view.getByText('Title A')).toBeDefined()) + expect(observations).toEqual(['loading:-', 'success:A']) +}) diff --git a/tests/react/hooks/useEntityList/batchNotifications.test.tsx b/tests/react/hooks/useEntityList/batchNotifications.test.tsx index b2905d8..0289ff9 100644 --- a/tests/react/hooks/useEntityList/batchNotifications.test.tsx +++ b/tests/react/hooks/useEntityList/batchNotifications.test.tsx @@ -1,10 +1,10 @@ import '../../../setup' import { afterEach, expect, test } from 'bun:test' -import { cleanup, render, waitFor } from '@testing-library/react' -import React, { useLayoutEffect } from 'react' +import { act, cleanup, render, waitFor } from '@testing-library/react' +import React, { useState } from 'react' import { - BindxProvider, MockAdapter, defineSchema, entityDef, scalar, - useEntityList, useSnapshotStore, + BindxProvider, MockAdapter, SnapshotStore, defineSchema, entityDef, scalar, + useEntityList, } from '@contember/bindx-react' afterEach(cleanup) @@ -18,31 +18,68 @@ const schema = defineSchema<{ Article: Article }>({ entities: { Article: { fields: { id: scalar(), title: scalar() } } }, }) const articleDef = entityDef
('Article') +const count = 200 + +function createRows(): Record { + return Object.fromEntries(Array.from({ length: count }, (_, i) => [String(i), { id: String(i), title: `Row ${i}` }])) +} test('a list response notifies global and entity subscribers only after all rows are loaded', async () => { - const count = 200 - const rows = Object.fromEntries(Array.from({ length: count }, (_, i) => [String(i), { id: String(i), title: `Row ${i}` }])) - const adapter = new MockAdapter({ Article: rows }, { delay: 0 }) - const observedCounts: number[] = [] - let entityCalls = 0 + const rows = createRows() + const store = new SnapshotStore() + const readLoaded = (): number => Object.keys(rows).filter(id => store.hasEntity('Article', id)).length + const globalObservations: number[] = [] + const entityObservations: number[] = [] + store.subscribe(() => globalObservations.push(readLoaded())) + store.subscribeToEntity('Article', '0', () => entityObservations.push(readLoaded())) function List(): React.JSX.Element { - const store = useSnapshotStore() - useLayoutEffect(() => { - const readCount = (): number => Object.keys(rows).filter(id => store.hasEntity('Article', id)).length - const unsubscribe = store.subscribe(() => observedCounts.push(readCount())) - const unsubscribeEntity = store.subscribeToEntity('Article', '0', () => { - entityCalls++ - expect(readCount()).toBe(count) - }) - return () => { unsubscribe(); unsubscribeEntity() } - }, [store]) const articles = useEntityList(articleDef, {}, a => a.id().title()) return
{articles.$status === 'ready' ? `Loaded ${articles.items.length}` : 'Loading'}
} - const view = render() + const view = render( + + + , + ) await waitFor(() => expect(view.getByText(`Loaded ${count}`)).toBeDefined()) - expect(observedCounts.filter(value => value > 0)).toEqual([count]) - expect(entityCalls).toBe(1) + expect(globalObservations.filter(value => value > 0)).toEqual([count]) + expect(entityObservations).toEqual([count]) +}) + +test('a refetch publishes all refreshed rows to mounted row subscribers at once', async () => { + const rows = createRows() + const store = new SnapshotStore() + let setQueryKey: (key: string) => void = () => {} + + function List(): React.JSX.Element { + const [queryKey, setKey] = useState('initial') + setQueryKey = setKey + const articles = useEntityList(articleDef, { queryKey }, a => a.id().title()) + if (articles.$status !== 'ready' || articles.$isRefetching) return
Loading
+ return
{`Last ${articles.items[count - 1]?.title.value}`}
+ } + + const view = render( + + + , + ) + await waitFor(() => expect(view.getByText(`Last Row ${count - 1}`)).toBeDefined()) + + for (const row of Object.values(rows)) row.title = `New ${row.id}` + const readRefreshed = (): number => Object.keys(rows) + .filter(id => store.getEntitySnapshot
('Article', id)?.data.title === `New ${id}`) + .length + const rowObservations: number[] = [] + const globalObservations: number[] = [] + store.subscribeToEntity('Article', '0', () => rowObservations.push(readRefreshed())) + store.subscribeToEntity('Article', String(count - 1), () => rowObservations.push(readRefreshed())) + store.subscribe(() => globalObservations.push(readRefreshed())) + + act(() => setQueryKey('refetch')) + await waitFor(() => expect(view.getByText(`Last New ${count - 1}`)).toBeDefined()) + expect(rowObservations).toEqual([count, count]) + expect(globalObservations.filter(value => value > 0)).toEqual([count]) }) diff --git a/tests/unit/persistence/persistNotificationBatch.test.ts b/tests/unit/persistence/persistNotificationBatch.test.ts new file mode 100644 index 0000000..b6e1ebb --- /dev/null +++ b/tests/unit/persistence/persistNotificationBatch.test.ts @@ -0,0 +1,53 @@ +import { expect, test } from 'bun:test' +import { + ActionDispatcher, + BatchPersister, + MockAdapter, + SnapshotStore, + buildQuery, + createEntityLoader, + type BackendAdapter, +} from '@contember/bindx' + +interface Article { + id: string + title: string +} + +async function countPersistNotifications(entityCount: number): Promise { + let createdCount = 0 + const adapter: BackendAdapter = { + query: () => Promise.resolve([]), + persist: () => Promise.resolve({ ok: true }), + create: (_entityType, data) => Promise.resolve({ ok: true, data: { ...data, id: `persisted-${++createdCount}` } }), + delete: () => Promise.resolve({ ok: true }), + } + const store = new SnapshotStore() + const persister = new BatchPersister(adapter, store, new ActionDispatcher(store)) + for (let i = 0; i < entityCount; i++) { + store.setEntityData('Article', `a${i}`, { id: `a${i}`, title: 'Original' }, true) + store.setFieldValue('Article', `a${i}`, ['title'], 'Updated') + store.createEntity('Article', { title: `Draft ${i}` }) + } + let notifications = 0 + store.subscribe(() => { notifications++ }) + const result = await persister.persistAll() + expect(result.success).toBe(true) + return notifications +} + +test('a persist notifies independently of how many entities it saves', async () => { + const single = await countPersistNotifications(1) + expect(await countPersistNotifications(20)).toBe(single) +}) + +test('loadMany notifies once for the whole list', async () => { + const rows = Object.fromEntries(Array.from({ length: 20 }, (_, i) => [`a${i}`, { id: `a${i}`, title: `Row ${i}` }])) + const store = new SnapshotStore() + const loader = createEntityLoader(new MockAdapter({ Article: rows }, { delay: 0 }), store) + let notifications = 0 + store.subscribe(() => { notifications++ }) + const result = await loader.loadMany({ entityType: 'Article', query: buildQuery(e => e.id().title()) }) + expect(result.status).toBe('success') + expect(notifications).toBe(1) +})