Mirror: The highly customizable and versatile GraphQL client with which you add on features like normalized caching as you grow.
fix(graphcache): Fix reference equality of lists not always being preserved (#3228)
authored by
Phil Pluckthun
and committed by
GitHub
81ce770b
10c3a978
+128
-17
+5
.changeset/spicy-comics-shave.md
+5
.changeset/spicy-comics-shave.md
···
1
+
---
2
+
'@urql/exchange-graphcache': patch
3
+
---
4
+
5
+
Fix reference equality not being preserved. This is a fix on top of [#3165](https://github.com/urql-graphql/urql/pull/3165), and was previously not addressed to avoid having to test for corner cases that are hard to cover. If you experience issues with this fix, please let us know.
+94
-2
exchanges/graphcache/src/operations/query.test.ts
+94
-2
exchanges/graphcache/src/operations/query.test.ts
···
363
363
expect(previousData).toHaveProperty('todos.0.textB', 'old');
364
364
});
365
365
366
-
it('should keep references stable', () => {
366
+
it('should keep references stable (1)', () => {
367
367
const QUERY = gql`
368
368
query todos {
369
369
__typename
370
+
todos {
371
+
__typename
372
+
test
373
+
}
370
374
todos {
371
375
__typename
372
376
id
···
383
387
{
384
388
__typename: 'Todo',
385
389
id: '0',
390
+
test: '0',
386
391
},
387
392
{
388
393
__typename: 'Todo',
389
394
id: '1',
395
+
test: '1',
390
396
},
391
397
{
392
398
__typename: 'Todo',
393
399
id: '2',
400
+
test: '2',
394
401
},
395
402
],
396
403
__typename: 'query_root',
···
405
412
todos: [
406
413
{
407
414
__typename: 'Todo',
408
-
id: 'prev-0',
415
+
id: '0',
416
+
test: 'prev-0',
409
417
},
410
418
{
411
419
__typename: 'Todo',
412
420
id: '1',
421
+
test: '1',
413
422
},
414
423
{
415
424
__typename: 'Todo',
416
425
id: '2',
426
+
test: '2',
417
427
},
418
428
],
419
429
__typename: 'query_root',
···
426
436
expect(prevData.todos[0]).toBe(data.todos[0]);
427
437
expect(prevData.todos[1]).toBe(data.todos[1]);
428
438
expect(prevData.todos[2]).toBe(data.todos[2]);
439
+
expect(prevData.todos).toBe(data.todos);
440
+
expect(prevData).toBe(data);
441
+
});
442
+
443
+
it('should keep references stable (negative test)', () => {
444
+
const QUERY = gql`
445
+
query todos {
446
+
__typename
447
+
todos {
448
+
__typename
449
+
id
450
+
}
451
+
todos {
452
+
__typename
453
+
test
454
+
}
455
+
}
456
+
`;
457
+
458
+
const store = new Store({
459
+
schema: alteredRoot,
460
+
});
461
+
462
+
const expected = {
463
+
todos: [
464
+
{
465
+
__typename: 'Todo',
466
+
id: '0',
467
+
test: '0',
468
+
},
469
+
{
470
+
__typename: 'Todo',
471
+
id: '1',
472
+
test: '1',
473
+
},
474
+
{
475
+
__typename: 'Todo',
476
+
id: '2',
477
+
test: '2',
478
+
},
479
+
],
480
+
__typename: 'query_root',
481
+
};
482
+
483
+
write(store, { query: QUERY }, expected);
484
+
485
+
const prevData = query(
486
+
store,
487
+
{ query: QUERY },
488
+
{
489
+
todos: [
490
+
{
491
+
__typename: 'Todo',
492
+
id: '0',
493
+
test: 'prev-0',
494
+
},
495
+
{
496
+
__typename: 'Todo',
497
+
id: '1',
498
+
test: '1',
499
+
},
500
+
{
501
+
__typename: 'Todo',
502
+
id: '2',
503
+
test: '2',
504
+
},
505
+
],
506
+
__typename: 'query_root',
507
+
}
508
+
).data as any;
509
+
510
+
expected.todos[0].test = 'x';
511
+
write(store, { query: QUERY }, expected);
512
+
513
+
const data = query(store, { query: QUERY }, prevData).data as any;
514
+
expect(data).toEqual(expected);
515
+
516
+
expect(prevData.todos[1]).toBe(data.todos[1]);
517
+
expect(prevData.todos[2]).toBe(data.todos[2]);
518
+
expect(prevData.todos[0]).not.toBe(data.todos[0]);
519
+
expect(prevData.todos).not.toBe(data.todos);
520
+
expect(prevData).not.toBe(data);
429
521
});
430
522
});
+7
-7
exchanges/graphcache/src/operations/query.ts
+7
-7
exchanges/graphcache/src/operations/query.ts
···
523
523
const _isListNullable = store.schema
524
524
? isListNullable(store.schema, typename, fieldName)
525
525
: false;
526
-
const data = new Array(result.length);
526
+
const data = InMemoryData.makeData(prevData, true);
527
527
let hasChanged =
528
-
!isOwnedData ||
528
+
InMemoryData.currentForeignData ||
529
529
!Array.isArray(prevData) ||
530
530
result.length !== prevData.length;
531
531
for (let i = 0, l = result.length; i < l; i++) {
···
561
561
} else if (!isOwnedData && prevData === null) {
562
562
return null;
563
563
} else if (isDataOrKey(result)) {
564
-
const data = (prevData || InMemoryData.makeData()) as Data;
564
+
const data = (prevData || InMemoryData.makeData(prevData)) as Data;
565
565
return typeof result === 'string'
566
566
? readSelection(ctx, result, select, data)
567
567
: readSelection(ctx, key, select, data, result);
···
592
592
const _isListNullable = store.schema
593
593
? isListNullable(store.schema, typename, fieldName)
594
594
: false;
595
-
const newLink = new Array(link.length);
595
+
const newLink = InMemoryData.makeData(prevData, true);
596
596
let hasChanged =
597
-
!isOwnedData ||
597
+
InMemoryData.currentForeignData ||
598
598
!Array.isArray(prevData) ||
599
-
newLink.length !== prevData.length;
599
+
link.length !== prevData.length;
600
600
for (let i = 0, l = link.length; i < l; i++) {
601
601
// Add the current index to the walked path before reading the field's value
602
602
ctx.__internal.path.push(i);
···
632
632
ctx,
633
633
link,
634
634
select,
635
-
(prevData || InMemoryData.makeData()) as Data
635
+
(prevData || InMemoryData.makeData(prevData)) as Data
636
636
);
637
637
};
638
638
+17
-8
exchanges/graphcache/src/store/data.ts
+17
-8
exchanges/graphcache/src/store/data.ts
···
8
8
SerializedEntries,
9
9
Dependencies,
10
10
OperationType,
11
+
DataField,
11
12
Data,
12
13
} from '../types';
13
14
···
59
60
storage: StorageAdapter | null;
60
61
}
61
62
62
-
let currentOwnership: null | WeakSet<Data> = null;
63
-
let currentDataMapping: null | WeakMap<Data, Data> = null;
63
+
let currentOwnership: null | WeakSet<any> = null;
64
+
let currentDataMapping: null | WeakMap<any, any> = null;
64
65
let currentData: null | InMemoryData = null;
65
66
let currentOptimisticKey: null | number = null;
66
67
export let currentOperation: null | OperationType = null;
···
68
69
export let currentForeignData = false;
69
70
export let currentOptimistic = false;
70
71
72
+
export function makeData(data: DataField | void, isArray?: false): Data;
73
+
export function makeData(data: DataField | void, isArray: true): DataField[];
74
+
71
75
/** Creates a new data object unless it's been created in this data run */
72
-
export const makeData = (data?: Data): Data => {
73
-
let newData: Data;
76
+
export function makeData(data?: DataField | void, isArray?: boolean) {
77
+
let newData: Data | Data[] | undefined;
74
78
if (data) {
75
79
if (currentOwnership!.has(data)) return data;
76
-
newData = currentDataMapping!.get(data) || ({} as Data);
80
+
newData = currentDataMapping!.get(data) as any;
81
+
}
82
+
83
+
if (newData == null) {
84
+
newData = (isArray ? [] : {}) as any;
85
+
}
86
+
87
+
if (data) {
77
88
currentDataMapping!.set(data, newData);
78
-
} else {
79
-
newData = {} as Data;
80
89
}
81
90
82
91
currentOwnership!.add(newData);
83
92
return newData;
84
-
};
93
+
}
85
94
86
95
export const ownsData = (data?: Data): boolean =>
87
96
!!data && currentOwnership!.has(data);