fix(graphcache): Fix reference equality of lists not always being preserved

.changeset/spicy-comics-shave.md

@@ -0,0 +1,5 @@
+---
+'@urql/exchange-graphcache': patch
+---
+
+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.

exchanges/graphcache/src/operations/query.test.ts

@@ -363,10 +363,14 @@ describe('Query', () => {
     expect(previousData).toHaveProperty('todos.0.textB', 'old');
   });
 
-  it('should keep references stable', () => {
+  it('should keep references stable (1)', () => {
     const QUERY = gql`
       query todos {
         __typename
+        todos {
+          __typename
+          test
+        }
         todos {
           __typename
           id
@@ -383,14 +387,17 @@ describe('Query', () => {
         {
           __typename: 'Todo',
           id: '0',
+          test: '0',
         },
         {
           __typename: 'Todo',
           id: '1',
+          test: '1',
         },
         {
           __typename: 'Todo',
           id: '2',
+          test: '2',
         },
       ],
       __typename: 'query_root',
@@ -405,15 +412,18 @@ describe('Query', () => {
         todos: [
           {
             __typename: 'Todo',
-            id: 'prev-0',
+            id: '0',
+            test: 'prev-0',
           },
           {
             __typename: 'Todo',
             id: '1',
+            test: '1',
           },
           {
             __typename: 'Todo',
             id: '2',
+            test: '2',
           },
         ],
         __typename: 'query_root',
@@ -426,5 +436,87 @@ describe('Query', () => {
     expect(prevData.todos[0]).toBe(data.todos[0]);
     expect(prevData.todos[1]).toBe(data.todos[1]);
     expect(prevData.todos[2]).toBe(data.todos[2]);
+    expect(prevData.todos).toBe(data.todos);
+    expect(prevData).toBe(data);
+  });
+
+  it('should keep references stable (negative test)', () => {
+    const QUERY = gql`
+      query todos {
+        __typename
+        todos {
+          __typename
+          id
+        }
+        todos {
+          __typename
+          test
+        }
+      }
+    `;
+
+    const store = new Store({
+      schema: alteredRoot,
+    });
+
+    const expected = {
+      todos: [
+        {
+          __typename: 'Todo',
+          id: '0',
+          test: '0',
+        },
+        {
+          __typename: 'Todo',
+          id: '1',
+          test: '1',
+        },
+        {
+          __typename: 'Todo',
+          id: '2',
+          test: '2',
+        },
+      ],
+      __typename: 'query_root',
+    };
+
+    write(store, { query: QUERY }, expected);
+
+    const prevData = query(
+      store,
+      { query: QUERY },
+      {
+        todos: [
+          {
+            __typename: 'Todo',
+            id: '0',
+            test: 'prev-0',
+          },
+          {
+            __typename: 'Todo',
+            id: '1',
+            test: '1',
+          },
+          {
+            __typename: 'Todo',
+            id: '2',
+            test: '2',
+          },
+        ],
+        __typename: 'query_root',
+      }
+    ).data as any;
+
+    expected.todos[0].test = 'x';
+    write(store, { query: QUERY }, expected);
+
+    const data = query(store, { query: QUERY }, prevData).data as any;
+    expect(data).toEqual(expected);
+
+    expect(prevData.todos[1]).toBe(data.todos[1]);
+    expect(prevData.todos[2]).toBe(data.todos[2]);
+    expect(prevData.todos[0]).not.toBe(data.todos[0]);
+    expect(prevData.todos).not.toBe(data.todos);
+    expect(prevData).not.toBe(data);
   });
 });

exchanges/graphcache/src/operations/query.ts

@@ -523,9 +523,9 @@ const resolveResolverResult = (
     const _isListNullable = store.schema
       ? isListNullable(store.schema, typename, fieldName)
       : false;
-    const data = new Array(result.length);
+    const data = InMemoryData.makeData(prevData, true);
     let hasChanged =
-      !isOwnedData ||
+      InMemoryData.currentForeignData ||
       !Array.isArray(prevData) ||
       result.length !== prevData.length;
     for (let i = 0, l = result.length; i < l; i++) {
@@ -561,7 +561,7 @@ const resolveResolverResult = (
   } else if (!isOwnedData && prevData === null) {
     return null;
   } else if (isDataOrKey(result)) {
-    const data = (prevData || InMemoryData.makeData()) as Data;
+    const data = (prevData || InMemoryData.makeData(prevData)) as Data;
     return typeof result === 'string'
       ? readSelection(ctx, result, select, data)
       : readSelection(ctx, key, select, data, result);
@@ -592,11 +592,11 @@ const resolveLink = (
     const _isListNullable = store.schema
       ? isListNullable(store.schema, typename, fieldName)
       : false;
-    const newLink = new Array(link.length);
+    const newLink = InMemoryData.makeData(prevData, true);
     let hasChanged =
-      !isOwnedData ||
+      InMemoryData.currentForeignData ||
       !Array.isArray(prevData) ||
-      newLink.length !== prevData.length;
+      link.length !== prevData.length;
     for (let i = 0, l = link.length; i < l; i++) {
       // Add the current index to the walked path before reading the field's value
       ctx.__internal.path.push(i);
@@ -632,7 +632,7 @@ const resolveLink = (
     ctx,
     link,
     select,
-    (prevData || InMemoryData.makeData()) as Data
+    (prevData || InMemoryData.makeData(prevData)) as Data
   );
 };
 

exchanges/graphcache/src/store/data.ts

@@ -8,6 +8,7 @@ import {
   SerializedEntries,
   Dependencies,
   OperationType,
+  DataField,
   Data,
 } from '../types';
 
@@ -59,29 +60,37 @@ export interface InMemoryData {
   storage: StorageAdapter | null;
 }
 
-let currentOwnership: null | WeakSet<Data> = null;
-let currentDataMapping: null | WeakMap<Data, Data> = null;
+let currentOwnership: null | WeakSet<any> = null;
+let currentDataMapping: null | WeakMap<any, any> = null;
 let currentData: null | InMemoryData = null;
 let currentOptimisticKey: null | number = null;
 export let currentOperation: null | OperationType = null;
 export let currentDependencies: null | Dependencies = null;
 export let currentForeignData = false;
 export let currentOptimistic = false;
 
+export function makeData(data: DataField | void, isArray?: false): Data;
+export function makeData(data: DataField | void, isArray: true): DataField[];
+
 /** Creates a new data object unless it's been created in this data run */
-export const makeData = (data?: Data): Data => {
-  let newData: Data;
+export function makeData(data?: DataField | void, isArray?: boolean) {
+  let newData: Data | Data[] | undefined;
   if (data) {
     if (currentOwnership!.has(data)) return data;
-    newData = currentDataMapping!.get(data) || ({} as Data);
+    newData = currentDataMapping!.get(data) as any;
+  }
+
+  if (newData == null) {
+    newData = (isArray ? [] : {}) as any;
+  }
+
+  if (data) {
     currentDataMapping!.set(data, newData);
-  } else {
-    newData = {} as Data;
   }
 
   currentOwnership!.add(newData);
   return newData;
-};
+}
 
 export const ownsData = (data?: Data): boolean =>
   !!data && currentOwnership!.has(data);

packages/site/vercel.json

@@ -0,0 +1,5 @@
+{
+  "github": {
+    "silent": true
+  }
+}