Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .changeset/collection-push-readd.md
Original file line number Diff line number Diff line change
@@ -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
```
81 changes: 71 additions & 10 deletions packages/endpoint/src/schemas/Collection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)}`;
Comment thread
ntucker marked this conversation as resolved.
// 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<any, any>,
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<C extends CollectionSchema<any, any>>(
collection: C,
addMerge: (existing: any, incoming: any) => any,
Expand Down
232 changes: 231 additions & 1 deletion packages/endpoint/src/schemas/__tests__/Collection.test.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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<typeof console.warn>;
afterEach(() => {
warnSpy.mockRestore();
Expand Down Expand Up @@ -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<unknown> = {
...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: {
Expand Down Expand Up @@ -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 = {
Expand Down
1 change: 1 addition & 0 deletions website/blog/2026-10-03-v0.19-batch-set.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down
Loading