diff --git a/.changeset/spicy-comics-shave.md b/.changeset/spicy-comics-shave.md new file mode 100644 index 0000000000..9b9b3c73ab --- /dev/null +++ b/.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. diff --git a/exchanges/graphcache/src/operations/query.test.ts b/exchanges/graphcache/src/operations/query.test.ts index e90d9639e9..6c99ddf447 100644 --- a/exchanges/graphcache/src/operations/query.test.ts +++ b/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); }); }); diff --git a/exchanges/graphcache/src/operations/query.ts b/exchanges/graphcache/src/operations/query.ts index 83f3cf6900..cf4d2b5f51 100644 --- a/exchanges/graphcache/src/operations/query.ts +++ b/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 ); }; diff --git a/exchanges/graphcache/src/store/data.ts b/exchanges/graphcache/src/store/data.ts index 6d93630322..7346eb7457 100644 --- a/exchanges/graphcache/src/store/data.ts +++ b/exchanges/graphcache/src/store/data.ts @@ -8,6 +8,7 @@ import { SerializedEntries, Dependencies, OperationType, + DataField, Data, } from '../types'; @@ -59,8 +60,8 @@ export interface InMemoryData { storage: StorageAdapter | null; } -let currentOwnership: null | WeakSet = null; -let currentDataMapping: null | WeakMap = null; +let currentOwnership: null | WeakSet = null; +let currentDataMapping: null | WeakMap = null; let currentData: null | InMemoryData = null; let currentOptimisticKey: null | number = null; export let currentOperation: null | OperationType = null; @@ -68,20 +69,28 @@ 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); diff --git a/packages/site/vercel.json b/packages/site/vercel.json new file mode 100644 index 0000000000..7ae9a3de54 --- /dev/null +++ b/packages/site/vercel.json @@ -0,0 +1,5 @@ +{ + "github": { + "silent": true + } +}