From 577a2a32abd7620a63831f0f11081daddfa257e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jind=C5=99ich=20Krupka?= Date: Thu, 24 Sep 2026 18:04:45 +0200 Subject: [PATCH 1/7] test: failing repro for a narrower nested selection of the same entity overwriting the wider one --- ...nestedSameEntityNarrowerSelection.test.tsx | 148 ++++++++++++++++++ 1 file changed, 148 insertions(+) create mode 100644 tests/react/relations/hasMany/nestedSameEntityNarrowerSelection.test.tsx diff --git a/tests/react/relations/hasMany/nestedSameEntityNarrowerSelection.test.tsx b/tests/react/relations/hasMany/nestedSameEntityNarrowerSelection.test.tsx new file mode 100644 index 00000000..9804b8ec --- /dev/null +++ b/tests/react/relations/hasMany/nestedSameEntityNarrowerSelection.test.tsx @@ -0,0 +1,148 @@ +// Regression test for +import '../../../setup' +import { afterEach, describe, expect, test } from 'bun:test' +import { cleanup, render, waitFor } from '@testing-library/react' +import React from 'react' +import { BindxProvider, defineSchema, entityDef, hasMany, hasOne, MockAdapter, scalar, useEntity } from '@contember/bindx-react' +import { getByTestId, queryByTestId } from './setup' + +afterEach(() => { + cleanup() +}) + +interface Attachment { + id: string + name: string + type: string +} + +interface Meeting { + id: string + session: Session | null +} + +interface Session { + id: string + attachments: Attachment[] + meetings: Meeting[] +} + +interface CycleSchema { + Session: Session + Meeting: Meeting + Attachment: Attachment +} + +const schema = defineSchema({ + entities: { + Session: { + fields: { + id: scalar(), + attachments: hasMany('Attachment'), + meetings: hasMany('Meeting'), + }, + }, + Meeting: { + fields: { + id: scalar(), + session: hasOne('Session', { nullable: true }), + }, + }, + Attachment: { + fields: { + id: scalar(), + name: scalar(), + type: scalar(), + }, + }, + }, +}) + +const entityDefs = { + Session: entityDef('Session'), +} as const + +function createMockData() { + const attachments = [{ id: 'att-1', name: 'Slides', type: 'learningMaterial' }] + return { + Session: { + 'session-1': { + id: 'session-1', + attachments, + // The meeting points back at the SAME session — a cycle the page selects + // through a second component with a narrower attachment selection. + meetings: [{ id: 'meeting-1', session: { id: 'session-1', attachments } }], + }, + }, + Meeting: {}, + Attachment: {}, + } +} + +/** + * One root entity whose selection reaches the same `Session` twice: directly with + * `attachments { name type }`, and through `meetings.session` with `attachments { name }`. + * `readNestedFirst` mirrors two sibling components where the one holding the narrower + * selection renders first. + */ +function SessionView({ readNestedFirst }: { readNestedFirst: boolean }): React.ReactElement { + const session = useEntity(entityDefs.Session, { by: { id: 'session-1' } }, e => + e.id() + .attachments(a => a.id().name().type()) + .meetings(m => m.id().session(s => s.id().attachments(a => a.id().name()))), + ) + + if (session.$isLoading || session.$isError || session.$isNotFound) { + return
Loading...
+ } + + const nestedNames = () => session.meetings.items.flatMap(m => m.session.attachments.items.map(a => a.$fields.name.value)).join(',') + const directTypes = () => session.attachments.items.map(a => String(a.$fields.type.value)).join(',') + + const nested = readNestedFirst ? nestedNames() : '' + const types = directTypes() + const nestedAfter = readNestedFirst ? nested : nestedNames() + + return ( +
+ {nestedAfter} + {types} +
+ ) +} + +describe('HasMany - the same entity reached twice with different selections', () => { + test('should keep the wider selection\'s fields when the narrower nested occurrence is read first', async () => { + const adapter = new MockAdapter(createMockData(), { delay: 0 }) + + const { container } = render( + + + , + ) + + await waitFor(() => { + expect(queryByTestId(container, 'direct-types')).not.toBeNull() + }) + + expect(getByTestId(container, 'nested-names').textContent).toBe('Slides') + expect(getByTestId(container, 'direct-types').textContent).toBe('learningMaterial') + }) + + test('should keep the wider selection\'s fields when the direct occurrence is read first', async () => { + const adapter = new MockAdapter(createMockData(), { delay: 0 }) + + const { container } = render( + + + , + ) + + await waitFor(() => { + expect(queryByTestId(container, 'direct-types')).not.toBeNull() + }) + + expect(getByTestId(container, 'nested-names').textContent).toBe('Slides') + expect(getByTestId(container, 'direct-types').textContent).toBe('learningMaterial') + }) +}) From 929ecaf2a892774cdb12636c29e318f860b921b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jind=C5=99ich=20Krupka?= Date: Thu, 24 Sep 2026 18:05:38 +0200 Subject: [PATCH 2/7] test: link the regression test to its issue --- .../hasMany/nestedSameEntityNarrowerSelection.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/react/relations/hasMany/nestedSameEntityNarrowerSelection.test.tsx b/tests/react/relations/hasMany/nestedSameEntityNarrowerSelection.test.tsx index 9804b8ec..547df96a 100644 --- a/tests/react/relations/hasMany/nestedSameEntityNarrowerSelection.test.tsx +++ b/tests/react/relations/hasMany/nestedSameEntityNarrowerSelection.test.tsx @@ -1,4 +1,4 @@ -// Regression test for +// Regression test for https://github.com/contember/bindx/issues/123 import '../../../setup' import { afterEach, describe, expect, test } from 'bun:test' import { cleanup, render, waitFor } from '@testing-library/react' From 1309b84d7077912a3058c96378cddb3d2ce57300 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Mon, 28 Sep 2026 11:17:27 +0200 Subject: [PATCH 3/7] fix(bindx): merge embedded relation data on a server refresh instead of replacing it A query can reach one entity through several paths, each with its own sub-selection of a relation, and every path refreshes the entity's snapshot. refreshServerData assigned each incoming field wholesale, so the occurrence read last with a narrower sub-selection dropped the fields a wider one had fetched, and they read back as undefined. The same happened when two roots loaded one entity with different selections. refreshServerData now takes the selection the data was read with and merges embedded relations into the stored values: a has-many takes its membership and order from the read and merges items by id, a has-one merges while it points to the same entity and is replaced otherwise. Scalars, including object-valued JSON columns, are still replaced; the selection is what tells them apart. Without a selection nothing changes. Closes #123 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Ge3kRufpi9b9HMBSBjRze5 --- packages/bindx-react/src/hooks/useEntity.ts | 2 +- .../bindx-react/src/hooks/useEntityList.ts | 2 +- packages/bindx/src/core/ActionDispatcher.ts | 2 + packages/bindx/src/core/actions.ts | 6 +- .../bindx/src/handles/HasManyListHandle.ts | 1 + packages/bindx/src/handles/HasOneHandle.ts | 1 + .../bindx/src/store/EntitySnapshotStore.ts | 10 +- packages/bindx/src/store/SnapshotStore.ts | 4 +- .../bindx/src/store/embeddedRelationMerge.ts | 76 +++++++++ ...nestedSameEntityNarrowerSelection.test.tsx | 37 +++++ .../refreshEmbeddedRelationMerge.test.ts | 148 ++++++++++++++++++ 11 files changed, 284 insertions(+), 5 deletions(-) create mode 100644 packages/bindx/src/store/embeddedRelationMerge.ts create mode 100644 tests/unit/store/refreshEmbeddedRelationMerge.test.ts diff --git a/packages/bindx-react/src/hooks/useEntity.ts b/packages/bindx-react/src/hooks/useEntity.ts index 97294e66..2811ea27 100644 --- a/packages/bindx-react/src/hooks/useEntity.ts +++ b/packages/bindx-react/src/hooks/useEntity.ts @@ -294,7 +294,7 @@ export function useEntity( // Revalidation: advance the server baseline but keep local dirty // edits intact (see EntitySnapshotStore.refreshServerData). store.batchNotifications(() => { - dispatcher.dispatch(refreshServerData(entityType, id, data)) + dispatcher.dispatch(refreshServerData(entityType, id, data, selectionMeta)) dispatcher.dispatch(setLoadState(entityType, id, 'success')) }) } diff --git a/packages/bindx-react/src/hooks/useEntityList.ts b/packages/bindx-react/src/hooks/useEntityList.ts index 7ec691f2..6719b19b 100644 --- a/packages/bindx-react/src/hooks/useEntityList.ts +++ b/packages/bindx-react/src/hooks/useEntityList.ts @@ -466,7 +466,7 @@ export function useEntityList( store.batchNotifications(() => { for (const item of items) { // Revalidation preserves local edits while advancing the server baseline. - dispatcher.dispatch(refreshServerData(entityType, item.id, item.data)) + dispatcher.dispatch(refreshServerData(entityType, item.id, item.data, selectionMeta)) } listStateRef.current = { status: 'ready', items, isRefetching: false } versionRef.current++ diff --git a/packages/bindx/src/core/ActionDispatcher.ts b/packages/bindx/src/core/ActionDispatcher.ts index 7945d6ce..3e297b0f 100644 --- a/packages/bindx/src/core/ActionDispatcher.ts +++ b/packages/bindx/src/core/ActionDispatcher.ts @@ -243,6 +243,8 @@ export class ActionDispatcher { action.entityType, action.entityId, action.data, + false, + action.selection, ) break diff --git a/packages/bindx/src/core/actions.ts b/packages/bindx/src/core/actions.ts index d48da813..8a33d862 100644 --- a/packages/bindx/src/core/actions.ts +++ b/packages/bindx/src/core/actions.ts @@ -1,5 +1,6 @@ import type { HasOneRelationState } from '../handles/types.js' import type { FieldError, FieldErrorFilter } from '../errors/types.js' +import type { SelectionMeta } from '../selection/types.js' /** * Action types for the ActionDispatcher. @@ -58,6 +59,8 @@ export interface RefreshServerDataAction { readonly entityType: string readonly entityId: string readonly data: Record + /** The selection `data` was read with; lets embedded relations merge instead of being replaced. */ + readonly selection?: SelectionMeta } // ==================== Relation Actions ==================== @@ -376,8 +379,9 @@ export function refreshServerData( entityType: string, entityId: string, data: Record, + selection?: SelectionMeta, ): RefreshServerDataAction { - return { type: 'REFRESH_SERVER_DATA', entityType, entityId, data } + return { type: 'REFRESH_SERVER_DATA', entityType, entityId, data, selection } } /** diff --git a/packages/bindx/src/handles/HasManyListHandle.ts b/packages/bindx/src/handles/HasManyListHandle.ts index 1f8984aa..36730ac0 100644 --- a/packages/bindx/src/handles/HasManyListHandle.ts +++ b/packages/bindx/src/handles/HasManyListHandle.ts @@ -308,6 +308,7 @@ export class HasManyListHandle id, embeddedData as Record, true, // skipNotify - called during render, data already exists embedded in parent + this.selection, ) this.store.markEmbeddedDataPropagated(this.entityType, this.entityId, this.dataFieldName, embeddedData) } diff --git a/packages/bindx/src/store/EntitySnapshotStore.ts b/packages/bindx/src/store/EntitySnapshotStore.ts index f60d45fc..7c22dba2 100644 --- a/packages/bindx/src/store/EntitySnapshotStore.ts +++ b/packages/bindx/src/store/EntitySnapshotStore.ts @@ -3,6 +3,8 @@ import { type EntitySnapshot, } from './snapshots.js' import type { RekeyContext, Rekeyable } from './RekeyOrchestrator.js' +import type { SelectionMeta } from '../selection/types.js' +import { mergeEmbeddedRelationFields } from './embeddedRelationMerge.js' /** * Manages entity snapshots — core CRUD for immutable entity data. @@ -143,12 +145,18 @@ export class EntitySnapshotStore implements Rekeyable { * "differs from the new server value" after the refresh. * * When no snapshot exists yet this behaves like a plain server load. + * + * Given the selection the data was read with, embedded relation values are + * merged into the stored ones rather than replacing them, so a narrower read + * of the same entity keeps what a wider one fetched + * (see {@link mergeEmbeddedRelationFields}). */ refreshServerData( key: string, id: string, entityType: string, data: T, + selection?: SelectionMeta, ): EntitySnapshot { const existing = this.snapshots.get(key) if (!existing) { @@ -157,7 +165,7 @@ export class EntitySnapshotStore implements Rekeyable { const prevData = existing.data as Record const prevServer = (existing.serverData ?? existing.data) as Record - const incoming = data as Record + const incoming = mergeEmbeddedRelationFields(prevServer, data as Record, selection) const newServerData: Record = { ...prevServer } const newData: Record = { ...prevData } diff --git a/packages/bindx/src/store/SnapshotStore.ts b/packages/bindx/src/store/SnapshotStore.ts index b5a2ca9a..66a2d401 100644 --- a/packages/bindx/src/store/SnapshotStore.ts +++ b/packages/bindx/src/store/SnapshotStore.ts @@ -1,6 +1,7 @@ import type { EntitySnapshot, LoadStatus } from './snapshots.js' import { createEntitySnapshot } from './snapshots.js' import type { FieldError, FieldErrorFilter } from '../errors/types.js' +import type { SelectionMeta } from '../selection/types.js' import { SubscriptionManager, type SnapshotVersionBumper, type SynchronousResult } from './SubscriptionManager.js' import { ErrorStore } from './ErrorStore.js' import { @@ -318,9 +319,10 @@ export class SnapshotStore implements SnapshotVersionBumper, JournalTarget { id: string, data: T, skipNotify: boolean = false, + selection?: SelectionMeta, ): EntitySnapshot { const key = this.getEntityKey(entityType, id) - const newSnapshot = this.entitySnapshots.refreshServerData(key, this.resolveEntityId(entityType, id), entityType, data) + const newSnapshot = this.entitySnapshots.refreshServerData(key, this.resolveEntityId(entityType, id), entityType, data, selection) this.meta.setExistsOnServer(key, true) if (!skipNotify) { this.notifyEntitySubscribers(key) diff --git a/packages/bindx/src/store/embeddedRelationMerge.ts b/packages/bindx/src/store/embeddedRelationMerge.ts new file mode 100644 index 00000000..d11b3bf3 --- /dev/null +++ b/packages/bindx/src/store/embeddedRelationMerge.ts @@ -0,0 +1,76 @@ +import type { SelectionFieldMeta, SelectionMeta } from '../selection/types.js' + +/** + * Merges a server read of an entity's embedded relation data into what the store + * already holds for it. + * + * One query can reach the same entity through several paths, each with its own + * sub-selection of a relation, and each path refreshes the entity's snapshot. + * Assigning the incoming relation value wholesale would let a narrower + * occurrence drop fields a wider one fetched. The server read still decides + * everything it carries: has-many membership and order, which entity a has-one + * points to, and every value it contains. Only keys it did not select survive, + * and only on the same related entity. + * + * The selection tells relations from scalars: a JSON column can hold an object + * with an `id` too, and it must be replaced, never merged. Without a selection, + * the incoming value replaces the stored one. + */ +export function mergeEmbeddedRelationFields( + existing: Record, + incoming: Record, + selection: SelectionMeta | undefined, +): Record { + const merged: Record = {} + for (const key of Object.keys(incoming)) { + const fieldMeta = selection ? findFieldByDataKey(selection, key) : undefined + merged[key] = fieldMeta ? mergeFieldValue(existing[key], incoming[key], fieldMeta) : incoming[key] + } + return merged +} + +// A fluent selection marks `isArray` only for a has-many given params, so the +// relation kind is read from the value: a has-many embeds an array, a has-one an object. +function mergeFieldValue(existing: unknown, incoming: unknown, fieldMeta: SelectionFieldMeta): unknown { + if (!fieldMeta.isRelation || !fieldMeta.nested) { + return incoming + } + if (Array.isArray(existing) && Array.isArray(incoming)) { + return mergeHasManyItems(existing, incoming, fieldMeta.nested) + } + return mergeHasOneEntity(existing, incoming, fieldMeta.nested) +} + +function mergeHasManyItems(existing: readonly unknown[], incoming: readonly unknown[], itemSelection: SelectionMeta): unknown[] { + const existingById = new Map>() + for (const item of existing) { + if (isRecord(item) && item['id'] !== undefined) { + existingById.set(item['id'], item) + } + } + return incoming.map(item => mergeHasOneEntity(isRecord(item) ? existingById.get(item['id']) : undefined, item, itemSelection)) +} + +function mergeHasOneEntity(existing: unknown, incoming: unknown, selection: SelectionMeta): unknown { + if (!isRecord(existing) || !isRecord(incoming) || incoming['id'] === undefined || existing['id'] !== incoming['id']) { + return incoming + } + return { ...existing, ...mergeEmbeddedRelationFields(existing, incoming, selection) } +} + +function findFieldByDataKey(selection: SelectionMeta, dataKey: string): SelectionFieldMeta | undefined { + const byKey = selection.fields.get(dataKey) + if (byKey?.alias === dataKey) { + return byKey + } + for (const fieldMeta of selection.fields.values()) { + if (fieldMeta.alias === dataKey) { + return fieldMeta + } + } + return undefined +} + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value) +} diff --git a/tests/react/relations/hasMany/nestedSameEntityNarrowerSelection.test.tsx b/tests/react/relations/hasMany/nestedSameEntityNarrowerSelection.test.tsx index 547df96a..b0a090ce 100644 --- a/tests/react/relations/hasMany/nestedSameEntityNarrowerSelection.test.tsx +++ b/tests/react/relations/hasMany/nestedSameEntityNarrowerSelection.test.tsx @@ -146,3 +146,40 @@ describe('HasMany - the same entity reached twice with different selections', () expect(getByTestId(container, 'direct-types').textContent).toBe('learningMaterial') }) }) + +function WideRoot(): React.ReactElement { + const session = useEntity(entityDefs.Session, { by: { id: 'session-1' } }, e => e.id().attachments(a => a.id().name().type())) + if (session.$isLoading || session.$isError || session.$isNotFound) { + return
Loading...
+ } + return {session.attachments.items.map(a => String(a.$fields.type.value)).join(',')} +} + +function NarrowRoot(): React.ReactElement { + const session = useEntity(entityDefs.Session, { by: { id: 'session-1' } }, e => e.id().attachments(a => a.id().name())) + if (session.$isLoading || session.$isError || session.$isNotFound) { + return
Loading...
+ } + return {session.attachments.items.map(a => a.$fields.name.value).join(',')} +} + +describe('HasMany - the same entity loaded by two roots with different selections', () => { + test('should keep the wider root\'s fields when the narrower root\'s read lands last', async () => { + const adapter = new MockAdapter(createMockData(), { delay: 0 }) + + const { container } = render( + + + + , + ) + + await waitFor(() => { + expect(queryByTestId(container, 'wide-types')).not.toBeNull() + expect(queryByTestId(container, 'narrow-names')).not.toBeNull() + }) + + expect(getByTestId(container, 'narrow-names').textContent).toBe('Slides') + expect(getByTestId(container, 'wide-types').textContent).toBe('learningMaterial') + }) +}) diff --git a/tests/unit/store/refreshEmbeddedRelationMerge.test.ts b/tests/unit/store/refreshEmbeddedRelationMerge.test.ts new file mode 100644 index 00000000..fab84679 --- /dev/null +++ b/tests/unit/store/refreshEmbeddedRelationMerge.test.ts @@ -0,0 +1,148 @@ +// Regression tests for https://github.com/contember/bindx/issues/123 +import { describe, test, expect, beforeEach } from 'bun:test' +import type { SelectionMeta, SnapshotStore } from '@contember/bindx' +import { __internal } from '@contember/bindx-react' +import { createTestStore } from '../shared/unitTestHelpers.js' + +const { createSelectionBuilder, getSelectionMeta } = __internal + +interface Attachment { + id: string + name: string + type: string +} + +interface Author { + id: string + name: string + email: string +} + +interface Session { + id: string + title: string + settings: { id: string; mode: string; extra?: string } | null + attachments: Attachment[] + author: Author | null +} + +const narrowAttachments = getSelectionMeta(createSelectionBuilder().id().attachments(a => a.id().name())) +const narrowAuthor = getSelectionMeta(createSelectionBuilder().id().author(a => a.id().name())) + +const wideSession = (): Record => ({ + id: 's-1', + title: 'Session', + attachments: [ + { id: 'att-1', name: 'Slides', type: 'learningMaterial' }, + { id: 'att-2', name: 'Notes', type: 'internal' }, + ], + author: { id: 'au-1', name: 'Ann', email: 'ann@example.com' }, +}) + +describe('SnapshotStore.refreshServerData — embedded relations read with a narrower selection', () => { + let store: SnapshotStore + + beforeEach(() => { + store = createTestStore() + store.setEntityData('Session', 's-1', wideSession(), true) + }) + + test('keeps has-many item fields the narrower selection did not ask for', () => { + store.refreshServerData('Session', 's-1', { + id: 's-1', + attachments: [ + { id: 'att-1', name: 'Slides v2' }, + { id: 'att-2', name: 'Notes' }, + ], + }, false, narrowAttachments) + + const snapshot = store.getEntitySnapshot>('Session', 's-1') + const expected = [ + { id: 'att-1', name: 'Slides v2', type: 'learningMaterial' }, + { id: 'att-2', name: 'Notes', type: 'internal' }, + ] + expect(snapshot?.serverData?.['attachments']).toEqual(expected) + expect(snapshot?.data['attachments']).toEqual(expected) + expect(snapshot?.data['title']).toBe('Session') + }) + + test('takes has-many membership and order from the server read', () => { + store.refreshServerData('Session', 's-1', { + id: 's-1', + attachments: [ + { id: 'att-3', name: 'Handout' }, + { id: 'att-1', name: 'Slides' }, + ], + }, false, narrowAttachments) + + const snapshot = store.getEntitySnapshot>('Session', 's-1') + expect(snapshot?.data['attachments']).toEqual([ + { id: 'att-3', name: 'Handout' }, + { id: 'att-1', name: 'Slides', type: 'learningMaterial' }, + ]) + }) + + test('keeps has-one fields the narrower selection did not ask for while the target is the same', () => { + store.refreshServerData('Session', 's-1', { id: 's-1', author: { id: 'au-1', name: 'Anna' } }, false, narrowAuthor) + + const snapshot = store.getEntitySnapshot>('Session', 's-1') + expect(snapshot?.data['author']).toEqual({ id: 'au-1', name: 'Anna', email: 'ann@example.com' }) + }) + + test('replaces a has-one that points to another entity', () => { + store.refreshServerData('Session', 's-1', { id: 's-1', author: { id: 'au-2', name: 'Bob' } }, false, narrowAuthor) + + const snapshot = store.getEntitySnapshot>('Session', 's-1') + expect(snapshot?.data['author']).toEqual({ id: 'au-2', name: 'Bob' }) + }) + + test('replaces a has-one the server read disconnected', () => { + store.refreshServerData('Session', 's-1', { id: 's-1', author: null }, false, narrowAuthor) + + const snapshot = store.getEntitySnapshot>('Session', 's-1') + expect(snapshot?.data['author']).toBeNull() + }) + + test('replaces an object-valued scalar even when it carries an id', () => { + store.setEntityData('Session', 's-1', { ...wideSession(), settings: { id: 'x', mode: 'a', extra: 'stale' } }, true) + // A JSON column: selected as a scalar, although its value is an object with an `id`. + const withSettings: SelectionMeta = { + fields: new Map([ + ['id', { fieldName: 'id', alias: 'id', path: ['id'], isRelation: false, isArray: false }], + ['settings', { fieldName: 'settings', alias: 'settings', path: ['settings'], isRelation: false, isArray: false }], + ]), + } + + store.refreshServerData('Session', 's-1', { id: 's-1', settings: { id: 'x', mode: 'b' } }, false, withSettings) + + const snapshot = store.getEntitySnapshot>('Session', 's-1') + expect(snapshot?.data['settings']).toEqual({ id: 'x', mode: 'b' }) + }) + + test('replaces relation data wholesale when no selection is given', () => { + store.refreshServerData('Session', 's-1', { id: 's-1', attachments: [{ id: 'att-1', name: 'Slides' }] }) + + const snapshot = store.getEntitySnapshot>('Session', 's-1') + expect(snapshot?.data['attachments']).toEqual([{ id: 'att-1', name: 'Slides' }]) + }) + + test('leaves the entity clean and keeps a dirty scalar edit', () => { + store.setFieldValue('Session', 's-1', ['title'], 'Local edit') + + store.refreshServerData('Session', 's-1', { + id: 's-1', + title: 'Server title', + attachments: [{ id: 'att-1', name: 'Slides' }], + }, false, getSelectionMeta(createSelectionBuilder().id().title().attachments(a => a.id().name()))) + + const snapshot = store.getEntitySnapshot>('Session', 's-1') + expect(snapshot?.data['title']).toBe('Local edit') + expect(snapshot?.serverData?.['title']).toBe('Server title') + expect(snapshot?.data['attachments']).toBe(snapshot?.serverData?.['attachments']) + + store.resetEntity('Session', 's-1') + const reset = store.getEntitySnapshot>('Session', 's-1') + expect(reset?.data['title']).toBe('Server title') + expect(reset?.data['attachments']).toEqual([{ id: 'att-1', name: 'Slides', type: 'learningMaterial' }]) + }) +}) From 432f5d9d23cc18b240884cb7632371889d1131ba Mon Sep 17 00:00:00 2001 From: David Matejka Date: Mon, 28 Sep 2026 14:59:05 +0200 Subject: [PATCH 4/7] fix(bindx): fill unread relation keys from the related entity's own snapshot The embedded copy of a relation is written only when its parent is read, so after a persist, or a read of the related entity through another path, it lags behind the related entity's snapshot. Filling the keys a narrower read did not select from that copy pushed the stale values back into the related snapshot, in data and server baseline alike, and the entity looked clean with the old value. A kept key now takes its value from the related entity's own snapshot when the store has one, and from the embedded copy only for an entity nothing has materialized yet. Existing related snapshots therefore end up exactly as they did before the merge existed; only the keys they lack are filled. Also: - connections ({ edges: [{ node }] }) merge their nodes by id - the non-enumerable totalCount of a paginated has-many survives a merge - a merge that keeps nothing returns the incoming value itself, and items are matched by position before an id index is built Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Ge3kRufpi9b9HMBSBjRze5 --- .../bindx/src/store/EntitySnapshotStore.ts | 9 +- .../bindx/src/store/embeddedRelationMerge.ts | 145 +++++++--- ...nestedSameEntityNarrowerSelection.test.tsx | 266 ++++++++++++------ .../refreshEmbeddedRelationMerge.test.ts | 162 ++++++++--- 4 files changed, 434 insertions(+), 148 deletions(-) diff --git a/packages/bindx/src/store/EntitySnapshotStore.ts b/packages/bindx/src/store/EntitySnapshotStore.ts index 7c22dba2..ac1f37cb 100644 --- a/packages/bindx/src/store/EntitySnapshotStore.ts +++ b/packages/bindx/src/store/EntitySnapshotStore.ts @@ -134,6 +134,13 @@ export class EntitySnapshotStore implements Rekeyable { return newSnapshot } + private readonly lookupServerData = (id: string): Readonly> | undefined => { + const key = this.idIndex.get(id) + const snapshot = key === undefined ? undefined : this.snapshots.get(key) + if (!snapshot) return undefined + return (snapshot.serverData ?? snapshot.data) as Readonly> + } + /** * Refreshes entity data from a fresh server read (revalidation). * @@ -165,7 +172,7 @@ export class EntitySnapshotStore implements Rekeyable { const prevData = existing.data as Record const prevServer = (existing.serverData ?? existing.data) as Record - const incoming = mergeEmbeddedRelationFields(prevServer, data as Record, selection) + const incoming = mergeEmbeddedRelationFields(prevServer, data as Record, selection, this.lookupServerData) const newServerData: Record = { ...prevServer } const newData: Record = { ...prevData } diff --git a/packages/bindx/src/store/embeddedRelationMerge.ts b/packages/bindx/src/store/embeddedRelationMerge.ts index d11b3bf3..dd293de5 100644 --- a/packages/bindx/src/store/embeddedRelationMerge.ts +++ b/packages/bindx/src/store/embeddedRelationMerge.ts @@ -1,4 +1,7 @@ -import type { SelectionFieldMeta, SelectionMeta } from '../selection/types.js' +import type { SelectionMeta } from '../selection/types.js' + +/** The server baseline the store holds for an entity of this id, if it has a snapshot of it. */ +export type StoredServerDataLookup = (id: string) => Readonly> | undefined /** * Merges a server read of an entity's embedded relation data into what the store @@ -9,66 +12,142 @@ import type { SelectionFieldMeta, SelectionMeta } from '../selection/types.js' * Assigning the incoming relation value wholesale would let a narrower * occurrence drop fields a wider one fetched. The server read still decides * everything it carries: has-many membership and order, which entity a has-one - * points to, and every value it contains. Only keys it did not select survive, - * and only on the same related entity. + * points to, and every value it contains. + * + * A key the read did not select is kept only on the same related entity, and + * its value comes from that entity's own snapshot when the store has one. The + * embedded copy is written only when its parent is read, so after a persist or + * a read through another path it lags behind the snapshot; it is the fallback + * for an entity nothing has materialized yet. * * The selection tells relations from scalars: a JSON column can hold an object * with an `id` too, and it must be replaced, never merged. Without a selection, * the incoming value replaces the stored one. + * + * Returns `incoming` itself when there is nothing to keep, so the common case + * allocates nothing. */ export function mergeEmbeddedRelationFields( - existing: Record, + existing: Readonly>, incoming: Record, selection: SelectionMeta | undefined, + lookup: StoredServerDataLookup, ): Record { - const merged: Record = {} - for (const key of Object.keys(incoming)) { - const fieldMeta = selection ? findFieldByDataKey(selection, key) : undefined - merged[key] = fieldMeta ? mergeFieldValue(existing[key], incoming[key], fieldMeta) : incoming[key] + if (!selection) { + return incoming } - return merged + let merged: Record | undefined + for (const fieldMeta of selection.fields.values()) { + const key = fieldMeta.alias + if (!fieldMeta.isRelation || !fieldMeta.nested || !Object.hasOwn(incoming, key)) { + continue + } + const value = mergeRelationValue(existing[key], incoming[key], fieldMeta.nested, lookup) + if (value !== incoming[key]) { + merged ??= { ...incoming } + merged[key] = value + } + } + return merged ?? incoming } // A fluent selection marks `isArray` only for a has-many given params, so the -// relation kind is read from the value: a has-many embeds an array, a has-one an object. -function mergeFieldValue(existing: unknown, incoming: unknown, fieldMeta: SelectionFieldMeta): unknown { - if (!fieldMeta.isRelation || !fieldMeta.nested) { - return incoming - } +// relation kind is read from the value: a has-many embeds an array (or a +// connection), a has-one an object. +function mergeRelationValue(existing: unknown, incoming: unknown, nested: SelectionMeta, lookup: StoredServerDataLookup): unknown { if (Array.isArray(existing) && Array.isArray(incoming)) { - return mergeHasManyItems(existing, incoming, fieldMeta.nested) + return mergeItems(existing, incoming, nested, lookup) } - return mergeHasOneEntity(existing, incoming, fieldMeta.nested) + if (isConnection(existing) && isConnection(incoming)) { + return mergeConnection(existing, incoming, nested, lookup) + } + return mergeRelatedEntity(existing, incoming, nested, lookup) } -function mergeHasManyItems(existing: readonly unknown[], incoming: readonly unknown[], itemSelection: SelectionMeta): unknown[] { - const existingById = new Map>() - for (const item of existing) { - if (isRecord(item) && item['id'] !== undefined) { - existingById.set(item['id'], item) +function mergeItems(existing: readonly unknown[], incoming: unknown[], itemSelection: SelectionMeta, lookup: StoredServerDataLookup): unknown[] { + let existingById: Map> | undefined + const findExisting = (item: unknown, index: number): Record | undefined => { + if (!isRecord(item)) return undefined + // A re-read usually keeps the order, so the item at the same position is checked before building an index. + const atIndex = existing[index] + if (isRecord(atIndex) && atIndex['id'] === item['id']) return atIndex + existingById ??= indexById(existing) + return existingById.get(item['id']) + } + let merged: unknown[] | undefined + incoming.forEach((item, index) => { + const value = mergeRelatedEntity(findExisting(item, index), item, itemSelection, lookup) + if (value !== item) { + merged ??= [...incoming] + merged[index] = value } + }) + if (!merged) { + return incoming } - return incoming.map(item => mergeHasOneEntity(isRecord(item) ? existingById.get(item['id']) : undefined, item, itemSelection)) + copyTotalCount(incoming, merged) + return merged } -function mergeHasOneEntity(existing: unknown, incoming: unknown, selection: SelectionMeta): unknown { - if (!isRecord(existing) || !isRecord(incoming) || incoming['id'] === undefined || existing['id'] !== incoming['id']) { +interface Connection extends Record { + readonly edges: readonly unknown[] +} + +function mergeConnection(existing: Connection, incoming: Connection, nodeSelection: SelectionMeta, lookup: StoredServerDataLookup): Connection { + const existingNodes = existing.edges.map(edge => (isRecord(edge) ? edge['node'] : undefined)) + const incomingNodes = incoming.edges.map(edge => (isRecord(edge) ? edge['node'] : undefined)) + const mergedNodes = mergeItems(existingNodes, incomingNodes, nodeSelection, lookup) + if (mergedNodes === incomingNodes) { return incoming } - return { ...existing, ...mergeEmbeddedRelationFields(existing, incoming, selection) } + const edges = incoming.edges.map((edge, index) => (isRecord(edge) ? { ...edge, node: mergedNodes[index] } : edge)) + return { ...incoming, edges } } -function findFieldByDataKey(selection: SelectionMeta, dataKey: string): SelectionFieldMeta | undefined { - const byKey = selection.fields.get(dataKey) - if (byKey?.alias === dataKey) { - return byKey +function mergeRelatedEntity(existing: unknown, incoming: unknown, selection: SelectionMeta, lookup: StoredServerDataLookup): unknown { + if (existing === incoming || !isRecord(existing) || !isRecord(incoming)) { + return incoming } - for (const fieldMeta of selection.fields.values()) { - if (fieldMeta.alias === dataKey) { - return fieldMeta + const id = incoming['id'] + if (id === undefined || existing['id'] !== id) { + return incoming + } + const withRelations = mergeEmbeddedRelationFields(existing, incoming, selection, lookup) + let merged: Record | undefined + let stored: Readonly> | undefined + for (const key of Object.keys(existing)) { + if (Object.hasOwn(incoming, key)) { + continue + } + if (!merged) { + merged = { ...withRelations } + stored = typeof id === 'string' ? lookup(id) : undefined + } + merged[key] = stored && Object.hasOwn(stored, key) ? stored[key] : existing[key] + } + return merged ?? withRelations +} + +function indexById(items: readonly unknown[]): Map> { + const byId = new Map>() + for (const item of items) { + if (isRecord(item) && item['id'] !== undefined) { + byId.set(item['id'], item) } } - return undefined + return byId +} + +// A paginated has-many carries its total count as a non-enumerable property of the item array. +function copyTotalCount(from: readonly unknown[], to: unknown[]): void { + const descriptor = Object.getOwnPropertyDescriptor(from, 'totalCount') + if (descriptor) { + Object.defineProperty(to, 'totalCount', descriptor) + } +} + +function isConnection(value: unknown): value is Connection { + return isRecord(value) && Array.isArray(value['edges']) } function isRecord(value: unknown): value is Record { diff --git a/tests/react/relations/hasMany/nestedSameEntityNarrowerSelection.test.tsx b/tests/react/relations/hasMany/nestedSameEntityNarrowerSelection.test.tsx index b0a090ce..46aaaa0e 100644 --- a/tests/react/relations/hasMany/nestedSameEntityNarrowerSelection.test.tsx +++ b/tests/react/relations/hasMany/nestedSameEntityNarrowerSelection.test.tsx @@ -1,9 +1,9 @@ // Regression test for https://github.com/contember/bindx/issues/123 import '../../../setup' import { afterEach, describe, expect, test } from 'bun:test' -import { cleanup, render, waitFor } from '@testing-library/react' +import { act, cleanup, render, waitFor } from '@testing-library/react' import React from 'react' -import { BindxProvider, defineSchema, entityDef, hasMany, hasOne, MockAdapter, scalar, useEntity } from '@contember/bindx-react' +import { BindxProvider, defineSchema, entityDef, hasMany, hasOne, MockAdapter, scalar, type SnapshotStore, useEntity, useEntityList, usePersist, useSnapshotStore } from '@contember/bindx-react' import { getByTestId, queryByTestId } from './setup' afterEach(() => { @@ -16,36 +16,45 @@ interface Attachment { type: string } -interface Meeting { +interface Author { id: string - session: Session | null + name: string + email: string +} + +interface Section { + id: string + article: Article | null } -interface Session { +interface Article { id: string attachments: Attachment[] - meetings: Meeting[] + sections: Section[] + author: Author | null } interface CycleSchema { - Session: Session - Meeting: Meeting + Article: Article + Section: Section Attachment: Attachment + Author: Author } const schema = defineSchema({ entities: { - Session: { + Article: { fields: { id: scalar(), attachments: hasMany('Attachment'), - meetings: hasMany('Meeting'), + sections: hasMany('Section'), + author: hasOne('Author', { nullable: true }), }, }, - Meeting: { + Section: { fields: { id: scalar(), - session: hasOne('Session', { nullable: true }), + article: hasOne('Article', { nullable: true }), }, }, Attachment: { @@ -55,131 +64,228 @@ const schema = defineSchema({ type: scalar(), }, }, + Author: { + fields: { + id: scalar(), + name: scalar(), + email: scalar(), + }, + }, }, }) const entityDefs = { - Session: entityDef('Session'), + Article: entityDef
('Article'), + Attachment: entityDef('Attachment'), } as const -function createMockData() { - const attachments = [{ id: 'att-1', name: 'Slides', type: 'learningMaterial' }] +const by = { by: { id: 'article-1' } } + +function createMockData(): ConstructorParameters[0] { + const attachments = [{ id: 'att-1', name: 'Slides', type: 'pdf' }] + const author = { id: 'author-1', name: 'Ann', email: 'ann@example.com' } return { - Session: { - 'session-1': { - id: 'session-1', + Article: { + 'article-1': { + id: 'article-1', attachments, - // The meeting points back at the SAME session — a cycle the page selects - // through a second component with a narrower attachment selection. - meetings: [{ id: 'meeting-1', session: { id: 'session-1', attachments } }], + author, + // The section points back at the SAME article — a cycle the page selects + // through a second component with a narrower selection. + sections: [{ id: 'section-1', article: { id: 'article-1', attachments, author } }], }, }, - Meeting: {}, - Attachment: {}, + Section: {}, + Attachment: { 'att-1': { ...attachments[0] } }, + Author: { 'author-1': { ...author } }, } } +function renderWithin(adapter: MockAdapter, children: React.ReactNode): ReturnType { + return render({children}) +} + +async function waitForTestIds(container: HTMLElement, ...testIds: string[]): Promise { + await waitFor(() => { + for (const testId of testIds) { + expect(queryByTestId(container, testId)).not.toBeNull() + } + }) +} + /** - * One root entity whose selection reaches the same `Session` twice: directly with - * `attachments { name type }`, and through `meetings.session` with `attachments { name }`. - * `readNestedFirst` mirrors two sibling components where the one holding the narrower - * selection renders first. + * One root entity whose selection reaches the same `Article` twice: directly with + * `attachments { name type }` and `author { name email }`, and through `sections.article` + * with `attachments { name }` and `author { name }`. `readNestedFirst` mirrors two sibling + * components where the one holding the narrower selection renders first. */ -function SessionView({ readNestedFirst }: { readNestedFirst: boolean }): React.ReactElement { - const session = useEntity(entityDefs.Session, { by: { id: 'session-1' } }, e => +function ArticleView({ readNestedFirst }: { readNestedFirst: boolean }): React.ReactElement { + const article = useEntity(entityDefs.Article, by, e => e.id() .attachments(a => a.id().name().type()) - .meetings(m => m.id().session(s => s.id().attachments(a => a.id().name()))), + .author(a => a.id().name().email()) + .sections(m => m.id().article(s => s.id().attachments(a => a.id().name()).author(a => a.id().name()))), ) - if (session.$isLoading || session.$isError || session.$isNotFound) { + if (article.$isLoading || article.$isError || article.$isNotFound) { return
Loading...
} - const nestedNames = () => session.meetings.items.flatMap(m => m.session.attachments.items.map(a => a.$fields.name.value)).join(',') - const directTypes = () => session.attachments.items.map(a => String(a.$fields.type.value)).join(',') + const nestedNames = (): string => article.sections.items + .flatMap(m => [...m.article.attachments.items.map(a => a.$fields.name.value), m.article.author.name.value]) + .join(',') + const directTypes = (): string => article.attachments.items.map(a => String(a.$fields.type.value)).join(',') + const directEmail = (): string => String(article.author.email.value) const nested = readNestedFirst ? nestedNames() : '' const types = directTypes() + const email = directEmail() const nestedAfter = readNestedFirst ? nested : nestedNames() return (
{nestedAfter} {types} + {email}
) } -describe('HasMany - the same entity reached twice with different selections', () => { +describe('the same entity reached twice in one query with different selections', () => { test('should keep the wider selection\'s fields when the narrower nested occurrence is read first', async () => { - const adapter = new MockAdapter(createMockData(), { delay: 0 }) - - const { container } = render( - - - , - ) - - await waitFor(() => { - expect(queryByTestId(container, 'direct-types')).not.toBeNull() - }) + const { container } = renderWithin(new MockAdapter(createMockData(), { delay: 0 }), ) + await waitForTestIds(container, 'direct-types') - expect(getByTestId(container, 'nested-names').textContent).toBe('Slides') - expect(getByTestId(container, 'direct-types').textContent).toBe('learningMaterial') + expect(getByTestId(container, 'nested-names').textContent).toBe('Slides,Ann') + expect(getByTestId(container, 'direct-types').textContent).toBe('pdf') + expect(getByTestId(container, 'direct-email').textContent).toBe('ann@example.com') }) test('should keep the wider selection\'s fields when the direct occurrence is read first', async () => { - const adapter = new MockAdapter(createMockData(), { delay: 0 }) - - const { container } = render( - - - , - ) - - await waitFor(() => { - expect(queryByTestId(container, 'direct-types')).not.toBeNull() - }) + const { container } = renderWithin(new MockAdapter(createMockData(), { delay: 0 }), ) + await waitForTestIds(container, 'direct-types') - expect(getByTestId(container, 'nested-names').textContent).toBe('Slides') - expect(getByTestId(container, 'direct-types').textContent).toBe('learningMaterial') + expect(getByTestId(container, 'nested-names').textContent).toBe('Slides,Ann') + expect(getByTestId(container, 'direct-types').textContent).toBe('pdf') + expect(getByTestId(container, 'direct-email').textContent).toBe('ann@example.com') }) }) +// ── Two roots over one entity ───────────────────────────────────────────── + +let editWideRoot: ((type: string, email: string) => void) | null = null +let persistAll: (() => Promise) | null = null +let bindxStore: SnapshotStore | null = null + +/** What the store holds for a related entity, as data and as its server baseline. */ +function storedField(entityType: string, id: string, field: string): string { + const snapshot = bindxStore!.getEntitySnapshot>(entityType, id) + return `${String(snapshot?.data[field])}/${String(snapshot?.serverData[field])}` +} + function WideRoot(): React.ReactElement { - const session = useEntity(entityDefs.Session, { by: { id: 'session-1' } }, e => e.id().attachments(a => a.id().name().type())) - if (session.$isLoading || session.$isError || session.$isNotFound) { + const persist = usePersist() + bindxStore = useSnapshotStore() + persistAll = () => persist.persistAll() + const article = useEntity(entityDefs.Article, by, e => e.id().attachments(a => a.id().name().type()).author(a => a.id().name().email())) + if (article.$isLoading || article.$isError || article.$isNotFound) { return
Loading...
} - return {session.attachments.items.map(a => String(a.$fields.type.value)).join(',')} + editWideRoot = (type, email) => { + article.attachments.items[0]?.$fields.type.setValue(type) + article.author.email.setValue(email) + } + return ( +
+ {article.attachments.items.map(a => String(a.$fields.type.value)).join(',')} + {String(article.author.email.value)} +
+ ) } function NarrowRoot(): React.ReactElement { - const session = useEntity(entityDefs.Session, { by: { id: 'session-1' } }, e => e.id().attachments(a => a.id().name())) - if (session.$isLoading || session.$isError || session.$isNotFound) { + const article = useEntity(entityDefs.Article, by, e => e.id().attachments(a => a.id().name()).author(a => a.id().name())) + if (article.$isLoading || article.$isError || article.$isNotFound) { return
Loading...
} - return {session.attachments.items.map(a => a.$fields.name.value).join(',')} + return {article.attachments.items.map(a => a.$fields.name.value).join(',')} } -describe('HasMany - the same entity loaded by two roots with different selections', () => { - test('should keep the wider root\'s fields when the narrower root\'s read lands last', async () => { - const adapter = new MockAdapter(createMockData(), { delay: 0 }) +function NarrowList(): React.ReactElement { + const articles = useEntityList(entityDefs.Article, {}, e => e.id().attachments(a => a.id().name()).author(a => a.id().name())) + if (articles.$status !== 'ready') { + return
Loading...
+ } + return {articles.items.flatMap(s => s.attachments.items.map(a => a.$fields.name.value)).join(',')} +} - const { container } = render( - - - - , - ) +function AttachmentRoot(): React.ReactElement { + const attachment = useEntity(entityDefs.Attachment, { by: { id: 'att-1' } }, e => e.id().type()) + if (attachment.$isLoading || attachment.$isError || attachment.$isNotFound) { + return
Loading...
+ } + return {String(attachment.type.value)} +} - await waitFor(() => { - expect(queryByTestId(container, 'wide-types')).not.toBeNull() - expect(queryByTestId(container, 'narrow-names')).not.toBeNull() - }) +describe('the same entity loaded by two roots with different selections', () => { + test('should keep the wider root\'s fields when the narrower useEntity read lands last', async () => { + const { container } = renderWithin(new MockAdapter(createMockData(), { delay: 0 }), <>) + await waitForTestIds(container, 'wide-types', 'narrow-names') + + expect(getByTestId(container, 'narrow-names').textContent).toBe('Slides') + expect(getByTestId(container, 'wide-types').textContent).toBe('pdf') + expect(getByTestId(container, 'wide-email').textContent).toBe('ann@example.com') + }) + + test('should keep the wider root\'s fields when the narrower useEntityList read lands last', async () => { + const { container } = renderWithin(new MockAdapter(createMockData(), { delay: 0 }), <>) + await waitForTestIds(container, 'wide-types', 'narrow-names') expect(getByTestId(container, 'narrow-names').textContent).toBe('Slides') - expect(getByTestId(container, 'wide-types').textContent).toBe('learningMaterial') + expect(getByTestId(container, 'wide-types').textContent).toBe('pdf') + expect(getByTestId(container, 'wide-email').textContent).toBe('ann@example.com') + }) +}) + +describe('a narrower read after the related entity moved past its embedded copy', () => { + test('should keep values persisted through the wider root', async () => { + const adapter = new MockAdapter(createMockData(), { delay: 0 }) + const { container, rerender } = renderWithin(adapter, ) + await waitForTestIds(container, 'wide-types') + + act(() => editWideRoot!('docx', 'ann@example.org')) + await act(async () => { + await persistAll!() + }) + expect(getByTestId(container, 'wide-types').textContent).toBe('docx') + + rerender() + await waitForTestIds(container, 'narrow-names') + + expect(storedField('Attachment', 'att-1', 'type')).toBe('docx/docx') + expect(storedField('Author', 'author-1', 'email')).toBe('ann@example.org/ann@example.org') + expect(getByTestId(container, 'wide-types').textContent).toBe('docx') + expect(getByTestId(container, 'wide-email').textContent).toBe('ann@example.org') + }) + + test('should keep values a read through another root brought in', async () => { + const data = createMockData() + // The attachment changed on the server after the article was read. + data['Attachment'] = { 'att-1': { id: 'att-1', name: 'Slides', type: 'docx' } } + const adapter = new MockAdapter(data, { delay: 0 }) + const { container, rerender } = renderWithin(adapter, ) + await waitForTestIds(container, 'wide-types') + expect(getByTestId(container, 'wide-types').textContent).toBe('pdf') + + rerender() + await waitForTestIds(container, 'attachment-type') + expect(getByTestId(container, 'wide-types').textContent).toBe('docx') + + rerender() + await waitForTestIds(container, 'narrow-names') + + expect(storedField('Attachment', 'att-1', 'type')).toBe('docx/docx') + expect(getByTestId(container, 'wide-types').textContent).toBe('docx') + expect(getByTestId(container, 'attachment-type').textContent).toBe('docx') }) }) diff --git a/tests/unit/store/refreshEmbeddedRelationMerge.test.ts b/tests/unit/store/refreshEmbeddedRelationMerge.test.ts index fab84679..074a00b3 100644 --- a/tests/unit/store/refreshEmbeddedRelationMerge.test.ts +++ b/tests/unit/store/refreshEmbeddedRelationMerge.test.ts @@ -6,10 +6,17 @@ import { createTestStore } from '../shared/unitTestHelpers.js' const { createSelectionBuilder, getSelectionMeta } = __internal +interface StoredFile { + id: string + url: string + size: number +} + interface Attachment { id: string name: string type: string + file: StoredFile | null } interface Author { @@ -18,7 +25,7 @@ interface Author { email: string } -interface Session { +interface Article { id: string title: string settings: { id: string; mode: string; extra?: string } | null @@ -26,15 +33,15 @@ interface Session { author: Author | null } -const narrowAttachments = getSelectionMeta(createSelectionBuilder().id().attachments(a => a.id().name())) -const narrowAuthor = getSelectionMeta(createSelectionBuilder().id().author(a => a.id().name())) +const narrowAttachments = getSelectionMeta(createSelectionBuilder
().id().attachments(a => a.id().name())) +const narrowAuthor = getSelectionMeta(createSelectionBuilder
().id().author(a => a.id().name())) -const wideSession = (): Record => ({ +const wideArticle = (): Record => ({ id: 's-1', - title: 'Session', + title: 'Article', attachments: [ - { id: 'att-1', name: 'Slides', type: 'learningMaterial' }, - { id: 'att-2', name: 'Notes', type: 'internal' }, + { id: 'att-1', name: 'Slides', type: 'pdf' }, + { id: 'att-2', name: 'Notes', type: 'docx' }, ], author: { id: 'au-1', name: 'Ann', email: 'ann@example.com' }, }) @@ -44,11 +51,11 @@ describe('SnapshotStore.refreshServerData — embedded relations read with a nar beforeEach(() => { store = createTestStore() - store.setEntityData('Session', 's-1', wideSession(), true) + store.setEntityData('Article', 's-1', wideArticle(), true) }) test('keeps has-many item fields the narrower selection did not ask for', () => { - store.refreshServerData('Session', 's-1', { + store.refreshServerData('Article', 's-1', { id: 's-1', attachments: [ { id: 'att-1', name: 'Slides v2' }, @@ -56,18 +63,18 @@ describe('SnapshotStore.refreshServerData — embedded relations read with a nar ], }, false, narrowAttachments) - const snapshot = store.getEntitySnapshot>('Session', 's-1') + const snapshot = store.getEntitySnapshot>('Article', 's-1') const expected = [ - { id: 'att-1', name: 'Slides v2', type: 'learningMaterial' }, - { id: 'att-2', name: 'Notes', type: 'internal' }, + { id: 'att-1', name: 'Slides v2', type: 'pdf' }, + { id: 'att-2', name: 'Notes', type: 'docx' }, ] expect(snapshot?.serverData?.['attachments']).toEqual(expected) expect(snapshot?.data['attachments']).toEqual(expected) - expect(snapshot?.data['title']).toBe('Session') + expect(snapshot?.data['title']).toBe('Article') }) test('takes has-many membership and order from the server read', () => { - store.refreshServerData('Session', 's-1', { + store.refreshServerData('Article', 's-1', { id: 's-1', attachments: [ { id: 'att-3', name: 'Handout' }, @@ -75,36 +82,36 @@ describe('SnapshotStore.refreshServerData — embedded relations read with a nar ], }, false, narrowAttachments) - const snapshot = store.getEntitySnapshot>('Session', 's-1') + const snapshot = store.getEntitySnapshot>('Article', 's-1') expect(snapshot?.data['attachments']).toEqual([ { id: 'att-3', name: 'Handout' }, - { id: 'att-1', name: 'Slides', type: 'learningMaterial' }, + { id: 'att-1', name: 'Slides', type: 'pdf' }, ]) }) test('keeps has-one fields the narrower selection did not ask for while the target is the same', () => { - store.refreshServerData('Session', 's-1', { id: 's-1', author: { id: 'au-1', name: 'Anna' } }, false, narrowAuthor) + store.refreshServerData('Article', 's-1', { id: 's-1', author: { id: 'au-1', name: 'Anna' } }, false, narrowAuthor) - const snapshot = store.getEntitySnapshot>('Session', 's-1') + const snapshot = store.getEntitySnapshot>('Article', 's-1') expect(snapshot?.data['author']).toEqual({ id: 'au-1', name: 'Anna', email: 'ann@example.com' }) }) test('replaces a has-one that points to another entity', () => { - store.refreshServerData('Session', 's-1', { id: 's-1', author: { id: 'au-2', name: 'Bob' } }, false, narrowAuthor) + store.refreshServerData('Article', 's-1', { id: 's-1', author: { id: 'au-2', name: 'Bob' } }, false, narrowAuthor) - const snapshot = store.getEntitySnapshot>('Session', 's-1') + const snapshot = store.getEntitySnapshot>('Article', 's-1') expect(snapshot?.data['author']).toEqual({ id: 'au-2', name: 'Bob' }) }) test('replaces a has-one the server read disconnected', () => { - store.refreshServerData('Session', 's-1', { id: 's-1', author: null }, false, narrowAuthor) + store.refreshServerData('Article', 's-1', { id: 's-1', author: null }, false, narrowAuthor) - const snapshot = store.getEntitySnapshot>('Session', 's-1') + const snapshot = store.getEntitySnapshot>('Article', 's-1') expect(snapshot?.data['author']).toBeNull() }) test('replaces an object-valued scalar even when it carries an id', () => { - store.setEntityData('Session', 's-1', { ...wideSession(), settings: { id: 'x', mode: 'a', extra: 'stale' } }, true) + store.setEntityData('Article', 's-1', { ...wideArticle(), settings: { id: 'x', mode: 'a', extra: 'stale' } }, true) // A JSON column: selected as a scalar, although its value is an object with an `id`. const withSettings: SelectionMeta = { fields: new Map([ @@ -113,36 +120,123 @@ describe('SnapshotStore.refreshServerData — embedded relations read with a nar ]), } - store.refreshServerData('Session', 's-1', { id: 's-1', settings: { id: 'x', mode: 'b' } }, false, withSettings) + store.refreshServerData('Article', 's-1', { id: 's-1', settings: { id: 'x', mode: 'b' } }, false, withSettings) - const snapshot = store.getEntitySnapshot>('Session', 's-1') + const snapshot = store.getEntitySnapshot>('Article', 's-1') expect(snapshot?.data['settings']).toEqual({ id: 'x', mode: 'b' }) }) test('replaces relation data wholesale when no selection is given', () => { - store.refreshServerData('Session', 's-1', { id: 's-1', attachments: [{ id: 'att-1', name: 'Slides' }] }) + store.refreshServerData('Article', 's-1', { id: 's-1', attachments: [{ id: 'att-1', name: 'Slides' }] }) - const snapshot = store.getEntitySnapshot>('Session', 's-1') + const snapshot = store.getEntitySnapshot>('Article', 's-1') expect(snapshot?.data['attachments']).toEqual([{ id: 'att-1', name: 'Slides' }]) }) test('leaves the entity clean and keeps a dirty scalar edit', () => { - store.setFieldValue('Session', 's-1', ['title'], 'Local edit') + store.setFieldValue('Article', 's-1', ['title'], 'Local edit') - store.refreshServerData('Session', 's-1', { + store.refreshServerData('Article', 's-1', { id: 's-1', title: 'Server title', attachments: [{ id: 'att-1', name: 'Slides' }], - }, false, getSelectionMeta(createSelectionBuilder().id().title().attachments(a => a.id().name()))) + }, false, getSelectionMeta(createSelectionBuilder
().id().title().attachments(a => a.id().name()))) - const snapshot = store.getEntitySnapshot>('Session', 's-1') + const snapshot = store.getEntitySnapshot>('Article', 's-1') expect(snapshot?.data['title']).toBe('Local edit') expect(snapshot?.serverData?.['title']).toBe('Server title') expect(snapshot?.data['attachments']).toBe(snapshot?.serverData?.['attachments']) - store.resetEntity('Session', 's-1') - const reset = store.getEntitySnapshot>('Session', 's-1') + store.resetEntity('Article', 's-1') + const reset = store.getEntitySnapshot>('Article', 's-1') expect(reset?.data['title']).toBe('Server title') - expect(reset?.data['attachments']).toEqual([{ id: 'att-1', name: 'Slides', type: 'learningMaterial' }]) + expect(reset?.data['attachments']).toEqual([{ id: 'att-1', name: 'Slides', type: 'pdf' }]) + }) + + test('prefers the related entity\'s own snapshot over the embedded copy for keys it did not read', () => { + // The attachment moved on after the article was read: persisted, or read through another path. + store.setEntityData('Attachment', 'att-1', { id: 'att-1', name: 'Slides', type: 'xlsx' }, true) + + store.refreshServerData('Article', 's-1', { + id: 's-1', + attachments: [{ id: 'att-1', name: 'Slides' }, { id: 'att-2', name: 'Notes' }], + }, false, narrowAttachments) + + const snapshot = store.getEntitySnapshot>('Article', 's-1') + expect(snapshot?.data['attachments']).toEqual([ + { id: 'att-1', name: 'Slides', type: 'xlsx' }, + { id: 'att-2', name: 'Notes', type: 'docx' }, + ]) + }) + + test('prefers the related has-one entity\'s own snapshot over the embedded copy', () => { + store.setEntityData('Author', 'au-1', { id: 'au-1', name: 'Ann', email: 'ann@example.org' }, true) + + store.refreshServerData('Article', 's-1', { id: 's-1', author: { id: 'au-1', name: 'Ann' } }, false, narrowAuthor) + + const snapshot = store.getEntitySnapshot>('Article', 's-1') + expect(snapshot?.data['author']).toEqual({ id: 'au-1', name: 'Ann', email: 'ann@example.org' }) + }) + + test('merges a relation inside a relation', () => { + store.setEntityData('Article', 's-1', { + id: 's-1', + attachments: [{ id: 'att-1', name: 'Slides', file: { id: 'f-1', url: '/a.pdf', size: 42 } }], + }, true) + + store.refreshServerData('Article', 's-1', { + id: 's-1', + attachments: [{ id: 'att-1', file: { id: 'f-1', url: '/b.pdf' } }], + }, false, getSelectionMeta(createSelectionBuilder
().id().attachments(a => a.id().file(f => f.id().url())))) + + const snapshot = store.getEntitySnapshot>('Article', 's-1') + expect(snapshot?.data['attachments']).toEqual([{ id: 'att-1', name: 'Slides', file: { id: 'f-1', url: '/b.pdf', size: 42 } }]) + }) + + test('merges an aliased has-many selected with params and keeps its total count', () => { + const withParams = getSelectionMeta( + createSelectionBuilder
().id().attachments({ orderBy: [{ name: 'asc' }], limit: 1, totalCount: true }, a => a.id().name()), + ) + const alias = [...withParams.fields.values()].find(field => field.fieldName === 'attachments')?.alias + if (!alias || alias === 'attachments') throw new Error('Expected an aliased has-many') + store.setEntityData('Article', 's-1', { id: 's-1', [alias]: [{ id: 'att-1', name: 'Slides', type: 'pdf' }] }, true) + + const incomingItems = [{ id: 'att-1', name: 'Slides v2' }] + Object.defineProperty(incomingItems, 'totalCount', { value: 2, enumerable: false, writable: false }) + store.refreshServerData('Article', 's-1', { id: 's-1', [alias]: incomingItems }, false, withParams) + + const merged = store.getEntitySnapshot>('Article', 's-1')?.data[alias] + expect(merged).toEqual([{ id: 'att-1', name: 'Slides v2', type: 'pdf' }]) + expect(Object.getOwnPropertyDescriptor(merged, 'totalCount')).toEqual({ value: 2, enumerable: false, writable: false, configurable: false }) + }) + + test('merges the nodes of a connection by id', () => { + store.setEntityData('Article', 's-1', { + id: 's-1', + attachments: { pageInfo: { totalCount: 1 }, edges: [{ node: { id: 'att-1', name: 'Slides', type: 'pdf' } }] }, + }, true) + + store.refreshServerData('Article', 's-1', { + id: 's-1', + attachments: { pageInfo: { totalCount: 2 }, edges: [{ node: { id: 'att-2', name: 'Notes' } }, { node: { id: 'att-1', name: 'Slides' } }] }, + }, false, narrowAttachments) + + const snapshot = store.getEntitySnapshot>('Article', 's-1') + expect(snapshot?.data['attachments']).toEqual({ + pageInfo: { totalCount: 2 }, + edges: [{ node: { id: 'att-2', name: 'Notes' } }, { node: { id: 'att-1', name: 'Slides', type: 'pdf' } }], + }) + }) + + test('stores the incoming relation value itself when it leaves nothing to keep', () => { + const incoming = [ + { id: 'att-1', name: 'Slides', type: 'pdf' }, + { id: 'att-2', name: 'Notes', type: 'docx' }, + ] + + store.refreshServerData('Article', 's-1', { id: 's-1', attachments: incoming }, false, + getSelectionMeta(createSelectionBuilder
().id().attachments(a => a.id().name().type()))) + + expect(store.getEntitySnapshot>('Article', 's-1')?.serverData['attachments']).toBe(incoming) }) }) From e824abaa05ceadbc9b382d93271c4357bbfbb0e1 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Mon, 28 Sep 2026 15:44:55 +0200 Subject: [PATCH 5/7] fix(bindx): unite the occurrences of an entity within one server response Replaces the cross-read merge of the previous commits. Merging a narrower read into what the store already held needs to know which copy is newer: the related entity's snapshot after a persist or a read through another path, the parent's embedded copy after a refetch nothing has propagated yet. Neither precedence is right in both directions. The occurrences of one response are equally fresh, so they need no recency rule. When a response enters the store, every occurrence is indexed by entity type and id (types come from the schema through the selection's field names, never from the id alone), and each occurrence resolves to the shallow union of all of them. Whatever writes an occurrence into a snapshot writes the union instead: useEntity, useEntityList and HasManyDataGrid for the rows they load, HasOneHandle and HasManyListHandle when they propagate embedded data. Related entities inside a union resolve to their own unions when they are written, so relations inside relations unite level by level without rewriting the response. Separate reads replace each other as they did before. Closes #123 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Ge3kRufpi9b9HMBSBjRze5 --- .../bindx-dataview/src/HasManyDataGrid.tsx | 5 +- packages/bindx-react/src/hooks/useEntity.ts | 7 +- .../bindx-react/src/hooks/useEntityList.ts | 7 +- packages/bindx/src/core/ActionDispatcher.ts | 2 - packages/bindx/src/core/actions.ts | 6 +- .../bindx/src/handles/HasManyListHandle.ts | 4 +- packages/bindx/src/handles/HasOneHandle.ts | 7 +- .../bindx/src/store/EntitySnapshotStore.ts | 17 +- .../src/store/ResponseOccurrenceIndex.ts | 138 +++++++ packages/bindx/src/store/SnapshotStore.ts | 23 +- .../bindx/src/store/embeddedRelationMerge.ts | 155 -------- ...nestedSameEntityNarrowerSelection.test.tsx | 343 +++++++++++------- .../refreshEmbeddedRelationMerge.test.ts | 242 ------------ .../store/responseOccurrenceIndex.test.ts | 159 ++++++++ 14 files changed, 547 insertions(+), 568 deletions(-) create mode 100644 packages/bindx/src/store/ResponseOccurrenceIndex.ts delete mode 100644 packages/bindx/src/store/embeddedRelationMerge.ts delete mode 100644 tests/unit/store/refreshEmbeddedRelationMerge.test.ts create mode 100644 tests/unit/store/responseOccurrenceIndex.test.ts diff --git a/packages/bindx-dataview/src/HasManyDataGrid.tsx b/packages/bindx-dataview/src/HasManyDataGrid.tsx index 12271112..0dd27743 100644 --- a/packages/bindx-dataview/src/HasManyDataGrid.tsx +++ b/packages/bindx-dataview/src/HasManyDataGrid.tsx @@ -189,7 +189,8 @@ function HasManyDataGridImpl({ return } - const items = relation.rows.map(data => ({ id: getRowId(targetEntityType, data), data })) + store.indexServerResponse(targetEntityType, relation.rows, setup.selection, schemaRegistry) + const items = relation.rows.map(data => ({ id: getRowId(targetEntityType, data), data: store.resolveServerOccurrence(data) })) store.batchNotifications(() => { for (const item of items) { dispatcher.dispatch(setEntityData(targetEntityType, item.id, item.data, true)) @@ -208,7 +209,7 @@ function HasManyDataGridImpl({ return () => { abortController.abort() } - }, [parentEntityType, parentEntityId, fieldName, targetEntityType, optionsKey, setup.selection, batcher, dispatcher, store]) + }, [parentEntityType, parentEntityId, fieldName, targetEntityType, optionsKey, setup.selection, batcher, dispatcher, store, schemaRegistry]) // ---- Build items from state ---- const items = useMemo((): EntityAccessor[] => { diff --git a/packages/bindx-react/src/hooks/useEntity.ts b/packages/bindx-react/src/hooks/useEntity.ts index 2811ea27..7bd976bc 100644 --- a/packages/bindx-react/src/hooks/useEntity.ts +++ b/packages/bindx-react/src/hooks/useEntity.ts @@ -290,11 +290,12 @@ 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 + store.indexServerResponse(entityType, result.data, selectionMeta, schemaRegistry) + const data = store.resolveServerOccurrence(result.data) // Revalidation: advance the server baseline but keep local dirty // edits intact (see EntitySnapshotStore.refreshServerData). store.batchNotifications(() => { - dispatcher.dispatch(refreshServerData(entityType, id, data, selectionMeta)) + dispatcher.dispatch(refreshServerData(entityType, id, data)) dispatcher.dispatch(setLoadState(entityType, id, 'success')) }) } @@ -321,7 +322,7 @@ export function useEntity( fetchingRef.current = null } } - }, [entityType, id, byKey, effectiveQueryKey, options.cache, batcher, store, dispatcher, selectionMeta]) + }, [entityType, id, byKey, effectiveQueryKey, options.cache, batcher, store, dispatcher, selectionMeta, schemaRegistry]) // --- EntityHandle --- // The handle keeps a stable identity across data changes — it is a stateless live view over the diff --git a/packages/bindx-react/src/hooks/useEntityList.ts b/packages/bindx-react/src/hooks/useEntityList.ts index 6719b19b..c7c721df 100644 --- a/packages/bindx-react/src/hooks/useEntityList.ts +++ b/packages/bindx-react/src/hooks/useEntityList.ts @@ -462,11 +462,12 @@ export function useEntityList( throw new Error('Unexpected query result type') } - const items = result.data.map(data => ({ id: getRowId(entityType, data), data })) + store.indexServerResponse(entityType, result.data, selectionMeta, schemaRegistry) + const items = result.data.map(data => ({ id: getRowId(entityType, data), data: store.resolveServerOccurrence(data) })) store.batchNotifications(() => { for (const item of items) { // Revalidation preserves local edits while advancing the server baseline. - dispatcher.dispatch(refreshServerData(entityType, item.id, item.data, selectionMeta)) + dispatcher.dispatch(refreshServerData(entityType, item.id, item.data)) } listStateRef.current = { status: 'ready', items, isRefetching: false } versionRef.current++ @@ -492,7 +493,7 @@ export function useEntityList( return () => { abortController.abort() } - }, [entityType, optionsKey, effectiveQueryKey, batcher, dispatcher, store, selectionMeta]) + }, [entityType, optionsKey, effectiveQueryKey, batcher, dispatcher, store, selectionMeta, schemaRegistry]) return accessor } diff --git a/packages/bindx/src/core/ActionDispatcher.ts b/packages/bindx/src/core/ActionDispatcher.ts index 3e297b0f..7945d6ce 100644 --- a/packages/bindx/src/core/ActionDispatcher.ts +++ b/packages/bindx/src/core/ActionDispatcher.ts @@ -243,8 +243,6 @@ export class ActionDispatcher { action.entityType, action.entityId, action.data, - false, - action.selection, ) break diff --git a/packages/bindx/src/core/actions.ts b/packages/bindx/src/core/actions.ts index 8a33d862..d48da813 100644 --- a/packages/bindx/src/core/actions.ts +++ b/packages/bindx/src/core/actions.ts @@ -1,6 +1,5 @@ import type { HasOneRelationState } from '../handles/types.js' import type { FieldError, FieldErrorFilter } from '../errors/types.js' -import type { SelectionMeta } from '../selection/types.js' /** * Action types for the ActionDispatcher. @@ -59,8 +58,6 @@ export interface RefreshServerDataAction { readonly entityType: string readonly entityId: string readonly data: Record - /** The selection `data` was read with; lets embedded relations merge instead of being replaced. */ - readonly selection?: SelectionMeta } // ==================== Relation Actions ==================== @@ -379,9 +376,8 @@ export function refreshServerData( entityType: string, entityId: string, data: Record, - selection?: SelectionMeta, ): RefreshServerDataAction { - return { type: 'REFRESH_SERVER_DATA', entityType, entityId, data, selection } + return { type: 'REFRESH_SERVER_DATA', entityType, entityId, data } } /** diff --git a/packages/bindx/src/handles/HasManyListHandle.ts b/packages/bindx/src/handles/HasManyListHandle.ts index 36730ac0..33afda36 100644 --- a/packages/bindx/src/handles/HasManyListHandle.ts +++ b/packages/bindx/src/handles/HasManyListHandle.ts @@ -284,7 +284,8 @@ export class HasManyListHandle>): void { - for (const itemData of listData) { + for (const embeddedItem of listData) { + const itemData = this.store.resolveServerOccurrence(embeddedItem) const itemId = itemData['id'] as string if (!itemId) continue @@ -308,7 +309,6 @@ export class HasManyListHandle return } + const occurrence = this.store.resolveServerOccurrence(embeddedData as Record) + // Skip if embedded data values match existing serverData — avoids overwriting // unpersisted local mutations when a re-fetch returns the same server data // (e.g. polling). A new reference with identical values means no actual change. const existing = this.store.getEntitySnapshot(this.targetType, id) - if (existing?.serverData && embeddedDataMatchesSnapshot(embeddedData as Record, existing.serverData as Record)) { + if (existing?.serverData && embeddedDataMatchesSnapshot(occurrence, existing.serverData as Record)) { this.store.markEmbeddedDataPropagated(this.entityType, this.entityId, this.dataFieldName, embeddedData) return } @@ -435,9 +437,8 @@ export class HasOneHandle this.store.refreshServerData( this.targetType, id, - embeddedData as Record, + occurrence, true, // skipNotify - called during render, data already exists embedded in parent - this.selection, ) this.store.markEmbeddedDataPropagated(this.entityType, this.entityId, this.dataFieldName, embeddedData) } diff --git a/packages/bindx/src/store/EntitySnapshotStore.ts b/packages/bindx/src/store/EntitySnapshotStore.ts index ac1f37cb..f60d45fc 100644 --- a/packages/bindx/src/store/EntitySnapshotStore.ts +++ b/packages/bindx/src/store/EntitySnapshotStore.ts @@ -3,8 +3,6 @@ import { type EntitySnapshot, } from './snapshots.js' import type { RekeyContext, Rekeyable } from './RekeyOrchestrator.js' -import type { SelectionMeta } from '../selection/types.js' -import { mergeEmbeddedRelationFields } from './embeddedRelationMerge.js' /** * Manages entity snapshots — core CRUD for immutable entity data. @@ -134,13 +132,6 @@ export class EntitySnapshotStore implements Rekeyable { return newSnapshot } - private readonly lookupServerData = (id: string): Readonly> | undefined => { - const key = this.idIndex.get(id) - const snapshot = key === undefined ? undefined : this.snapshots.get(key) - if (!snapshot) return undefined - return (snapshot.serverData ?? snapshot.data) as Readonly> - } - /** * Refreshes entity data from a fresh server read (revalidation). * @@ -152,18 +143,12 @@ export class EntitySnapshotStore implements Rekeyable { * "differs from the new server value" after the refresh. * * When no snapshot exists yet this behaves like a plain server load. - * - * Given the selection the data was read with, embedded relation values are - * merged into the stored ones rather than replacing them, so a narrower read - * of the same entity keeps what a wider one fetched - * (see {@link mergeEmbeddedRelationFields}). */ refreshServerData( key: string, id: string, entityType: string, data: T, - selection?: SelectionMeta, ): EntitySnapshot { const existing = this.snapshots.get(key) if (!existing) { @@ -172,7 +157,7 @@ export class EntitySnapshotStore implements Rekeyable { const prevData = existing.data as Record const prevServer = (existing.serverData ?? existing.data) as Record - const incoming = mergeEmbeddedRelationFields(prevServer, data as Record, selection, this.lookupServerData) + const incoming = data as Record const newServerData: Record = { ...prevServer } const newData: Record = { ...prevData } diff --git a/packages/bindx/src/store/ResponseOccurrenceIndex.ts b/packages/bindx/src/store/ResponseOccurrenceIndex.ts new file mode 100644 index 00000000..b821901a --- /dev/null +++ b/packages/bindx/src/store/ResponseOccurrenceIndex.ts @@ -0,0 +1,138 @@ +import type { SelectionMeta } from '../selection/types.js' + +/** The schema knowledge the index needs: where a relation points and whether it is a list. */ +export interface RelationTargetResolver { + getRelationTarget(entityType: string, fieldName: string): string | undefined + isHasMany(entityType: string, fieldName: string): boolean +} + +type EntityRecord = Record + +interface PendingVisit { + readonly value: unknown + readonly entityType: string + readonly selection: SelectionMeta + readonly isList: boolean +} + +/** + * Unites the occurrences of one entity within a single server response. + * + * One query can reach the same entity through several paths, each with its own + * sub-selection (`article.attachments` next to `article.sections.article.attachments`). + * Every occurrence is later written into the entity's snapshot, so without this + * the narrower one could replace fields the wider one carried. Occurrences of + * one response are equally fresh, so they are merged without any recency rule: + * each maps to the union of all of them, keyed by entity type and id. A scalar two + * occurrences disagree on (which one server read should not produce) takes the + * value of the occurrence visited last. The response is walked breadth first, + * relations in selection order and list items in list order, so the outcome + * follows from the response and the selection alone. + * + * The union is shallow. A relation value in it is one occurrence's raw value, and + * the related entities inside it resolve to their own unions when they are + * written, so relations inside relations unite level by level without rewriting + * the response. + * + * Reads never merge across responses: a later read replaces what an earlier one + * stored, as before. + */ +export class ResponseOccurrenceIndex { + private readonly unions = new WeakMap>() + + /** + * Indexes a response read for `entityType` with `selection`. The type of every + * nested occurrence comes from the schema through the selection's field names; + * an occurrence whose type cannot be resolved is left out rather than matched + * by id alone. + */ + index(entityType: string, data: unknown, selection: SelectionMeta, schema: RelationTargetResolver): void { + const occurrencesByType = collectOccurrences({ value: data, entityType, selection, isList: Array.isArray(data) }, schema) + for (const occurrencesById of occurrencesByType.values()) { + for (const occurrences of occurrencesById.values()) { + if (Array.isArray(occurrences)) { + this.unite(occurrences) + } + } + } + } + + private unite(occurrences: readonly EntityRecord[]): void { + const union: EntityRecord = {} + for (const occurrence of occurrences) { + Object.assign(union, occurrence) + } + Object.freeze(union) + for (const occurrence of occurrences) { + this.unions.set(occurrence, union) + } + } + + /** The union of every occurrence of this entity in its response, or the occurrence itself when it had no sibling. */ + resolve(occurrence: EntityRecord): EntityRecord { + return this.unions.get(occurrence) ?? occurrence + } +} + +/** Occurrences by entity type and id. A lone occurrence is kept as is; an array only appears once an id repeats. */ +type OccurrencesByType = Map> + +function collectOccurrences(root: PendingVisit, schema: RelationTargetResolver): OccurrencesByType { + const occurrencesByType: OccurrencesByType = new Map() + const pending: PendingVisit[] = [root] + for (let next = 0; next < pending.length; next++) { + const visit = pending[next]! + for (const entity of entitiesOf(visit.value, visit.isList)) { + addOccurrence(occurrencesByType, visit.entityType, entity) + for (const fieldMeta of visit.selection.fields.values()) { + if (!fieldMeta.isRelation || !fieldMeta.nested || entity[fieldMeta.alias] == null) continue + const targetType = schema.getRelationTarget(visit.entityType, fieldMeta.fieldName) + if (targetType === undefined) continue + pending.push({ + value: entity[fieldMeta.alias], + entityType: targetType, + selection: fieldMeta.nested, + isList: schema.isHasMany(visit.entityType, fieldMeta.fieldName), + }) + } + } + } + return occurrencesByType +} + +function addOccurrence(occurrencesByType: OccurrencesByType, entityType: string, entity: EntityRecord): void { + const id = entity['id'] + if (typeof id !== 'string' && typeof id !== 'number') return + let occurrencesById = occurrencesByType.get(entityType) + if (!occurrencesById) { + occurrencesById = new Map() + occurrencesByType.set(entityType, occurrencesById) + } + const known = occurrencesById.get(id) + if (known === undefined) { + occurrencesById.set(id, entity) + } else if (Array.isArray(known)) { + known.push(entity) + } else if (known !== entity) { + occurrencesById.set(id, [known, entity]) + } +} + +/** The entity records a relation value holds: a has-one object, or a has-many array or connection (`{ edges: [{ node }] }`). */ +function entitiesOf(value: unknown, isList: boolean): readonly EntityRecord[] { + if (Array.isArray(value)) { + return value.filter(isRecord) + } + if (!isRecord(value)) { + return [] + } + if (!isList) { + return [value] + } + const edges = value['edges'] + return Array.isArray(edges) ? edges.map(edge => (isRecord(edge) ? edge['node'] : undefined)).filter(isRecord) : [] +} + +function isRecord(value: unknown): value is EntityRecord { + return typeof value === 'object' && value !== null && !Array.isArray(value) +} diff --git a/packages/bindx/src/store/SnapshotStore.ts b/packages/bindx/src/store/SnapshotStore.ts index 66a2d401..05568e9a 100644 --- a/packages/bindx/src/store/SnapshotStore.ts +++ b/packages/bindx/src/store/SnapshotStore.ts @@ -19,6 +19,7 @@ import { generateTempId } from './entityId.js' import { DirtyTracker } from './DirtyTracker.js' import { EntitySnapshotStore } from './EntitySnapshotStore.js' import { RootRegistry } from './RootRegistry.js' +import { ResponseOccurrenceIndex, type RelationTargetResolver } from './ResponseOccurrenceIndex.js' import { ReachabilityAnalyzer } from './ReachabilityAnalyzer.js' import { RekeyOrchestrator } from './RekeyOrchestrator.js' import type { RekeyContext, Rekeyable } from './RekeyOrchestrator.js' @@ -84,6 +85,8 @@ export class SnapshotStore implements SnapshotVersionBumper, JournalTarget { */ private readonly lastPropagatedData = new Map() + private readonly responseOccurrences = new ResponseOccurrenceIndex() + /** * Optional write-journal. When set (by an attached UndoManager), mutating * methods record editable-layer pre-images of the cells they touch so a gesture @@ -310,6 +313,23 @@ export class SnapshotStore implements SnapshotVersionBumper, JournalTarget { return newSnapshot as EntitySnapshot } + /** + * Unites the occurrences of each entity within one server response, so the + * narrower of two occurrences cannot replace fields the wider one carried. + * Call it before any of the response is written. See {@link ResponseOccurrenceIndex}. + */ + indexServerResponse(entityType: string, data: unknown, selection: SelectionMeta, schema: RelationTargetResolver): void { + this.responseOccurrences.index(entityType, data, selection, schema) + } + + /** + * What to write for an entity occurrence of a server response: the union of + * its occurrences in that response, or the occurrence itself. + */ + resolveServerOccurrence(occurrence: Record): Record { + return this.responseOccurrences.resolve(occurrence) + } + /** * Refreshes server data from a revalidation read while preserving the user's * local dirty edits. See {@link EntitySnapshotStore.refreshServerData}. @@ -319,10 +339,9 @@ export class SnapshotStore implements SnapshotVersionBumper, JournalTarget { id: string, data: T, skipNotify: boolean = false, - selection?: SelectionMeta, ): EntitySnapshot { const key = this.getEntityKey(entityType, id) - const newSnapshot = this.entitySnapshots.refreshServerData(key, this.resolveEntityId(entityType, id), entityType, data, selection) + const newSnapshot = this.entitySnapshots.refreshServerData(key, this.resolveEntityId(entityType, id), entityType, data) this.meta.setExistsOnServer(key, true) if (!skipNotify) { this.notifyEntitySubscribers(key) diff --git a/packages/bindx/src/store/embeddedRelationMerge.ts b/packages/bindx/src/store/embeddedRelationMerge.ts deleted file mode 100644 index dd293de5..00000000 --- a/packages/bindx/src/store/embeddedRelationMerge.ts +++ /dev/null @@ -1,155 +0,0 @@ -import type { SelectionMeta } from '../selection/types.js' - -/** The server baseline the store holds for an entity of this id, if it has a snapshot of it. */ -export type StoredServerDataLookup = (id: string) => Readonly> | undefined - -/** - * Merges a server read of an entity's embedded relation data into what the store - * already holds for it. - * - * One query can reach the same entity through several paths, each with its own - * sub-selection of a relation, and each path refreshes the entity's snapshot. - * Assigning the incoming relation value wholesale would let a narrower - * occurrence drop fields a wider one fetched. The server read still decides - * everything it carries: has-many membership and order, which entity a has-one - * points to, and every value it contains. - * - * A key the read did not select is kept only on the same related entity, and - * its value comes from that entity's own snapshot when the store has one. The - * embedded copy is written only when its parent is read, so after a persist or - * a read through another path it lags behind the snapshot; it is the fallback - * for an entity nothing has materialized yet. - * - * The selection tells relations from scalars: a JSON column can hold an object - * with an `id` too, and it must be replaced, never merged. Without a selection, - * the incoming value replaces the stored one. - * - * Returns `incoming` itself when there is nothing to keep, so the common case - * allocates nothing. - */ -export function mergeEmbeddedRelationFields( - existing: Readonly>, - incoming: Record, - selection: SelectionMeta | undefined, - lookup: StoredServerDataLookup, -): Record { - if (!selection) { - return incoming - } - let merged: Record | undefined - for (const fieldMeta of selection.fields.values()) { - const key = fieldMeta.alias - if (!fieldMeta.isRelation || !fieldMeta.nested || !Object.hasOwn(incoming, key)) { - continue - } - const value = mergeRelationValue(existing[key], incoming[key], fieldMeta.nested, lookup) - if (value !== incoming[key]) { - merged ??= { ...incoming } - merged[key] = value - } - } - return merged ?? incoming -} - -// A fluent selection marks `isArray` only for a has-many given params, so the -// relation kind is read from the value: a has-many embeds an array (or a -// connection), a has-one an object. -function mergeRelationValue(existing: unknown, incoming: unknown, nested: SelectionMeta, lookup: StoredServerDataLookup): unknown { - if (Array.isArray(existing) && Array.isArray(incoming)) { - return mergeItems(existing, incoming, nested, lookup) - } - if (isConnection(existing) && isConnection(incoming)) { - return mergeConnection(existing, incoming, nested, lookup) - } - return mergeRelatedEntity(existing, incoming, nested, lookup) -} - -function mergeItems(existing: readonly unknown[], incoming: unknown[], itemSelection: SelectionMeta, lookup: StoredServerDataLookup): unknown[] { - let existingById: Map> | undefined - const findExisting = (item: unknown, index: number): Record | undefined => { - if (!isRecord(item)) return undefined - // A re-read usually keeps the order, so the item at the same position is checked before building an index. - const atIndex = existing[index] - if (isRecord(atIndex) && atIndex['id'] === item['id']) return atIndex - existingById ??= indexById(existing) - return existingById.get(item['id']) - } - let merged: unknown[] | undefined - incoming.forEach((item, index) => { - const value = mergeRelatedEntity(findExisting(item, index), item, itemSelection, lookup) - if (value !== item) { - merged ??= [...incoming] - merged[index] = value - } - }) - if (!merged) { - return incoming - } - copyTotalCount(incoming, merged) - return merged -} - -interface Connection extends Record { - readonly edges: readonly unknown[] -} - -function mergeConnection(existing: Connection, incoming: Connection, nodeSelection: SelectionMeta, lookup: StoredServerDataLookup): Connection { - const existingNodes = existing.edges.map(edge => (isRecord(edge) ? edge['node'] : undefined)) - const incomingNodes = incoming.edges.map(edge => (isRecord(edge) ? edge['node'] : undefined)) - const mergedNodes = mergeItems(existingNodes, incomingNodes, nodeSelection, lookup) - if (mergedNodes === incomingNodes) { - return incoming - } - const edges = incoming.edges.map((edge, index) => (isRecord(edge) ? { ...edge, node: mergedNodes[index] } : edge)) - return { ...incoming, edges } -} - -function mergeRelatedEntity(existing: unknown, incoming: unknown, selection: SelectionMeta, lookup: StoredServerDataLookup): unknown { - if (existing === incoming || !isRecord(existing) || !isRecord(incoming)) { - return incoming - } - const id = incoming['id'] - if (id === undefined || existing['id'] !== id) { - return incoming - } - const withRelations = mergeEmbeddedRelationFields(existing, incoming, selection, lookup) - let merged: Record | undefined - let stored: Readonly> | undefined - for (const key of Object.keys(existing)) { - if (Object.hasOwn(incoming, key)) { - continue - } - if (!merged) { - merged = { ...withRelations } - stored = typeof id === 'string' ? lookup(id) : undefined - } - merged[key] = stored && Object.hasOwn(stored, key) ? stored[key] : existing[key] - } - return merged ?? withRelations -} - -function indexById(items: readonly unknown[]): Map> { - const byId = new Map>() - for (const item of items) { - if (isRecord(item) && item['id'] !== undefined) { - byId.set(item['id'], item) - } - } - return byId -} - -// A paginated has-many carries its total count as a non-enumerable property of the item array. -function copyTotalCount(from: readonly unknown[], to: unknown[]): void { - const descriptor = Object.getOwnPropertyDescriptor(from, 'totalCount') - if (descriptor) { - Object.defineProperty(to, 'totalCount', descriptor) - } -} - -function isConnection(value: unknown): value is Connection { - return isRecord(value) && Array.isArray(value['edges']) -} - -function isRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value) -} diff --git a/tests/react/relations/hasMany/nestedSameEntityNarrowerSelection.test.tsx b/tests/react/relations/hasMany/nestedSameEntityNarrowerSelection.test.tsx index 46aaaa0e..daab31dd 100644 --- a/tests/react/relations/hasMany/nestedSameEntityNarrowerSelection.test.tsx +++ b/tests/react/relations/hasMany/nestedSameEntityNarrowerSelection.test.tsx @@ -3,17 +3,37 @@ import '../../../setup' import { afterEach, describe, expect, test } from 'bun:test' import { act, cleanup, render, waitFor } from '@testing-library/react' import React from 'react' -import { BindxProvider, defineSchema, entityDef, hasMany, hasOne, MockAdapter, scalar, type SnapshotStore, useEntity, useEntityList, usePersist, useSnapshotStore } from '@contember/bindx-react' +import { + BindxProvider, + defineSchema, + entityDef, + hasMany, + hasOne, + MockAdapter, + scalar, + type SnapshotStore, + useEntity, + useEntityList, + usePersist, + useSnapshotStore, +} from '@contember/bindx-react' import { getByTestId, queryByTestId } from './setup' afterEach(() => { cleanup() }) +interface StoredFile { + id: string + url: string + size: number +} + interface Attachment { id: string name: string type: string + file: StoredFile | null } interface Author { @@ -39,6 +59,7 @@ interface CycleSchema { Section: Section Attachment: Attachment Author: Author + StoredFile: StoredFile } const schema = defineSchema({ @@ -62,6 +83,7 @@ const schema = defineSchema({ id: scalar(), name: scalar(), type: scalar(), + file: hasOne('StoredFile', { nullable: true }), }, }, Author: { @@ -71,6 +93,13 @@ const schema = defineSchema({ email: scalar(), }, }, + StoredFile: { + fields: { + id: scalar(), + url: scalar(), + size: scalar(), + }, + }, }, }) @@ -81,23 +110,24 @@ const entityDefs = { const by = { by: { id: 'article-1' } } -function createMockData(): ConstructorParameters[0] { - const attachments = [{ id: 'att-1', name: 'Slides', type: 'pdf' }] - const author = { id: 'author-1', name: 'Ann', email: 'ann@example.com' } +interface MockOptions { + readonly type?: string + readonly authorId?: string +} + +function createMockData({ type = 'pdf', authorId = 'author-1' }: MockOptions = {}): ConstructorParameters[0] { + const file = { id: 'file-1', url: '/slides.pdf', size: 42 } + const attachment = { id: 'att-1', name: 'Slides', type, file } + const author = { id: authorId, name: 'Ann', email: 'ann@example.com' } + const article = { id: 'article-1', attachments: [attachment], author, sections: [] as unknown[] } + // The section points back at the SAME article: one response reaches it twice. + article.sections = [{ id: 'section-1', article }] return { - Article: { - 'article-1': { - id: 'article-1', - attachments, - author, - // The section points back at the SAME article — a cycle the page selects - // through a second component with a narrower selection. - sections: [{ id: 'section-1', article: { id: 'article-1', attachments, author } }], - }, - }, + Article: { 'article-1': article }, Section: {}, - Attachment: { 'att-1': { ...attachments[0] } }, - Author: { 'author-1': { ...author } }, + Attachment: { 'att-1': { ...attachment } }, + Author: { [authorId]: { ...author } }, + StoredFile: {}, } } @@ -113,110 +143,120 @@ async function waitForTestIds(container: HTMLElement, ...testIds: string[]): Pro }) } +let bindxStore: SnapshotStore | null = null +let persistAll: (() => Promise) | null = null +let editArticle: ((type: string, email: string) => void) | null = null + +/** What the store holds for an entity field, as data and as its server baseline. */ +function storedField(entityType: string, id: string, field: string): string { + const snapshot = bindxStore!.getEntitySnapshot>(entityType, id) + return `${String(snapshot?.data[field])}/${String(snapshot?.serverData[field])}` +} + +function hasStoredField(entityType: string, id: string, field: string): boolean { + const snapshot = bindxStore!.getEntitySnapshot>(entityType, id) + return snapshot !== undefined && Object.hasOwn(snapshot.data, field) +} + /** - * One root entity whose selection reaches the same `Article` twice: directly with - * `attachments { name type }` and `author { name email }`, and through `sections.article` - * with `attachments { name }` and `author { name }`. `readNestedFirst` mirrors two sibling - * components where the one holding the narrower selection renders first. + * One root whose selection reaches the same article twice: directly with + * `attachments { name type file { url size } }` and `author { name email }`, and through + * `sections.article` with `attachments { name file { url } }` and `author { name }`. + * `readNestedFirst` mirrors two sibling components where the one holding the narrower + * selection renders first. */ function ArticleView({ readNestedFirst }: { readNestedFirst: boolean }): React.ReactElement { + bindxStore = useSnapshotStore() + const persist = usePersist() + persistAll = () => persist.persistAll() const article = useEntity(entityDefs.Article, by, e => e.id() - .attachments(a => a.id().name().type()) + .attachments(a => a.id().name().type().file(f => f.id().url().size())) .author(a => a.id().name().email()) - .sections(m => m.id().article(s => s.id().attachments(a => a.id().name()).author(a => a.id().name()))), + .sections(s => s.id().article(r => r.id().attachments(a => a.id().name().file(f => f.id().url())).author(a => a.id().name()))), ) if (article.$isLoading || article.$isError || article.$isNotFound) { return
Loading...
} + editArticle = (type, email) => { + article.attachments.items[0]?.$fields.type.setValue(type) + article.author.email.setValue(email) + } + const nestedNames = (): string => article.sections.items - .flatMap(m => [...m.article.attachments.items.map(a => a.$fields.name.value), m.article.author.name.value]) + .flatMap(s => [...s.article.attachments.items.map(a => `${a.$fields.name.value}@${a.file.url.value}`), s.article.author.name.value]) + .join(',') + const direct = (): string => article.attachments.items + .map(a => `${String(a.$fields.type.value)}:${String(a.file.size.value)}`) + .concat(String(article.author.email.value)) .join(',') - const directTypes = (): string => article.attachments.items.map(a => String(a.$fields.type.value)).join(',') - const directEmail = (): string => String(article.author.email.value) - const nested = readNestedFirst ? nestedNames() : '' - const types = directTypes() - const email = directEmail() - const nestedAfter = readNestedFirst ? nested : nestedNames() + const nestedBefore = readNestedFirst ? nestedNames() : '' + const directValues = direct() + const nested = readNestedFirst ? nestedBefore : nestedNames() return (
- {nestedAfter} - {types} - {email} + {nested} + {directValues}
) } -describe('the same entity reached twice in one query with different selections', () => { - test('should keep the wider selection\'s fields when the narrower nested occurrence is read first', async () => { - const { container } = renderWithin(new MockAdapter(createMockData(), { delay: 0 }), ) - await waitForTestIds(container, 'direct-types') - - expect(getByTestId(container, 'nested-names').textContent).toBe('Slides,Ann') - expect(getByTestId(container, 'direct-types').textContent).toBe('pdf') - expect(getByTestId(container, 'direct-email').textContent).toBe('ann@example.com') - }) - - test('should keep the wider selection\'s fields when the direct occurrence is read first', async () => { - const { container } = renderWithin(new MockAdapter(createMockData(), { delay: 0 }), ) - await waitForTestIds(container, 'direct-types') - - expect(getByTestId(container, 'nested-names').textContent).toBe('Slides,Ann') - expect(getByTestId(container, 'direct-types').textContent).toBe('pdf') - expect(getByTestId(container, 'direct-email').textContent).toBe('ann@example.com') - }) -}) - -// ── Two roots over one entity ───────────────────────────────────────────── - -let editWideRoot: ((type: string, email: string) => void) | null = null -let persistAll: (() => Promise) | null = null -let bindxStore: SnapshotStore | null = null - -/** What the store holds for a related entity, as data and as its server baseline. */ -function storedField(entityType: string, id: string, field: string): string { - const snapshot = bindxStore!.getEntitySnapshot>(entityType, id) - return `${String(snapshot?.data[field])}/${String(snapshot?.serverData[field])}` -} - -function WideRoot(): React.ReactElement { - const persist = usePersist() +function ArticleList({ readNestedFirst }: { readNestedFirst: boolean }): React.ReactElement { bindxStore = useSnapshotStore() - persistAll = () => persist.persistAll() - const article = useEntity(entityDefs.Article, by, e => e.id().attachments(a => a.id().name().type()).author(a => a.id().name().email())) - if (article.$isLoading || article.$isError || article.$isNotFound) { + const articles = useEntityList(entityDefs.Article, {}, e => + e.id() + .attachments(a => a.id().name().type()) + .sections(s => s.id().article(r => r.id().attachments(a => a.id().name()))), + ) + if (articles.$status !== 'ready') { return
Loading...
} - editWideRoot = (type, email) => { - article.attachments.items[0]?.$fields.type.setValue(type) - article.author.email.setValue(email) - } + const nested = (): string => articles.items + .flatMap(it => it.sections.items.flatMap(s => s.article.attachments.items.map(a => a.$fields.name.value))) + .join(',') + const direct = (): string => articles.items.flatMap(it => it.attachments.items.map(a => String(a.$fields.type.value))).join(',') + const nestedBefore = readNestedFirst ? nested() : '' + const directValues = direct() return (
- {article.attachments.items.map(a => String(a.$fields.type.value)).join(',')} - {String(article.author.email.value)} + {readNestedFirst ? nestedBefore : nested()} + {directValues}
) } -function NarrowRoot(): React.ReactElement { - const article = useEntity(entityDefs.Article, by, e => e.id().attachments(a => a.id().name()).author(a => a.id().name())) +/** Both occurrences select the has-many with the same params, so they share one aliased, paginated key. */ +function PaginatedView({ readNestedFirst }: { readNestedFirst: boolean }): React.ReactElement { + const params = { orderBy: [{ name: 'asc' as const }], limit: 10 } + const article = useEntity(entityDefs.Article, by, e => + e.id() + .attachments(params, a => a.id().name().type()) + .sections(s => s.id().article(r => r.id().attachments(params, a => a.id().name()))), + ) if (article.$isLoading || article.$isError || article.$isNotFound) { return
Loading...
} - return {article.attachments.items.map(a => a.$fields.name.value).join(',')} + const nested = (): string => article.sections.items.flatMap(s => s.article.attachments.items.map(a => a.$fields.name.value)).join(',') + const nestedBefore = readNestedFirst ? nested() : '' + const direct = article.attachments.items.map(a => String(a.$fields.type.value)).join(',') + return ( +
+ {readNestedFirst ? nestedBefore : nested()} + {direct} +
+ ) } -function NarrowList(): React.ReactElement { - const articles = useEntityList(entityDefs.Article, {}, e => e.id().attachments(a => a.id().name()).author(a => a.id().name())) - if (articles.$status !== 'ready') { +function NarrowRoot(): React.ReactElement { + const article = useEntity(entityDefs.Article, by, e => e.id().attachments(a => a.id().name()).author(a => a.id().name())) + if (article.$isLoading || article.$isError || article.$isNotFound) { return
Loading...
} - return {articles.items.flatMap(s => s.attachments.items.map(a => a.$fields.name.value)).join(',')} + return {article.attachments.items.map(a => a.$fields.name.value).join(',')} } function AttachmentRoot(): React.ReactElement { @@ -227,65 +267,102 @@ function AttachmentRoot(): React.ReactElement { return {String(attachment.type.value)} } -describe('the same entity loaded by two roots with different selections', () => { - test('should keep the wider root\'s fields when the narrower useEntity read lands last', async () => { - const { container } = renderWithin(new MockAdapter(createMockData(), { delay: 0 }), <>) - await waitForTestIds(container, 'wide-types', 'narrow-names') +for (const readNestedFirst of [true, false]) { + const order = readNestedFirst ? 'narrower nested occurrence read first' : 'wider direct occurrence read first' - expect(getByTestId(container, 'narrow-names').textContent).toBe('Slides') - expect(getByTestId(container, 'wide-types').textContent).toBe('pdf') - expect(getByTestId(container, 'wide-email').textContent).toBe('ann@example.com') - }) + describe(`one response reaching the same entity twice (${order})`, () => { + test('keeps the wider has-many, has-one and nested has-one fields', async () => { + const { container } = renderWithin(new MockAdapter(createMockData(), { delay: 0 }), ) + await waitForTestIds(container, 'direct') - test('should keep the wider root\'s fields when the narrower useEntityList read lands last', async () => { - const { container } = renderWithin(new MockAdapter(createMockData(), { delay: 0 }), <>) - await waitForTestIds(container, 'wide-types', 'narrow-names') + expect(getByTestId(container, 'nested').textContent).toBe('Slides@/slides.pdf,Ann') + expect(getByTestId(container, 'direct').textContent).toBe('pdf:42,ann@example.com') + expect(storedField('Attachment', 'att-1', 'type')).toBe('pdf/pdf') + expect(storedField('StoredFile', 'file-1', 'size')).toBe('42/42') + }) - expect(getByTestId(container, 'narrow-names').textContent).toBe('Slides') - expect(getByTestId(container, 'wide-types').textContent).toBe('pdf') - expect(getByTestId(container, 'wide-email').textContent).toBe('ann@example.com') - }) -}) + test('keeps the wider fields across the items of a list response', async () => { + const { container } = renderWithin(new MockAdapter(createMockData(), { delay: 0 }), ) + await waitForTestIds(container, 'direct') + + expect(getByTestId(container, 'nested').textContent).toBe('Slides') + expect(getByTestId(container, 'direct').textContent).toBe('pdf') + }) -describe('a narrower read after the related entity moved past its embedded copy', () => { - test('should keep values persisted through the wider root', async () => { - const adapter = new MockAdapter(createMockData(), { delay: 0 }) - const { container, rerender } = renderWithin(adapter, ) - await waitForTestIds(container, 'wide-types') + test('keeps the wider fields of a paginated has-many selected with params', async () => { + const { container } = renderWithin(new MockAdapter(createMockData(), { delay: 0 }), ) + await waitForTestIds(container, 'direct') - act(() => editWideRoot!('docx', 'ann@example.org')) - await act(async () => { - await persistAll!() + expect(getByTestId(container, 'nested').textContent).toBe('Slides') + expect(getByTestId(container, 'direct').textContent).toBe('pdf') }) - expect(getByTestId(container, 'wide-types').textContent).toBe('docx') - rerender() - await waitForTestIds(container, 'narrow-names') + test('keeps two entity types that share an id apart', async () => { + const adapter = new MockAdapter(createMockData({ authorId: 'att-1' }), { delay: 0 }) + const { container } = renderWithin(adapter, ) + await waitForTestIds(container, 'direct') - expect(storedField('Attachment', 'att-1', 'type')).toBe('docx/docx') - expect(storedField('Author', 'author-1', 'email')).toBe('ann@example.org/ann@example.org') - expect(getByTestId(container, 'wide-types').textContent).toBe('docx') - expect(getByTestId(container, 'wide-email').textContent).toBe('ann@example.org') + expect(getByTestId(container, 'direct').textContent).toBe('pdf:42,ann@example.com') + expect(hasStoredField('Attachment', 'att-1', 'email')).toBe(false) + expect(hasStoredField('Author', 'att-1', 'type')).toBe(false) + }) }) - test('should keep values a read through another root brought in', async () => { - const data = createMockData() - // The attachment changed on the server after the article was read. - data['Attachment'] = { 'att-1': { id: 'att-1', name: 'Slides', type: 'docx' } } - const adapter = new MockAdapter(data, { delay: 0 }) - const { container, rerender } = renderWithin(adapter, ) - await waitForTestIds(container, 'wide-types') - expect(getByTestId(container, 'wide-types').textContent).toBe('pdf') - - rerender() - await waitForTestIds(container, 'attachment-type') - expect(getByTestId(container, 'wide-types').textContent).toBe('docx') - - rerender() - await waitForTestIds(container, 'narrow-names') - - expect(storedField('Attachment', 'att-1', 'type')).toBe('docx/docx') - expect(getByTestId(container, 'wide-types').textContent).toBe('docx') - expect(getByTestId(container, 'attachment-type').textContent).toBe('docx') + describe(`separate reads behave as before (${order})`, () => { + test('a narrower read after a persist keeps the persisted values', async () => { + const adapter = new MockAdapter(createMockData(), { delay: 0 }) + const { container, rerender } = renderWithin(adapter, ) + await waitForTestIds(container, 'direct') + + act(() => editArticle!('docx', 'ann@example.org')) + await act(async () => { + await persistAll!() + }) + + rerender() + await waitForTestIds(container, 'narrow') + + expect(storedField('Attachment', 'att-1', 'type')).toBe('docx/docx') + expect(storedField('Author', 'author-1', 'email')).toBe('ann@example.org/ann@example.org') + expect(getByTestId(container, 'direct').textContent).toBe('docx:42,ann@example.org') + }) + + test('a refetch after a server change shows the new value', async () => { + const adapter = new MockAdapter(createMockData(), { delay: 0 }) + const { container, rerender } = renderWithin(adapter, ) + await waitForTestIds(container, 'direct') + expect(getByTestId(container, 'direct').textContent).toBe('pdf:42,ann@example.com') + + adapter.resetStore(createMockData({ type: 'docx' })) + rerender() + + await waitFor(() => { + expect(getByTestId(container, 'direct').textContent).toBe('docx:42,ann@example.com') + }) + expect(storedField('Attachment', 'att-1', 'type')).toBe('docx/docx') + }) + + test('a fresher read through another root is not overwritten by an older read', async () => { + const adapter = new MockAdapter(createMockData(), { delay: 0 }) + const { container, rerender } = renderWithin(adapter, ) + await waitForTestIds(container, 'direct') + + adapter.resetStore(createMockData({ type: 'docx' })) + rerender() + await waitForTestIds(container, 'attachment-type') + + rerender( + + + + + , + ) + await waitForTestIds(container, 'narrow') + + expect(storedField('Attachment', 'att-1', 'type')).toBe('docx/docx') + expect(getByTestId(container, 'attachment-type').textContent).toBe('docx') + expect(getByTestId(container, 'direct').textContent).toBe('docx:42,ann@example.com') + }) }) -}) +} diff --git a/tests/unit/store/refreshEmbeddedRelationMerge.test.ts b/tests/unit/store/refreshEmbeddedRelationMerge.test.ts deleted file mode 100644 index 074a00b3..00000000 --- a/tests/unit/store/refreshEmbeddedRelationMerge.test.ts +++ /dev/null @@ -1,242 +0,0 @@ -// Regression tests for https://github.com/contember/bindx/issues/123 -import { describe, test, expect, beforeEach } from 'bun:test' -import type { SelectionMeta, SnapshotStore } from '@contember/bindx' -import { __internal } from '@contember/bindx-react' -import { createTestStore } from '../shared/unitTestHelpers.js' - -const { createSelectionBuilder, getSelectionMeta } = __internal - -interface StoredFile { - id: string - url: string - size: number -} - -interface Attachment { - id: string - name: string - type: string - file: StoredFile | null -} - -interface Author { - id: string - name: string - email: string -} - -interface Article { - id: string - title: string - settings: { id: string; mode: string; extra?: string } | null - attachments: Attachment[] - author: Author | null -} - -const narrowAttachments = getSelectionMeta(createSelectionBuilder
().id().attachments(a => a.id().name())) -const narrowAuthor = getSelectionMeta(createSelectionBuilder
().id().author(a => a.id().name())) - -const wideArticle = (): Record => ({ - id: 's-1', - title: 'Article', - attachments: [ - { id: 'att-1', name: 'Slides', type: 'pdf' }, - { id: 'att-2', name: 'Notes', type: 'docx' }, - ], - author: { id: 'au-1', name: 'Ann', email: 'ann@example.com' }, -}) - -describe('SnapshotStore.refreshServerData — embedded relations read with a narrower selection', () => { - let store: SnapshotStore - - beforeEach(() => { - store = createTestStore() - store.setEntityData('Article', 's-1', wideArticle(), true) - }) - - test('keeps has-many item fields the narrower selection did not ask for', () => { - store.refreshServerData('Article', 's-1', { - id: 's-1', - attachments: [ - { id: 'att-1', name: 'Slides v2' }, - { id: 'att-2', name: 'Notes' }, - ], - }, false, narrowAttachments) - - const snapshot = store.getEntitySnapshot>('Article', 's-1') - const expected = [ - { id: 'att-1', name: 'Slides v2', type: 'pdf' }, - { id: 'att-2', name: 'Notes', type: 'docx' }, - ] - expect(snapshot?.serverData?.['attachments']).toEqual(expected) - expect(snapshot?.data['attachments']).toEqual(expected) - expect(snapshot?.data['title']).toBe('Article') - }) - - test('takes has-many membership and order from the server read', () => { - store.refreshServerData('Article', 's-1', { - id: 's-1', - attachments: [ - { id: 'att-3', name: 'Handout' }, - { id: 'att-1', name: 'Slides' }, - ], - }, false, narrowAttachments) - - const snapshot = store.getEntitySnapshot>('Article', 's-1') - expect(snapshot?.data['attachments']).toEqual([ - { id: 'att-3', name: 'Handout' }, - { id: 'att-1', name: 'Slides', type: 'pdf' }, - ]) - }) - - test('keeps has-one fields the narrower selection did not ask for while the target is the same', () => { - store.refreshServerData('Article', 's-1', { id: 's-1', author: { id: 'au-1', name: 'Anna' } }, false, narrowAuthor) - - const snapshot = store.getEntitySnapshot>('Article', 's-1') - expect(snapshot?.data['author']).toEqual({ id: 'au-1', name: 'Anna', email: 'ann@example.com' }) - }) - - test('replaces a has-one that points to another entity', () => { - store.refreshServerData('Article', 's-1', { id: 's-1', author: { id: 'au-2', name: 'Bob' } }, false, narrowAuthor) - - const snapshot = store.getEntitySnapshot>('Article', 's-1') - expect(snapshot?.data['author']).toEqual({ id: 'au-2', name: 'Bob' }) - }) - - test('replaces a has-one the server read disconnected', () => { - store.refreshServerData('Article', 's-1', { id: 's-1', author: null }, false, narrowAuthor) - - const snapshot = store.getEntitySnapshot>('Article', 's-1') - expect(snapshot?.data['author']).toBeNull() - }) - - test('replaces an object-valued scalar even when it carries an id', () => { - store.setEntityData('Article', 's-1', { ...wideArticle(), settings: { id: 'x', mode: 'a', extra: 'stale' } }, true) - // A JSON column: selected as a scalar, although its value is an object with an `id`. - const withSettings: SelectionMeta = { - fields: new Map([ - ['id', { fieldName: 'id', alias: 'id', path: ['id'], isRelation: false, isArray: false }], - ['settings', { fieldName: 'settings', alias: 'settings', path: ['settings'], isRelation: false, isArray: false }], - ]), - } - - store.refreshServerData('Article', 's-1', { id: 's-1', settings: { id: 'x', mode: 'b' } }, false, withSettings) - - const snapshot = store.getEntitySnapshot>('Article', 's-1') - expect(snapshot?.data['settings']).toEqual({ id: 'x', mode: 'b' }) - }) - - test('replaces relation data wholesale when no selection is given', () => { - store.refreshServerData('Article', 's-1', { id: 's-1', attachments: [{ id: 'att-1', name: 'Slides' }] }) - - const snapshot = store.getEntitySnapshot>('Article', 's-1') - expect(snapshot?.data['attachments']).toEqual([{ id: 'att-1', name: 'Slides' }]) - }) - - test('leaves the entity clean and keeps a dirty scalar edit', () => { - store.setFieldValue('Article', 's-1', ['title'], 'Local edit') - - store.refreshServerData('Article', 's-1', { - id: 's-1', - title: 'Server title', - attachments: [{ id: 'att-1', name: 'Slides' }], - }, false, getSelectionMeta(createSelectionBuilder
().id().title().attachments(a => a.id().name()))) - - const snapshot = store.getEntitySnapshot>('Article', 's-1') - expect(snapshot?.data['title']).toBe('Local edit') - expect(snapshot?.serverData?.['title']).toBe('Server title') - expect(snapshot?.data['attachments']).toBe(snapshot?.serverData?.['attachments']) - - store.resetEntity('Article', 's-1') - const reset = store.getEntitySnapshot>('Article', 's-1') - expect(reset?.data['title']).toBe('Server title') - expect(reset?.data['attachments']).toEqual([{ id: 'att-1', name: 'Slides', type: 'pdf' }]) - }) - - test('prefers the related entity\'s own snapshot over the embedded copy for keys it did not read', () => { - // The attachment moved on after the article was read: persisted, or read through another path. - store.setEntityData('Attachment', 'att-1', { id: 'att-1', name: 'Slides', type: 'xlsx' }, true) - - store.refreshServerData('Article', 's-1', { - id: 's-1', - attachments: [{ id: 'att-1', name: 'Slides' }, { id: 'att-2', name: 'Notes' }], - }, false, narrowAttachments) - - const snapshot = store.getEntitySnapshot>('Article', 's-1') - expect(snapshot?.data['attachments']).toEqual([ - { id: 'att-1', name: 'Slides', type: 'xlsx' }, - { id: 'att-2', name: 'Notes', type: 'docx' }, - ]) - }) - - test('prefers the related has-one entity\'s own snapshot over the embedded copy', () => { - store.setEntityData('Author', 'au-1', { id: 'au-1', name: 'Ann', email: 'ann@example.org' }, true) - - store.refreshServerData('Article', 's-1', { id: 's-1', author: { id: 'au-1', name: 'Ann' } }, false, narrowAuthor) - - const snapshot = store.getEntitySnapshot>('Article', 's-1') - expect(snapshot?.data['author']).toEqual({ id: 'au-1', name: 'Ann', email: 'ann@example.org' }) - }) - - test('merges a relation inside a relation', () => { - store.setEntityData('Article', 's-1', { - id: 's-1', - attachments: [{ id: 'att-1', name: 'Slides', file: { id: 'f-1', url: '/a.pdf', size: 42 } }], - }, true) - - store.refreshServerData('Article', 's-1', { - id: 's-1', - attachments: [{ id: 'att-1', file: { id: 'f-1', url: '/b.pdf' } }], - }, false, getSelectionMeta(createSelectionBuilder
().id().attachments(a => a.id().file(f => f.id().url())))) - - const snapshot = store.getEntitySnapshot>('Article', 's-1') - expect(snapshot?.data['attachments']).toEqual([{ id: 'att-1', name: 'Slides', file: { id: 'f-1', url: '/b.pdf', size: 42 } }]) - }) - - test('merges an aliased has-many selected with params and keeps its total count', () => { - const withParams = getSelectionMeta( - createSelectionBuilder
().id().attachments({ orderBy: [{ name: 'asc' }], limit: 1, totalCount: true }, a => a.id().name()), - ) - const alias = [...withParams.fields.values()].find(field => field.fieldName === 'attachments')?.alias - if (!alias || alias === 'attachments') throw new Error('Expected an aliased has-many') - store.setEntityData('Article', 's-1', { id: 's-1', [alias]: [{ id: 'att-1', name: 'Slides', type: 'pdf' }] }, true) - - const incomingItems = [{ id: 'att-1', name: 'Slides v2' }] - Object.defineProperty(incomingItems, 'totalCount', { value: 2, enumerable: false, writable: false }) - store.refreshServerData('Article', 's-1', { id: 's-1', [alias]: incomingItems }, false, withParams) - - const merged = store.getEntitySnapshot>('Article', 's-1')?.data[alias] - expect(merged).toEqual([{ id: 'att-1', name: 'Slides v2', type: 'pdf' }]) - expect(Object.getOwnPropertyDescriptor(merged, 'totalCount')).toEqual({ value: 2, enumerable: false, writable: false, configurable: false }) - }) - - test('merges the nodes of a connection by id', () => { - store.setEntityData('Article', 's-1', { - id: 's-1', - attachments: { pageInfo: { totalCount: 1 }, edges: [{ node: { id: 'att-1', name: 'Slides', type: 'pdf' } }] }, - }, true) - - store.refreshServerData('Article', 's-1', { - id: 's-1', - attachments: { pageInfo: { totalCount: 2 }, edges: [{ node: { id: 'att-2', name: 'Notes' } }, { node: { id: 'att-1', name: 'Slides' } }] }, - }, false, narrowAttachments) - - const snapshot = store.getEntitySnapshot>('Article', 's-1') - expect(snapshot?.data['attachments']).toEqual({ - pageInfo: { totalCount: 2 }, - edges: [{ node: { id: 'att-2', name: 'Notes' } }, { node: { id: 'att-1', name: 'Slides', type: 'pdf' } }], - }) - }) - - test('stores the incoming relation value itself when it leaves nothing to keep', () => { - const incoming = [ - { id: 'att-1', name: 'Slides', type: 'pdf' }, - { id: 'att-2', name: 'Notes', type: 'docx' }, - ] - - store.refreshServerData('Article', 's-1', { id: 's-1', attachments: incoming }, false, - getSelectionMeta(createSelectionBuilder
().id().attachments(a => a.id().name().type()))) - - expect(store.getEntitySnapshot>('Article', 's-1')?.serverData['attachments']).toBe(incoming) - }) -}) diff --git a/tests/unit/store/responseOccurrenceIndex.test.ts b/tests/unit/store/responseOccurrenceIndex.test.ts new file mode 100644 index 00000000..a85ecebe --- /dev/null +++ b/tests/unit/store/responseOccurrenceIndex.test.ts @@ -0,0 +1,159 @@ +// Regression tests for https://github.com/contember/bindx/issues/123 +import { describe, expect, test } from 'bun:test' +import { __internal } from '@contember/bindx-react' +import { ResponseOccurrenceIndex, type RelationTargetResolver } from '../../../packages/bindx/src/store/ResponseOccurrenceIndex.js' + +const { createSelectionBuilder, getSelectionMeta } = __internal + +interface Tag { + id: string + name: string + color: string +} + +interface Author { + id: string + name: string + email: string + tags: Tag[] +} + +interface Article { + id: string + title: string + author: Author | null + coAuthor: Author | null + tags: Tag[] +} + +const relations: Record> = { + Article: { + author: { target: 'Author', isHasMany: false }, + coAuthor: { target: 'Author', isHasMany: false }, + tags: { target: 'Tag', isHasMany: true }, + }, + Author: { + tags: { target: 'Tag', isHasMany: true }, + }, +} + +const schema: RelationTargetResolver = { + getRelationTarget: (entityType, fieldName) => relations[entityType]?.[fieldName]?.target, + isHasMany: (entityType, fieldName) => relations[entityType]?.[fieldName]?.isHasMany ?? false, +} + +describe('ResponseOccurrenceIndex', () => { + test('resolves every occurrence of an entity to the union of their fields', () => { + const selection = getSelectionMeta( + createSelectionBuilder
().id().author(a => a.id().name()).coAuthor(a => a.id().email()), + ) + const author = { id: 'author-1', name: 'Ann' } + const coAuthor = { id: 'author-1', email: 'ann@example.com' } + const index = new ResponseOccurrenceIndex() + + index.index('Article', { id: 'article-1', author, coAuthor }, selection, schema) + + const expected = { id: 'author-1', name: 'Ann', email: 'ann@example.com' } + expect(index.resolve(author)).toEqual(expected) + expect(index.resolve(coAuthor)).toBe(index.resolve(author)) + }) + + test('returns an occurrence with no sibling as is', () => { + const selection = getSelectionMeta(createSelectionBuilder
().id().author(a => a.id().name())) + const author = { id: 'author-1', name: 'Ann' } + const index = new ResponseOccurrenceIndex() + + index.index('Article', { id: 'article-1', author }, selection, schema) + + expect(index.resolve(author)).toBe(author) + }) + + test('unites relations inside relations level by level', () => { + const selection = getSelectionMeta( + createSelectionBuilder
() + .id() + .author(a => a.id().tags(t => t.id().name())) + .coAuthor(a => a.id().tags(t => t.id().color())), + ) + const narrowTag = { id: 'tag-1', name: 'News' } + const otherTag = { id: 'tag-1', color: 'red' } + const index = new ResponseOccurrenceIndex() + + index.index('Article', { + id: 'article-1', + author: { id: 'author-1', tags: [narrowTag] }, + coAuthor: { id: 'author-1', tags: [otherTag] }, + }, selection, schema) + + expect(index.resolve(narrowTag)).toEqual({ id: 'tag-1', name: 'News', color: 'red' }) + }) + + test('unites the nodes of a paginated connection by id', () => { + const selection = getSelectionMeta( + createSelectionBuilder
().id().tags(t => t.id().name()).author(a => a.id().tags(t => t.id().color())), + ) + const node = { id: 'tag-1', name: 'News' } + const otherNode = { id: 'tag-1', color: 'red' } + const index = new ResponseOccurrenceIndex() + + index.index('Article', { + id: 'article-1', + tags: { pageInfo: { totalCount: 1 }, edges: [{ node }] }, + author: { id: 'author-1', tags: { pageInfo: { totalCount: 1 }, edges: [{ node: otherNode }] } }, + }, selection, schema) + + expect(index.resolve(node)).toEqual({ id: 'tag-1', name: 'News', color: 'red' }) + }) + + test('keeps entity types that share an id apart', () => { + const selection = getSelectionMeta(createSelectionBuilder
().id().author(a => a.id().name()).tags(t => t.id().color())) + const author = { id: 'shared', name: 'Ann' } + const tag = { id: 'shared', color: 'red' } + const index = new ResponseOccurrenceIndex() + + index.index('Article', { id: 'article-1', author, tags: [tag] }, selection, schema) + + expect(index.resolve(author)).toBe(author) + expect(index.resolve(tag)).toBe(tag) + }) + + test('leaves out a relation whose target type the schema does not know', () => { + const selection = getSelectionMeta(createSelectionBuilder
().id().author(a => a.id().name()).coAuthor(a => a.id().email())) + const author = { id: 'author-1', name: 'Ann' } + const coAuthor = { id: 'author-1', email: 'ann@example.com' } + const index = new ResponseOccurrenceIndex() + const withoutCoAuthor: RelationTargetResolver = { + getRelationTarget: (entityType, fieldName) => (fieldName === 'coAuthor' ? undefined : schema.getRelationTarget(entityType, fieldName)), + isHasMany: schema.isHasMany, + } + + index.index('Article', { id: 'article-1', author, coAuthor }, selection, withoutCoAuthor) + + expect(index.resolve(author)).toBe(author) + expect(index.resolve(coAuthor)).toBe(coAuthor) + }) + + test('takes a scalar the occurrences disagree on from the one visited last', () => { + const selection = getSelectionMeta(createSelectionBuilder
().id().author(a => a.id().name()).coAuthor(a => a.id().name())) + const author = { id: 'author-1', name: 'Ann' } + const index = new ResponseOccurrenceIndex() + + index.index('Article', { id: 'article-1', author, coAuthor: { id: 'author-1', name: 'Anna' } }, selection, schema) + + // Breadth first, relations in selection order: `coAuthor` is visited after `author`. + expect(index.resolve(author)['name']).toBe('Anna') + }) + + test('indexes the items of a list response together', () => { + const selection = getSelectionMeta(createSelectionBuilder
().id().author(a => a.id().name()).coAuthor(a => a.id().email())) + const author = { id: 'author-1', name: 'Ann' } + const index = new ResponseOccurrenceIndex() + + index.index('Article', [ + { id: 'article-1', author }, + { id: 'article-2', coAuthor: { id: 'author-1', email: 'ann@example.com' } }, + ], selection, schema) + + expect(index.resolve(author)).toEqual({ id: 'author-1', name: 'Ann', email: 'ann@example.com' }) + }) +}) From f3c603d2e429d6aad552ef080e348503c33068c4 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Mon, 28 Sep 2026 16:05:53 +0200 Subject: [PATCH 6/7] perf(bindx): skip the response walk when the selection repeats no entity type Occurrences of one entity type at one selection node carry the same fields, so uniting them changes nothing. A walk plan, computed once per schema, selection and root type, lists the entity types the selection reaches at two or more nodes and the selection nodes on a path to them. A selection with no repeated type skips the walk; otherwise only those nodes are walked and only those types are collected. Also documents that EntityLoader writes responses without uniting occurrences, since it has a QuerySpec but neither the selection nor the schema. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Ge3kRufpi9b9HMBSBjRze5 --- packages/bindx/src/core/EntityLoader.ts | 7 ++ .../src/store/ResponseOccurrenceIndex.ts | 85 ++++++++++++++- .../store/responseOccurrenceIndex.test.ts | 100 +++++++++++++++++- 3 files changed, 184 insertions(+), 8 deletions(-) diff --git a/packages/bindx/src/core/EntityLoader.ts b/packages/bindx/src/core/EntityLoader.ts index 7c758fad..5c163aa7 100644 --- a/packages/bindx/src/core/EntityLoader.ts +++ b/packages/bindx/src/core/EntityLoader.ts @@ -60,6 +60,13 @@ export interface LoadEntityListOptions { /** * Non-React service for loading entities. * Can be used in any JavaScript environment. + * + * It writes each response as it arrives, without uniting the occurrences of one + * entity within it (see `SnapshotStore.indexServerResponse`): that needs the + * selection and the schema, and the loader has only a `QuerySpec`. A query that + * reaches one entity through two paths with different sub-selections can + * therefore lose the wider one's fields, as described in + * https://github.com/contember/bindx/issues/123. The React hooks do not go through it. */ export class EntityLoader { constructor( diff --git a/packages/bindx/src/store/ResponseOccurrenceIndex.ts b/packages/bindx/src/store/ResponseOccurrenceIndex.ts index b821901a..13b92002 100644 --- a/packages/bindx/src/store/ResponseOccurrenceIndex.ts +++ b/packages/bindx/src/store/ResponseOccurrenceIndex.ts @@ -45,9 +45,19 @@ export class ResponseOccurrenceIndex { * nested occurrence comes from the schema through the selection's field names; * an occurrence whose type cannot be resolved is left out rather than matched * by id alone. + * + * Occurrences of one type at one selection node carry the same fields, so + * their union changes nothing. Only types the selection reaches at two or more + * nodes are collected, only the relations leading to them are walked, and a + * selection with none skips the walk entirely (see {@link planOccurrenceWalk}). */ index(entityType: string, data: unknown, selection: SelectionMeta, schema: RelationTargetResolver): void { - const occurrencesByType = collectOccurrences({ value: data, entityType, selection, isList: Array.isArray(data) }, schema) + const plan = planOccurrenceWalk(entityType, selection, schema) + if (plan.repeatedTypes.size === 0) { + return + } + const root: PendingVisit = { value: data, entityType, selection, isList: Array.isArray(data) } + const occurrencesByType = collectOccurrences(root, plan, schema) for (const occurrencesById of occurrencesByType.values()) { for (const occurrences of occurrencesById.values()) { if (Array.isArray(occurrences)) { @@ -77,15 +87,17 @@ export class ResponseOccurrenceIndex { /** Occurrences by entity type and id. A lone occurrence is kept as is; an array only appears once an id repeats. */ type OccurrencesByType = Map> -function collectOccurrences(root: PendingVisit, schema: RelationTargetResolver): OccurrencesByType { +function collectOccurrences(root: PendingVisit, plan: OccurrenceWalkPlan, schema: RelationTargetResolver): OccurrencesByType { const occurrencesByType: OccurrencesByType = new Map() const pending: PendingVisit[] = [root] for (let next = 0; next < pending.length; next++) { const visit = pending[next]! for (const entity of entitiesOf(visit.value, visit.isList)) { - addOccurrence(occurrencesByType, visit.entityType, entity) + if (plan.repeatedTypes.has(visit.entityType)) { + addOccurrence(occurrencesByType, visit.entityType, entity) + } for (const fieldMeta of visit.selection.fields.values()) { - if (!fieldMeta.isRelation || !fieldMeta.nested || entity[fieldMeta.alias] == null) continue + if (!fieldMeta.nested || !plan.nodesToWalk.has(fieldMeta.nested) || entity[fieldMeta.alias] == null) continue const targetType = schema.getRelationTarget(visit.entityType, fieldMeta.fieldName) if (targetType === undefined) continue pending.push({ @@ -100,6 +112,71 @@ function collectOccurrences(root: PendingVisit, schema: RelationTargetResolver): return occurrencesByType } +/** What walking a response needs from its selection. */ +export interface OccurrenceWalkPlan { + /** Entity types the selection reaches at two or more nodes, such as `article` and `article.sections.article`. Only these are united. */ + readonly repeatedTypes: ReadonlySet + /** Selection nodes on a path to a repeated type. The walk descends into no other. */ + readonly nodesToWalk: ReadonlySet +} + +const planCache = new WeakMap>>() + +/** Depends only on the schema, the selection and its root type, so it is computed once per combination. */ +export function planOccurrenceWalk(entityType: string, selection: SelectionMeta, schema: RelationTargetResolver): OccurrenceWalkPlan { + let bySelection = planCache.get(schema) + if (!bySelection) { + bySelection = new WeakMap() + planCache.set(schema, bySelection) + } + let byRootType = bySelection.get(selection) + if (!byRootType) { + byRootType = new Map() + bySelection.set(selection, byRootType) + } + let plan = byRootType.get(entityType) + if (!plan) { + plan = computeOccurrenceWalkPlan(entityType, selection, schema) + byRootType.set(entityType, plan) + } + return plan +} + +interface SelectionNode { + readonly entityType: string + readonly selection: SelectionMeta + readonly parent: number | null +} + +function computeOccurrenceWalkPlan(entityType: string, selection: SelectionMeta, schema: RelationTargetResolver): OccurrenceWalkPlan { + const nodes: SelectionNode[] = [{ entityType, selection, parent: null }] + const seenTypes = new Set() + const repeatedTypes = new Set() + for (let index = 0; index < nodes.length; index++) { + const node = nodes[index]! + if (seenTypes.has(node.entityType)) { + repeatedTypes.add(node.entityType) + } + seenTypes.add(node.entityType) + for (const fieldMeta of node.selection.fields.values()) { + if (!fieldMeta.isRelation || !fieldMeta.nested) continue + const targetType = schema.getRelationTarget(node.entityType, fieldMeta.fieldName) + if (targetType !== undefined) { + nodes.push({ entityType: targetType, selection: fieldMeta.nested, parent: index }) + } + } + } + const nodesToWalk = new Set() + for (const node of nodes) { + if (!repeatedTypes.has(node.entityType)) continue + for (let current: SelectionNode | undefined = node; current && !nodesToWalk.has(current.selection);) { + nodesToWalk.add(current.selection) + current = current.parent === null ? undefined : nodes[current.parent] + } + } + return { repeatedTypes, nodesToWalk } +} + function addOccurrence(occurrencesByType: OccurrencesByType, entityType: string, entity: EntityRecord): void { const id = entity['id'] if (typeof id !== 'string' && typeof id !== 'number') return diff --git a/tests/unit/store/responseOccurrenceIndex.test.ts b/tests/unit/store/responseOccurrenceIndex.test.ts index a85ecebe..8afe21bd 100644 --- a/tests/unit/store/responseOccurrenceIndex.test.ts +++ b/tests/unit/store/responseOccurrenceIndex.test.ts @@ -1,7 +1,7 @@ // Regression tests for https://github.com/contember/bindx/issues/123 import { describe, expect, test } from 'bun:test' import { __internal } from '@contember/bindx-react' -import { ResponseOccurrenceIndex, type RelationTargetResolver } from '../../../packages/bindx/src/store/ResponseOccurrenceIndex.js' +import { planOccurrenceWalk, ResponseOccurrenceIndex, type RelationTargetResolver } from '../../../packages/bindx/src/store/ResponseOccurrenceIndex.js' const { createSelectionBuilder, getSelectionMeta } = __internal @@ -18,12 +18,18 @@ interface Author { tags: Tag[] } +interface Section { + id: string + article: Article | null +} + interface Article { id: string title: string author: Author | null coAuthor: Author | null tags: Tag[] + sections: Section[] } const relations: Record> = { @@ -31,6 +37,10 @@ const relations: Record { }) test('keeps entity types that share an id apart', () => { - const selection = getSelectionMeta(createSelectionBuilder
().id().author(a => a.id().name()).tags(t => t.id().color())) - const author = { id: 'shared', name: 'Ann' } + const selection = getSelectionMeta( + createSelectionBuilder
() + .id() + .author(a => a.id().name().tags(t => t.id().name())) + .coAuthor(a => a.id().email()) + .tags(t => t.id().color()), + ) + const author = { id: 'shared', name: 'Ann', tags: [{ id: 'tag-1', name: 'News' }] } const tag = { id: 'shared', color: 'red' } const index = new ResponseOccurrenceIndex() - index.index('Article', { id: 'article-1', author, tags: [tag] }, selection, schema) + index.index('Article', { id: 'article-1', author, coAuthor: { id: 'author-2', email: 'bob@example.com' }, tags: [tag] }, selection, schema) expect(index.resolve(author)).toBe(author) expect(index.resolve(tag)).toBe(tag) @@ -156,4 +172,80 @@ describe('ResponseOccurrenceIndex', () => { expect(index.resolve(author)).toEqual({ id: 'author-1', name: 'Ann', email: 'ann@example.com' }) }) + + describe('fast path', () => { + const countingSchema = (): { resolver: RelationTargetResolver; calls: () => number } => { + let calls = 0 + return { + resolver: { + getRelationTarget: (entityType, fieldName) => { + calls++ + return schema.getRelationTarget(entityType, fieldName) + }, + isHasMany: schema.isHasMany, + }, + calls: () => calls, + } + } + const row = (i: number): { article: Record; nestedArticle: Record } => { + const nestedArticle = { id: `article-${i}`, author: { id: 'author-1', name: 'Ann' } } + const article = { id: `article-${i}`, author: { id: 'author-1', name: 'Ann' }, sections: [{ id: `section-${i}`, article: nestedArticle }] } + return { article, nestedArticle } + } + const rows = (count: number): Record[] => Array.from({ length: count }, (_, i) => row(i).article) + + test('finds no repeated type in a selection that reaches every type once', () => { + const selection = getSelectionMeta(createSelectionBuilder
().id().author(a => a.id().name()).tags(t => t.id().name())) + + expect([...planOccurrenceWalk('Article', selection, schema).repeatedTypes]).toEqual([]) + }) + + test('finds the type a cycle reaches twice', () => { + const selection = getSelectionMeta( + createSelectionBuilder
().id().tags(t => t.id().name()).sections(s => s.id().article(a => a.id().tags(t => t.id().color()))), + ) + const plan = planOccurrenceWalk('Article', selection, schema) + + expect([...plan.repeatedTypes].sort()).toEqual(['Article', 'Tag']) + }) + + test('walks only the relations that lead to a repeated type', () => { + const selection = getSelectionMeta( + createSelectionBuilder
().id().author(a => a.id().name()).sections(s => s.id().article(a => a.id().title())), + ) + const plan = planOccurrenceWalk('Article', selection, schema) + + expect([...plan.repeatedTypes]).toEqual(['Article']) + expect(plan.nodesToWalk.has(selection)).toBe(true) + expect(plan.nodesToWalk.has(selection.fields.get('sections')!.nested!)).toBe(true) + expect(plan.nodesToWalk.has(selection.fields.get('author')!.nested!)).toBe(false) + }) + + test('skips walking a response whose selection repeats no type', () => { + const selection = getSelectionMeta(createSelectionBuilder
().id().author(a => a.id().name())) + const { resolver, calls } = countingSchema() + const index = new ResponseOccurrenceIndex() + + index.index('Article', rows(100), selection, resolver) + const afterFirst = calls() + index.index('Article', rows(100), selection, resolver) + + expect(afterFirst).toBeLessThan(10) + expect(calls()).toBe(afterFirst) + }) + + test('walks a response whose selection reaches a type twice', () => { + const selection = getSelectionMeta( + createSelectionBuilder
().id().author(a => a.id().name()).sections(s => s.id().article(a => a.id().author(u => u.id().name()))), + ) + const { resolver, calls } = countingSchema() + const index = new ResponseOccurrenceIndex() + const first = row(0) + + index.index('Article', [first.article, ...rows(100).slice(1)], selection, resolver) + + expect(calls()).toBeGreaterThan(100) + expect(index.resolve(first.nestedArticle)).toBe(index.resolve(first.article)) + }) + }) }) From 7271c25fcb089cb445b349ff31bea7e5e76dd023 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Mon, 28 Sep 2026 16:20:31 +0200 Subject: [PATCH 7/7] fix(bindx): walk every position a reused fragment takes The walk plan climbed from each position of a repeated type to the root and stopped at the first selection object it had already marked. A reused fragment is one selection object at several positions, so the climb from its second position stopped at once and never marked that position's own ancestors: the branch was not walked, its occurrences were not united, and a narrower fragment occurrence could again replace fields a wider occurrence of the same entity carried. The climb now tracks tree positions. The plan is no longer cached per selection. A selection can still grow after its first fetch (mergeSelections extends nested selections in place, and a fragment's selection is shared), and computing the plan is one pass over the selection tree, independent of the response size. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Ge3kRufpi9b9HMBSBjRze5 --- .../src/store/ResponseOccurrenceIndex.ts | 73 ++++++++------- ...nestedSameEntityNarrowerSelection.test.tsx | 89 ++++++++++++++++++- .../store/responseOccurrenceIndex.test.ts | 36 ++++++-- 3 files changed, 154 insertions(+), 44 deletions(-) diff --git a/packages/bindx/src/store/ResponseOccurrenceIndex.ts b/packages/bindx/src/store/ResponseOccurrenceIndex.ts index 13b92002..30527164 100644 --- a/packages/bindx/src/store/ResponseOccurrenceIndex.ts +++ b/packages/bindx/src/store/ResponseOccurrenceIndex.ts @@ -120,26 +120,18 @@ export interface OccurrenceWalkPlan { readonly nodesToWalk: ReadonlySet } -const planCache = new WeakMap>>() - -/** Depends only on the schema, the selection and its root type, so it is computed once per combination. */ +/** + * Plans the walk of one response. It depends only on the schema, the selection + * and its root type, and costs one pass over the selection tree, not over the + * response. It is computed per response rather than cached, because a selection + * can still grow after its first fetch (`mergeSelections` extends nested + * selections in place, and a fragment's selection is shared by every place + * that uses it). + */ export function planOccurrenceWalk(entityType: string, selection: SelectionMeta, schema: RelationTargetResolver): OccurrenceWalkPlan { - let bySelection = planCache.get(schema) - if (!bySelection) { - bySelection = new WeakMap() - planCache.set(schema, bySelection) - } - let byRootType = bySelection.get(selection) - if (!byRootType) { - byRootType = new Map() - bySelection.set(selection, byRootType) - } - let plan = byRootType.get(entityType) - if (!plan) { - plan = computeOccurrenceWalkPlan(entityType, selection, schema) - byRootType.set(entityType, plan) - } - return plan + const nodes = listSelectionNodes(entityType, selection, schema) + const repeatedTypes = findRepeatedTypes(nodes) + return { repeatedTypes, nodesToWalk: findNodesToWalk(nodes, repeatedTypes) } } interface SelectionNode { @@ -148,16 +140,11 @@ interface SelectionNode { readonly parent: number | null } -function computeOccurrenceWalkPlan(entityType: string, selection: SelectionMeta, schema: RelationTargetResolver): OccurrenceWalkPlan { +/** Every position in the selection tree, breadth first, each with the index of its parent. */ +function listSelectionNodes(entityType: string, selection: SelectionMeta, schema: RelationTargetResolver): readonly SelectionNode[] { const nodes: SelectionNode[] = [{ entityType, selection, parent: null }] - const seenTypes = new Set() - const repeatedTypes = new Set() for (let index = 0; index < nodes.length; index++) { const node = nodes[index]! - if (seenTypes.has(node.entityType)) { - repeatedTypes.add(node.entityType) - } - seenTypes.add(node.entityType) for (const fieldMeta of node.selection.fields.values()) { if (!fieldMeta.isRelation || !fieldMeta.nested) continue const targetType = schema.getRelationTarget(node.entityType, fieldMeta.fieldName) @@ -166,15 +153,37 @@ function computeOccurrenceWalkPlan(entityType: string, selection: SelectionMeta, } } } - const nodesToWalk = new Set() + return nodes +} + +function findRepeatedTypes(nodes: readonly SelectionNode[]): ReadonlySet { + const seenTypes = new Set() + const repeatedTypes = new Set() for (const node of nodes) { - if (!repeatedTypes.has(node.entityType)) continue - for (let current: SelectionNode | undefined = node; current && !nodesToWalk.has(current.selection);) { - nodesToWalk.add(current.selection) - current = current.parent === null ? undefined : nodes[current.parent] + if (seenTypes.has(node.entityType)) { + repeatedTypes.add(node.entityType) } + seenTypes.add(node.entityType) } - return { repeatedTypes, nodesToWalk } + return repeatedTypes +} + +/** + * The selections on a path from the root to a repeated type. The climb tracks + * tree positions, not selection objects: a reused fragment is one selection + * object at several positions, and each position has ancestors of its own. + */ +function findNodesToWalk(nodes: readonly SelectionNode[], repeatedTypes: ReadonlySet): ReadonlySet { + const nodesToWalk = new Set() + const climbed = new Set() + nodes.forEach((node, index) => { + if (!repeatedTypes.has(node.entityType)) return + for (let position: number | null = index; position !== null && !climbed.has(position); position = nodes[position]!.parent) { + climbed.add(position) + nodesToWalk.add(nodes[position]!.selection) + } + }) + return nodesToWalk } function addOccurrence(occurrencesByType: OccurrencesByType, entityType: string, entity: EntityRecord): void { diff --git a/tests/react/relations/hasMany/nestedSameEntityNarrowerSelection.test.tsx b/tests/react/relations/hasMany/nestedSameEntityNarrowerSelection.test.tsx index daab31dd..a66ac8d8 100644 --- a/tests/react/relations/hasMany/nestedSameEntityNarrowerSelection.test.tsx +++ b/tests/react/relations/hasMany/nestedSameEntityNarrowerSelection.test.tsx @@ -5,6 +5,7 @@ import { act, cleanup, render, waitFor } from '@testing-library/react' import React from 'react' import { BindxProvider, + createFragment, defineSchema, entityDef, hasMany, @@ -36,15 +37,28 @@ interface Attachment { file: StoredFile | null } +interface Company { + id: string + address: string +} + +interface Badge { + id: string + label: string +} + interface Author { id: string name: string email: string + company: Company | null + badges: Badge[] } interface Section { id: string article: Article | null + reviewer: Author | null } interface Article { @@ -52,6 +66,7 @@ interface Article { attachments: Attachment[] sections: Section[] author: Author | null + editor: Author | null } interface CycleSchema { @@ -60,6 +75,8 @@ interface CycleSchema { Attachment: Attachment Author: Author StoredFile: StoredFile + Company: Company + Badge: Badge } const schema = defineSchema({ @@ -70,12 +87,14 @@ const schema = defineSchema({ attachments: hasMany('Attachment'), sections: hasMany('Section'), author: hasOne('Author', { nullable: true }), + editor: hasOne('Author', { nullable: true }), }, }, Section: { fields: { id: scalar(), article: hasOne('Article', { nullable: true }), + reviewer: hasOne('Author', { nullable: true }), }, }, Attachment: { @@ -91,6 +110,20 @@ const schema = defineSchema({ id: scalar(), name: scalar(), email: scalar(), + company: hasOne('Company', { nullable: true }), + badges: hasMany('Badge'), + }, + }, + Company: { + fields: { + id: scalar(), + address: scalar(), + }, + }, + Badge: { + fields: { + id: scalar(), + label: scalar(), }, }, StoredFile: { @@ -118,16 +151,24 @@ interface MockOptions { function createMockData({ type = 'pdf', authorId = 'author-1' }: MockOptions = {}): ConstructorParameters[0] { const file = { id: 'file-1', url: '/slides.pdf', size: 42 } const attachment = { id: 'att-1', name: 'Slides', type, file } - const author = { id: authorId, name: 'Ann', email: 'ann@example.com' } - const article = { id: 'article-1', attachments: [attachment], author, sections: [] as unknown[] } - // The section points back at the SAME article: one response reaches it twice. - article.sections = [{ id: 'section-1', article }] + const author = { + id: authorId, + name: 'Ann', + email: 'ann@example.com', + company: { id: 'company-1', address: 'Main St' }, + badges: [{ id: 'badge-1', label: 'Maintainer' }], + } + const article = { id: 'article-1', attachments: [attachment], author, editor: author, sections: [] as unknown[] } + // The section points back at the SAME article, and at the same person as its author: one response reaches both twice. + article.sections = [{ id: 'section-1', article, reviewer: author }] return { Article: { 'article-1': article }, Section: {}, Attachment: { 'att-1': { ...attachment } }, Author: { [authorId]: { ...author } }, StoredFile: {}, + Company: {}, + Badge: {}, } } @@ -251,6 +292,38 @@ function PaginatedView({ readNestedFirst }: { readNestedFirst: boolean }): React ) } +const PersonFragment = createFragment()(a => a.id().name().company(c => c.id()).badges(b => b.id())) + +/** + * One person reached three times: through the same fragment as the article's author and as a + * section's reviewer (a has-many away), and as the editor with a wider has-one and has-many. + * `readNestedFirst` reads the fragment positions before the editor; otherwise the editor is + * touched first, the fragment positions next, and the editor's relations last. + */ +function SharedFragmentView({ readNestedFirst }: { readNestedFirst: boolean }): React.ReactElement { + const article = useEntity(entityDefs.Article, by, e => + e.id() + .author(PersonFragment) + .editor(p => p.id().company(c => c.id().address()).badges(b => b.id().label())) + .sections(s => s.id().reviewer(PersonFragment)), + ) + if (article.$isLoading || article.$isError || article.$isNotFound) { + return
Loading...
+ } + const fragmentNames = (): string => [article.author.name.value, ...article.sections.items.map(s => s.reviewer.name.value)].join(',') + const editorWide = (): string => [article.editor.company.address.value, ...article.editor.badges.items.map(b => b.$fields.label.value)].join(',') + + const editorId = readNestedFirst ? null : article.editor.$id + const names = fragmentNames() + const wide = editorWide() + return ( +
+ {names} + {wide} +
+ ) +} + function NarrowRoot(): React.ReactElement { const article = useEntity(entityDefs.Article, by, e => e.id().attachments(a => a.id().name()).author(a => a.id().name())) if (article.$isLoading || article.$isError || article.$isNotFound) { @@ -297,6 +370,14 @@ for (const readNestedFirst of [true, false]) { expect(getByTestId(container, 'direct').textContent).toBe('pdf') }) + test('keeps the wider has-one and has-many fields of an entity also selected through a reused fragment', async () => { + const { container } = renderWithin(new MockAdapter(createMockData(), { delay: 0 }), ) + await waitForTestIds(container, 'direct') + + expect(getByTestId(container, 'nested').textContent).toBe('Ann,Ann') + expect(getByTestId(container, 'direct').textContent).toBe('Main St,Maintainer') + }) + test('keeps two entity types that share an id apart', async () => { const adapter = new MockAdapter(createMockData({ authorId: 'att-1' }), { delay: 0 }) const { container } = renderWithin(adapter, ) diff --git a/tests/unit/store/responseOccurrenceIndex.test.ts b/tests/unit/store/responseOccurrenceIndex.test.ts index 8afe21bd..1d9fe370 100644 --- a/tests/unit/store/responseOccurrenceIndex.test.ts +++ b/tests/unit/store/responseOccurrenceIndex.test.ts @@ -1,6 +1,6 @@ // Regression tests for https://github.com/contember/bindx/issues/123 import { describe, expect, test } from 'bun:test' -import { __internal } from '@contember/bindx-react' +import { __internal, createFragment } from '@contember/bindx-react' import { planOccurrenceWalk, ResponseOccurrenceIndex, type RelationTargetResolver } from '../../../packages/bindx/src/store/ResponseOccurrenceIndex.js' const { createSelectionBuilder, getSelectionMeta } = __internal @@ -21,6 +21,7 @@ interface Author { interface Section { id: string article: Article | null + reviewer: Author | null } interface Article { @@ -41,6 +42,7 @@ const relations: Record { test('skips walking a response whose selection repeats no type', () => { const selection = getSelectionMeta(createSelectionBuilder
().id().author(a => a.id().name())) - const { resolver, calls } = countingSchema() - const index = new ResponseOccurrenceIndex() + const callsFor = (rowCount: number): number => { + const { resolver, calls } = countingSchema() + new ResponseOccurrenceIndex().index('Article', rows(rowCount), selection, resolver) + return calls() + } + + // Only the plan consults the schema, once per selection position; the rows are never walked. + expect(callsFor(1000)).toBe(callsFor(10)) + expect(callsFor(10)).toBeLessThan(10) + }) + + test('walks the ancestors of every position a reused fragment takes', () => { + const PersonFragment = createFragment()(a => a.id().name()) + const selection = getSelectionMeta( + createSelectionBuilder
().id().author(PersonFragment).sections(s => s.id().reviewer(PersonFragment)), + ) + const plan = planOccurrenceWalk('Article', selection, schema) - index.index('Article', rows(100), selection, resolver) - const afterFirst = calls() - index.index('Article', rows(100), selection, resolver) + expect([...plan.repeatedTypes]).toEqual(['Author']) + expect(plan.nodesToWalk.has(selection.fields.get('sections')!.nested!)).toBe(true) + + const reviewer = { id: 'author-1', name: 'Ann' } + const author = { id: 'author-1', name: 'Ann' } + const index = new ResponseOccurrenceIndex() + index.index('Article', { id: 'article-1', author, sections: [{ id: 'section-1', reviewer }] }, selection, schema) - expect(afterFirst).toBeLessThan(10) - expect(calls()).toBe(afterFirst) + expect(index.resolve(reviewer)).toBe(index.resolve(author)) }) test('walks a response whose selection reaches a type twice', () => {