Skip to content

Commit e2fff91

Browse files
ntuckerntucker
committed
fix: Collection.remove (#3564)
1 parent 9a6e994 commit e2fff91

File tree

4 files changed

+286
-200
lines changed

4 files changed

+286
-200
lines changed

‎.changeset/large-walls-accept.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
'@data-client/endpoint': patch
3+
'@data-client/graphql': patch
4+
'@data-client/rest': patch
5+
---
6+
7+
fix: Collection.remove with Unions

‎packages/endpoint/src/schemas/Collection.ts‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,29 @@ const unshiftMerge = (existing: any, incoming: any) => {
1919
const valuesMerge = (existing: any, incoming: any) => {
2020
return { ...existing, ...incoming };
2121
};
22-
const removeMerge = (existing: Array<any>, incoming: any) => {
23-
return existing.filter((item: any) => !incoming.includes(item));
22+
const isShallowEqual = (
23+
a: { id: string; schema: string } | string,
24+
b: { id: string; schema: string } | string,
25+
): boolean => {
26+
// TODO: make this extensible in the child's schema
27+
// where they are both objects, they are equal if they have the same id and type
28+
if (typeof a === 'object' && typeof b === 'object') {
29+
return a.id === b.id && a.schema === b.schema;
30+
}
31+
// where they are both strings, they are equal if they are the same
32+
return a === b;
33+
};
34+
35+
const removeMerge = (
36+
existing: Array<{ id: string; schema: string } | string>,
37+
incoming: Array<{ id: string; schema: string } | string>,
38+
) => {
39+
return existing.filter(
40+
(item: { id: string; schema: string } | string) =>
41+
!incoming.some((incomingItem: { id: string; schema: string } | string) =>
42+
isShallowEqual(item, incomingItem),
43+
),
44+
);
2445
};
2546

2647
const createArray = (value: any) => [...value];
@@ -143,7 +164,7 @@ export default class CollectionSchema<
143164
toJSON() {
144165
return {
145166
key: this.key,
146-
schema: this.schema.schema.toJSON(),
167+
schema: this.schema.schema,
147168
};
148169
}
149170

‎packages/endpoint/src/schemas/__tests__/Collection.test.ts‎

Lines changed: 255 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// eslint-env jest
2-
import { initialState } from '@data-client/core';
2+
import { initialState, State } from '@data-client/core';
33
import { normalize, denormalize, MemoCache } from '@data-client/normalizr';
44
import { ArticleResource, IDEntity } from '__tests__/new';
55
import { Record } from 'immutable';
@@ -227,6 +227,250 @@ describe(`${schema.Collection.name} normalization`, () => {
227227
expect(result).toMatchSnapshot();
228228
expect(entities).toMatchSnapshot();
229229
});
230+
231+
test('remove works with Union members', () => {
232+
const collectionPK = collectionUnion.pk({}, {}, '', [
233+
{ completed: false },
234+
]);
235+
const init: State<unknown> = {
236+
...initialState,
237+
entities: {
238+
[collectionUnion.key]: {
239+
[collectionPK]: [
240+
{ id: '1', schema: 'users' },
241+
{ id: '2', schema: 'groups' },
242+
{ id: '3', schema: 'groups' },
243+
],
244+
},
245+
User: {
246+
'1': {
247+
id: '1',
248+
type: 'users',
249+
},
250+
},
251+
Group: {
252+
'2': {
253+
id: '2',
254+
type: 'groups',
255+
},
256+
'3': {
257+
id: '3',
258+
type: 'groups',
259+
},
260+
},
261+
},
262+
entitiesMeta: {
263+
[collectionUnion.key]: {
264+
[collectionPK]: {
265+
date: 1557831718135,
266+
expiresAt: Infinity,
267+
fetchedAt: 0,
268+
},
269+
},
270+
User: {
271+
'1': {
272+
date: 1557831718135,
273+
expiresAt: Infinity,
274+
fetchedAt: 0,
275+
},
276+
},
277+
Group: {
278+
'2': {
279+
date: 1557831718135,
280+
expiresAt: Infinity,
281+
fetchedAt: 0,
282+
},
283+
'3': {
284+
date: 1557831718135,
285+
expiresAt: Infinity,
286+
fetchedAt: 0,
287+
},
288+
},
289+
},
290+
indexes: {},
291+
};
292+
const state = normalize(
293+
collectionUnion.remove,
294+
{ id: '1', type: 'users' },
295+
[{ completed: false }],
296+
init,
297+
);
298+
expect(state.entities[collectionUnion.key]?.[collectionPK]).toEqual([
299+
{ id: '2', schema: 'groups' },
300+
{ id: '3', schema: 'groups' },
301+
]);
302+
});
303+
304+
test('remove works updates the correct collections', () => {
305+
const init = {
306+
entities: {
307+
[collectionUnion.key]: {
308+
// Collection for userId: '1', completed: true
309+
'{"userId":"1","completed":"true"}': [
310+
{ id: '1', schema: 'users' },
311+
{ id: '2', schema: 'groups' },
312+
],
313+
// Collection for userId: '1', completed: false
314+
'{"userId":"1","completed":"false"}': [
315+
{ id: '3', schema: 'users' },
316+
{ id: '4', schema: 'groups' },
317+
],
318+
// Collection for userId: '1' (no completed filter)
319+
'{"userId":"1"}': [
320+
{ id: '1', schema: 'users' },
321+
{ id: '3', schema: 'users' },
322+
],
323+
// Collection for userId: '2'
324+
'{"userId":"2"}': [{ id: '5', schema: 'users' }],
325+
// Collection for userId: '1', completed: true, priority: high (additional arg)
326+
'{"userId":"1","completed":"true","priority":"high"}': [
327+
{ id: '1', schema: 'users' },
328+
{ id: '2', schema: 'groups' },
329+
],
330+
// Collection for userId: '1', completed: true, priority: low (additional arg)
331+
'{"userId":"1","completed":"true","priority":"low"}': [
332+
{ id: '3', schema: 'users' },
333+
{ id: '4', schema: 'groups' },
334+
],
335+
},
336+
User: {
337+
'1': {
338+
id: '1',
339+
type: 'users',
340+
userId: '1',
341+
completed: true,
342+
priority: 'high',
343+
},
344+
'3': {
345+
id: '3',
346+
type: 'users',
347+
userId: '1',
348+
completed: false,
349+
priority: 'low',
350+
},
351+
'5': {
352+
id: '5',
353+
type: 'users',
354+
userId: '2',
355+
completed: false,
356+
},
357+
},
358+
Group: {
359+
'2': {
360+
id: '2',
361+
type: 'groups',
362+
userId: '1',
363+
completed: true,
364+
priority: 'high',
365+
},
366+
'4': {
367+
id: '4',
368+
type: 'groups',
369+
userId: '1',
370+
completed: false,
371+
priority: 'low',
372+
},
373+
},
374+
},
375+
entitiesMeta: {
376+
[collectionUnion.key]: {
377+
'{"userId":"1","completed":"true"}': {
378+
date: 1557831718135,
379+
expiresAt: Infinity,
380+
fetchedAt: 0,
381+
},
382+
'{"userId":"1","completed":"false"}': {
383+
date: 1557831718135,
384+
expiresAt: Infinity,
385+
fetchedAt: 0,
386+
},
387+
'{"userId":"1"}': {
388+
date: 1557831718135,
389+
expiresAt: Infinity,
390+
fetchedAt: 0,
391+
},
392+
'{"userId":"2"}': {
393+
date: 1557831718135,
394+
expiresAt: Infinity,
395+
fetchedAt: 0,
396+
},
397+
'{"userId":"1","completed":"true","priority":"high"}': {
398+
date: 1557831718135,
399+
expiresAt: Infinity,
400+
fetchedAt: 0,
401+
},
402+
'{"userId":"1","completed":"true","priority":"low"}': {
403+
date: 1557831718135,
404+
expiresAt: Infinity,
405+
fetchedAt: 0,
406+
},
407+
},
408+
User: {
409+
'1': { date: 1557831718135, expiresAt: Infinity, fetchedAt: 0 },
410+
'3': { date: 1557831718135, expiresAt: Infinity, fetchedAt: 0 },
411+
'5': { date: 1557831718135, expiresAt: Infinity, fetchedAt: 0 },
412+
},
413+
Group: {
414+
'2': { date: 1557831718135, expiresAt: Infinity, fetchedAt: 0 },
415+
'4': { date: 1557831718135, expiresAt: Infinity, fetchedAt: 0 },
416+
},
417+
},
418+
indexes: {},
419+
};
420+
421+
// Remove a union member with userId: '1' and completed: true
422+
// This should remove from collections that match userId: '1' and optionally have completed: true
423+
const state = normalize(
424+
collectionUnion.remove,
425+
{ id: '1', type: 'users' },
426+
[{ userId: '1', completed: true }],
427+
init,
428+
);
429+
430+
// Should remove from collections that match the args:
431+
// - '{"userId":"1","completed":"true"}' (exact match)
432+
// - '{"userId":"1"}' (partial match - has userId: '1', missing completed)
433+
// Should NOT remove from:
434+
// - '{"userId":"1","completed":"false"}' (conflicting completed value)
435+
// - '{"userId":"2"}' (different userId)
436+
// - '{"userId":"1","completed":"true","priority":"high"}' (has additional priority arg)
437+
// - '{"userId":"1","completed":"true","priority":"low"}' (has additional priority arg)
438+
expect(
439+
state.entities[collectionUnion.key][
440+
'{"userId":"1","completed":"true"}'
441+
],
442+
).toEqual([{ id: '2', schema: 'groups' }]);
443+
expect(
444+
state.entities[collectionUnion.key][
445+
'{"userId":"1","completed":"false"}'
446+
],
447+
).toEqual([
448+
{ id: '3', schema: 'users' },
449+
{ id: '4', schema: 'groups' },
450+
]);
451+
expect(state.entities[collectionUnion.key]['{"userId":"1"}']).toEqual([
452+
{ id: '3', schema: 'users' },
453+
]);
454+
expect(state.entities[collectionUnion.key]['{"userId":"2"}']).toEqual([
455+
{ id: '5', schema: 'users' },
456+
]);
457+
expect(
458+
state.entities[collectionUnion.key][
459+
'{"userId":"1","completed":"true","priority":"high"}'
460+
],
461+
).toEqual([
462+
{ id: '1', schema: 'users' },
463+
{ id: '2', schema: 'groups' },
464+
]); // Should remain unchanged (has additional priority arg)
465+
expect(
466+
state.entities[collectionUnion.key][
467+
'{"userId":"1","completed":"true","priority":"low"}'
468+
],
469+
).toEqual([
470+
{ id: '3', schema: 'users' },
471+
{ id: '4', schema: 'groups' },
472+
]); // Should remain unchanged (has additional priority arg)
473+
});
230474
});
231475

232476
test('normalizes push onto the end', () => {
@@ -343,7 +587,9 @@ describe(`${schema.Collection.name} normalization`, () => {
343587
[{ userId: '1' }],
344588
init,
345589
);
346-
expect(state).toMatchSnapshot();
590+
expect(state.entities[User.schema.todos.key]['{"userId":"1"}']).toEqual([
591+
'6',
592+
]);
347593
});
348594

349595
test('normalizes remove from collection with multiple items', () => {
@@ -415,7 +661,10 @@ describe(`${schema.Collection.name} normalization`, () => {
415661
[{ userId: '1' }],
416662
init,
417663
);
418-
expect(state).toMatchSnapshot();
664+
expect(state.entities[User.schema.todos.key]['{"userId":"1"}']).toEqual([
665+
'5',
666+
'7',
667+
]);
419668
});
420669

421670
test('normalizes remove non-existent item from collection', () => {
@@ -469,7 +718,9 @@ describe(`${schema.Collection.name} normalization`, () => {
469718
[{ userId: '1' }],
470719
init,
471720
);
472-
expect(state).toMatchSnapshot();
721+
expect(state.entities[User.schema.todos.key]['{"userId":"1"}']).toEqual([
722+
'5',
723+
]);
473724
});
474725

475726
describe('push should add only to collections matching filterArgumentKeys', () => {

0 commit comments

Comments
 (0)