diff --git a/.changeset/collection-push-readd.md b/.changeset/collection-push-readd.md new file mode 100644 index 000000000000..23d76be18c47 --- /dev/null +++ b/.changeset/collection-push-readd.md @@ -0,0 +1,15 @@ +--- +'@data-client/endpoint': patch +'@data-client/graphql': patch +'@data-client/rest': patch +--- + +Fix Collection push and unshift showing an item twice after it was deleted and added back + +After an item was deleted, adding an item with the same id through [push](https://dataclient.io/rest/api/Collection#push) or [unshift](https://dataclient.io/rest/api/Collection#unshift) (for example `getList.push`) listed it twice, while [All](https://dataclient.io/rest/api/All) counted it once. This happens with lists keyed by another entity's id, like cart lines keyed by product or a list of followed users. Now, when an added item was deleted, its old id is dropped first, so it shows once: at the end (push) or start (unshift). Ids of items that weren't deleted are untouched, so pushing an id the list already shows still adds it again, as `Array.push()` would. + +```ts +await ctrl.fetch(CartResource.delete, { id: 'mug' }); +await ctrl.fetch(CartResource.getList.push, { id: 'mug', quantity: 1 }); +// getList shows the mug once, at the end +``` diff --git a/packages/endpoint/src/schemas/Collection.ts b/packages/endpoint/src/schemas/Collection.ts index 84af90b3aeee..b5eb159fde75 100644 --- a/packages/endpoint/src/schemas/Collection.ts +++ b/packages/endpoint/src/schemas/Collection.ts @@ -28,7 +28,8 @@ type NormalizedRef = { id: string; schema: string } | string; const isShallowEqual = (a: NormalizedRef, b: NormalizedRef): boolean => { // TODO: make this extensible in the child's schema - if (typeof a === 'object' && typeof b === 'object') { + // arrays keep null members, and typeof null is 'object' + if (typeof a === 'object' && typeof b === 'object' && a && b) { return a.id === b.id && a.schema === b.schema; } return a === b; @@ -359,26 +360,86 @@ function normalizeCreate( // visit instances are created on each normalize call so this will safely be reset visit.creating = true; } - const normalizedValue = this.schema.normalize( - !(this.schema instanceof ArraySchema) || Array.isArray(input) ? - input - : [input], - parent, - key, - delegate, - ); + const isArray = this.schema instanceof ArraySchema; + if (isArray && !Array.isArray(input)) input = [input]; + // before normalize, which revives the entity + const deletedAt = + isArray ? deletedIndexes(this.schema, input, parent, key, delegate) : []; + const normalizedValue = this.schema.normalize(input, parent, key, delegate); // parent is args when not nested const filterCollections = (this.createCollectionFilter as any)(...args); + // ArraySchema.normalize maps input 1:1, so normalizedValue[i] is input[i]'s ref + const deletedRefs: NormalizedRef[] = deletedAt.map(i => normalizedValue[i]); // add to any collections that match this + const target = deletedRefs.length ? withoutRefs(this, deletedRefs) : this; const entities = delegate.getEntities(this.key); if (entities) for (const collectionKey of entities.keys()) { if (!filterCollections(JSON.parse(collectionKey))) continue; - delegate.mergeEntity(this, collectionKey, normalizedValue); + delegate.mergeEntity(target, collectionKey, normalizedValue); } return normalizedValue as any; } +/** Indexes of the incoming items whose entity was deleted. + * + * A deleted entity's ref stays in its collections, where denormalize skips it. + * Re-creating that pk would show it twice, so its old ref is dropped first. + */ +function deletedIndexes( + members: any, + input: any[], + parent: any, + key: string, + delegate: INormalizeDelegate, +): number[] { + const indexes: number[] = []; + for (let i = 0; i < input.length; i++) { + const item = input[i]; + // refs (ids) pass through normalize, so they can't revive an entity + if (typeof item !== 'object' || item === null) continue; + const schema = members.inferSchema(item, parent, key); + // only entities have a pk; skip nested objects and unknown members + if (typeof schema?.process !== 'function' || typeof schema.key !== 'string') + continue; + const processed = schema.process(item, parent, key, delegate.args); + if (processed === undefined) continue; + const pk = `${schema.pk(processed, parent, key, delegate.args)}`; + // Invalidate stores a symbol in place of the entity + if (typeof delegate.getEntity(schema.key, pk) === 'symbol') indexes.push(i); + } + return indexes; +} + +/** Merges like `collection`, after dropping `refs` from the stored list */ +function withoutRefs( + collection: CollectionSchema, + refs: NormalizedRef[], +) { + const drop = (existing: any) => + Array.isArray(existing) ? removeMerge(existing, refs) : existing; + return Object.create(collection, { + merge: { + value: (existing: any, incoming: any) => + collection.merge(drop(existing), incoming), + }, + mergeWithStore: { + value: ( + existingMeta: any, + incomingMeta: any, + existing: any, + incoming: any, + ) => + collection.mergeWithStore( + existingMeta, + incomingMeta, + drop(existing), + incoming, + ), + }, + }); +} + function CreateMover>( collection: C, addMerge: (existing: any, incoming: any) => any, diff --git a/packages/endpoint/src/schemas/__tests__/Collection.test.ts b/packages/endpoint/src/schemas/__tests__/Collection.test.ts index 98412c243b90..60afe24cd725 100644 --- a/packages/endpoint/src/schemas/__tests__/Collection.test.ts +++ b/packages/endpoint/src/schemas/__tests__/Collection.test.ts @@ -1,6 +1,11 @@ // eslint-env jest import { initialState, State } from '@data-client/core'; -import { normalize, denormalize, MemoCache } from '@data-client/normalizr'; +import { + normalize, + denormalize, + MemoCache, + INVALID, +} from '@data-client/normalizr'; import { ArticleResource, IDEntity } from '__tests__/new'; import { Record } from 'immutable'; @@ -104,6 +109,8 @@ test('key works with custom schema', () => { }); describe(`${schema.Collection.name} normalization`, () => { + const meta = { date: 0, expiresAt: Infinity, fetchedAt: 0 }; + let warnSpy: jest.Spied; afterEach(() => { warnSpy.mockRestore(); @@ -333,6 +340,43 @@ describe(`${schema.Collection.name} normalization`, () => { ]); }); + test('push of a deleted then re-created Union member adds it once', () => { + const collectionPK = collectionUnion.pk({}, {}, '', [ + { completed: false }, + ]); + const init: State = { + ...initialState, + entities: { + [collectionUnion.key]: { + [collectionPK]: [ + null, + { id: '1', schema: 'users' }, + { id: '2', schema: 'groups' }, + ], + }, + User: { '1': INVALID }, + Group: { '2': { id: '2', type: 'groups' } }, + }, + entitiesMeta: { + [collectionUnion.key]: { [collectionPK]: meta }, + User: { '1': meta }, + Group: { '2': meta }, + }, + indexes: {}, + }; + const state = normalize( + collectionUnion.push, + [{ id: '1', type: 'users' }], + [{ completed: false }], + init, + ); + expect(state.entities[collectionUnion.key]?.[collectionPK]).toEqual([ + null, + { id: '2', schema: 'groups' }, + { id: '1', schema: 'users' }, + ]); + }); + test('remove works updates the correct collections', () => { const init = { entities: { @@ -755,6 +799,192 @@ describe(`${schema.Collection.name} normalization`, () => { ]); }); + const todo5 = { '5': { id: '5', title: 'finish collections' } }; + /** User.schema.todos lists and Todo entities, each stored with `meta` */ + const todosState = ( + lists: { [pk: string]: string[] }, + todos: { [pk: string]: unknown }, + listMeta = meta, + ) => ({ + entities: { [User.schema.todos.key]: lists, Todo: todos }, + entitiesMeta: { + [User.schema.todos.key]: Object.fromEntries( + Object.keys(lists).map(pk => [pk, listMeta]), + ), + Todo: Object.fromEntries(Object.keys(todos).map(pk => [pk, meta])), + }, + indexes: {}, + }); + + test.each<['push' | 'unshift', string[], string[]]>([ + ['push', ['10', '5'], ['5', '10']], + ['unshift', ['5', '10'], ['10', '5']], + ])( + '%s of a deleted then re-created id adds it once', + (method, before, after) => { + const init = todosState( + { '{"userId":"1"}': before }, + // deleted, so the collection still holds its id + { ...todo5, '10': INVALID }, + ); + const state = normalize( + User.schema.todos[method], + [{ id: '10', title: 'recreated' }], + [{ userId: '1' }], + init, + ); + expect(state.entities[User.schema.todos.key]['{"userId":"1"}']).toEqual( + after, + ); + }, + ); + + test('push drops only the deleted ids among several items', () => { + const init = todosState( + { '{"userId":"1"}': ['10', '5'] }, + { ...todo5, '10': INVALID }, + ); + const state = normalize( + User.schema.todos.push, + [ + { id: '5', title: 'again' }, + { id: '10', title: 'recreated' }, + ], + [{ userId: '1' }], + init, + ); + expect(state.entities[User.schema.todos.key]['{"userId":"1"}']).toEqual([ + '5', + '5', + '10', + ]); + }); + + test('re-creating a deleted id leaves collections it is not added to alone', () => { + const init = todosState( + { '{"userId":"1"}': ['10'], '{"userId":"2"}': ['10'] }, + { '10': INVALID }, + ); + const state = normalize( + User.schema.todos.push, + [{ id: '10', title: 'recreated' }], + [{ userId: '1' }], + init, + ); + expect(state.entities[User.schema.todos.key]).toEqual({ + '{"userId":"1"}': ['10'], + '{"userId":"2"}': ['10'], + }); + }); + + test('re-creating a deleted id keeps newer collection meta', () => { + const init = todosState( + { '{"userId":"1"}': ['10', '5'] }, + { ...todo5, '10': INVALID }, + { date: 100, expiresAt: Infinity, fetchedAt: 100 }, + ); + // a create response that was fetched before the list + const state = normalize( + User.schema.todos.push, + [{ id: '10', title: 'recreated' }], + [{ userId: '1' }], + init, + { date: 10, expiresAt: Infinity, fetchedAt: 10 }, + ); + expect(state.entities[User.schema.todos.key]['{"userId":"1"}']).toEqual([ + '10', + '5', + ]); + expect( + state.entitiesMeta[User.schema.todos.key]['{"userId":"1"}']?.fetchedAt, + ).toBe(100); + }); + + test('two pushes in one response each add their item once', () => { + const init = { + entities: { + [userTodos.key]: { '{"userId":"1"}': ['11', '5'] }, + Todo: { '5': { id: '5', title: 'finish collections' }, '11': INVALID }, + }, + entitiesMeta: { + [userTodos.key]: { '{"userId":"1"}': meta }, + Todo: { '5': meta, '11': meta }, + }, + indexes: {}, + }; + const state = normalize( + { first: userTodos.push, second: userTodos.push }, + { first: [{ id: '10' }], second: [{ id: '11' }] }, + [{ userId: '1' }], + init, + ); + expect(state.entities[userTodos.key]['{"userId":"1"}']).toEqual([ + '5', + '10', + '11', + ]); + }); + + test('push skips items whose process() invalidates them', () => { + class Draft extends IDEntity { + static key = 'Draft'; + static process(input: any) { + return input.discard ? undefined : { ...input }; + } + } + const drafts = new Collection([Draft]); + const state = normalize(drafts.push, [{ id: '1', discard: true }], [{}], { + entities: { [drafts.key]: { '{}': ['1'] }, Draft: { '1': INVALID } }, + entitiesMeta: { [drafts.key]: { '{}': meta }, Draft: { '1': meta } }, + indexes: {}, + }); + expect(state.entities[drafts.key]['{}']).toEqual(['1', '1']); + }); + + test('push into a collection of nested objects appends', () => { + const assignments = new Collection([{ todo: Todo }]); + const state = normalize(assignments.push, [{ todo: { id: '2' } }], [{}], { + entities: { [assignments.key]: { '{}': [{ todo: '1' }] } }, + entitiesMeta: { [assignments.key]: { '{}': meta } }, + indexes: {}, + }); + expect(state.entities[assignments.key]['{}']).toEqual([ + { todo: '1' }, + { todo: '2' }, + ]); + }); + + test('push of ids does not process them', () => { + const process = jest.spyOn(Todo, 'process'); + const init = todosState({ '{"userId":"1"}': ['5'] }, todo5); + const state = normalize( + User.schema.todos.push, + ['5'], + [{ userId: '1' }], + init, + ); + expect(process).not.toHaveBeenCalled(); + expect(state.entities[User.schema.todos.key]['{"userId":"1"}']).toEqual([ + '5', + '5', + ]); + process.mockRestore(); + }); + + test('push of an id already in the collection adds it again, like Array.push()', () => { + const init = todosState({ '{"userId":"1"}': ['5'] }, todo5); + const state = normalize( + User.schema.todos.push, + [{ id: '5', title: 'finish collections' }], + [{ userId: '1' }], + init, + ); + expect(state.entities[User.schema.todos.key]['{"userId":"1"}']).toEqual([ + '5', + '5', + ]); + }); + describe('push should add only to collections matching filterArgumentKeys', () => { const initializingSchema = new Collection([Todo]); let state = { diff --git a/website/blog/2026-10-03-v0.19-batch-set.md b/website/blog/2026-10-03-v0.19-batch-set.md index 333ed9e206fd..2d5c5e22fc79 100644 --- a/website/blog/2026-10-03-v0.19-batch-set.md +++ b/website/blog/2026-10-03-v0.19-batch-set.md @@ -56,6 +56,7 @@ Coding agents also get a Claude Code plugin, Vue skills and llms.txt; see - Polling keeps the schema of endpoints given [pollFrequency](/rest/api/RestEndpoint#pollfrequency) with `.extend()`, so a polled [useLive()](/docs/api/useLive) list updates right away from mutations and optimistic updates instead of waiting for the next poll ([#4437](https://github.com/reactive/data-client/pull/4437)) - [DataProvider](/docs/api/DataProvider) only warns that it doesn't update during SSR when something fetches while the server renders, instead of on every server render, so pages that load data after mount (like Expo Router's static web output) stay quiet ([#4449](https://github.com/reactive/data-client/pull/4449)) - Server rendering with React 19 no longer logs "Uncaught suspense" when nothing suspended; React renders and discards fallbacks of large boundaries, and the warning now waits for a fallback that actually shows ([#4450](https://github.com/reactive/data-client/pull/4450)) +- [Collection push](/rest/api/Collection#push) and `unshift` (like `getList.push`) show an item once when it was deleted and then added back, instead of twice ([#4487](https://github.com/reactive/data-client/pull/4487)) - Vue `createDataClient()` is deprecated, since it's internal to [DataClientPlugin](/vue/api/DataClientPlugin); install the plugin with `app.use(DataClientPlugin, options)` instead of calling it directly ([#4193](https://github.com/reactive/data-client/pull/4193)) - Vue `renderDataCompose()`'s `waitForNextUpdate()` is deprecated, since it gives up silently after 1 second and lets a test pass while still suspended; `await result` instead, or `await allSettled()` after a change, which now also waits for fetches a prop change starts ([unit testing composables](/vue/guides/unit-testing-composables#renderdatacompose-api), [#4221](https://github.com/reactive/data-client/pull/4221))