Mirror: The highly customizable and versatile GraphQL client with which you add on features like normalized caching as you grow.
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

(graphcache) - Fix API results from being reused incorrectly for queries (#1196)

* Add test case demonstrating original data affecting results

* Remove unexpected __typename fields from test fixtures

* Assert in test that result data shouldn't be the same reference

* Prevent query original result data from being reused

* Add changeset

* Simplify some checks from `undefined` to truthiness

authored by

Phil Pluckthun and committed by
GitHub
11686945 eee82891

+106 -18
+5
.changeset/wise-rats-whisper.md
··· 1 + --- 2 + '@urql/exchange-graphcache': patch 3 + --- 4 + 5 + Fix reusing original query data from APIs accidentally, which can lead to subtle mismatches in results when the API's incoming `query` results are being updated by the `cacheExchange`, to apply resolvers. Specifically this may lead to relations from being set back to `null` when the resolver returns a different list of links than the result, since some `null` relations may unintentionally exist but aren't related. If you're using `relayPagination` then this fix is critical.
-3
exchanges/graphcache/src/cacheExchange.test.ts
··· 1062 1062 expect(fakeResolver).toHaveBeenCalledTimes(1); 1063 1063 expect(result).toHaveBeenCalledTimes(1); 1064 1064 expect(result.mock.calls[0][0].data).toEqual({ 1065 - __typename: 'Query', 1066 1065 author: { 1067 - __typename: 'Author', 1068 1066 id: '123', 1069 1067 name: 'newName', 1070 1068 }, ··· 1430 1428 expect(response).toHaveBeenCalledTimes(1); 1431 1429 expect(reexec).toHaveBeenCalledTimes(0); 1432 1430 expect(result.mock.calls[0][0].data).toEqual({ 1433 - __typename: 'Query', 1434 1431 todos: [ 1435 1432 { 1436 1433 __typename: 'Todo',
+4 -1
exchanges/graphcache/src/offlineExchange.test.ts
··· 168 168 169 169 next(queryOp); 170 170 expect(result).toBeCalledTimes(1); 171 - expect(result.mock.calls[0][0].data).toEqual(queryOneData); 171 + expect(result.mock.calls[0][0].data).toEqual({ 172 + ...queryOneData, 173 + __typename: undefined, 174 + }); 172 175 173 176 next(mutationOp); 174 177 expect(result).toBeCalledTimes(1);
+80
exchanges/graphcache/src/operations/query.test.ts
··· 3 3 import { gql } from '@urql/core'; 4 4 import { minifyIntrospectionQuery } from '@urql/introspection'; 5 5 6 + import { Data } from '../types'; 6 7 import { Store } from '../store'; 7 8 import { write } from './write'; 8 9 import { query } from './query'; ··· 128 129 query(store, { query: INVALID_TODO_QUERY }); 129 130 expect(console.warn).toHaveBeenCalledTimes(1); 130 131 expect((console.warn as any).mock.calls[0][0]).toMatch(/writer/); 132 + }); 133 + 134 + it('should not overwrite different arrays from results and queries', () => { 135 + const TODO_QUERY = gql` 136 + query Todos { 137 + todos { 138 + __typename 139 + node { 140 + __typename 141 + id 142 + text 143 + author { 144 + __typename 145 + id 146 + } 147 + } 148 + } 149 + } 150 + `; 151 + 152 + store = new Store({ 153 + resolvers: { 154 + Query: { 155 + todos: (_parent, _args, cache) => cache.resolve('Query', 'todos'), 156 + }, 157 + }, 158 + }); 159 + 160 + const expected = ({ 161 + todos: [ 162 + { 163 + __typename: 'TodoEdge', 164 + node: { 165 + __typename: 'Todo', 166 + id: '0', 167 + text: 'Teach', 168 + author: { 169 + __typename: 'Author', 170 + id: 'writy-mcwriteface', 171 + }, 172 + }, 173 + }, 174 + { 175 + __typename: 'TodoEdge', 176 + node: { 177 + __typename: 'Todo', 178 + id: '1', 179 + text: 'Learn', 180 + author: null, 181 + }, 182 + }, 183 + ], 184 + } as any) as Data; 185 + 186 + write(store, { query: TODO_QUERY }, expected); 187 + 188 + const result = query( 189 + store, 190 + { query: TODO_QUERY }, 191 + { 192 + __typename: 'Query', 193 + todos: [ 194 + // NOTE: This is a partial list of later results 195 + { 196 + __typename: 'TodoEdge', 197 + node: { 198 + id: '1', 199 + text: 'Learn', 200 + __typename: 'Todo', 201 + author: null, 202 + }, 203 + }, 204 + ], 205 + } 206 + ); 207 + 208 + expect(result.data).toEqual(expected); 209 + expect(result.data).not.toBe(expected); 210 + expect(result.data!.todos![0]).not.toBe(expected!.todos![0]); 131 211 }); 132 212 133 213 // Issue#64
+15 -12
exchanges/graphcache/src/operations/query.ts
··· 90 90 pushDebugNode(rootKey, operation); 91 91 } 92 92 93 - let data: Data | undefined = input || ({} as Data); 94 - data = 93 + // NOTE: This may reuse "previous result data" as indicated by the 94 + // `originalData` argument in readRoot(). This behaviour isn't used 95 + // for readSelection() however, which always produces results from 96 + // scratch 97 + const data = 95 98 rootKey !== ctx.store.rootFields['query'] 96 - ? readRoot(ctx, rootKey, rootSelect, data) 97 - : readSelection(ctx, rootKey, rootSelect, data); 99 + ? readRoot(ctx, rootKey, rootSelect, input || ({} as Data)) 100 + : readSelection(ctx, rootKey, rootSelect, {} as Data); 98 101 99 102 if (process.env.NODE_ENV !== 'production') { 100 103 popDebugNode(); ··· 102 105 103 106 return { 104 107 dependencies: getCurrentDependencies(), 105 - partial: data === undefined ? false : ctx.partial, 106 - data: data === undefined ? null : data, 108 + partial: ctx.partial || !data, 109 + data: data || null, 107 110 }; 108 111 }; 109 112 ··· 122 125 data.__typename = originalData.__typename; 123 126 124 127 let node: FieldNode | void; 125 - while ((node = iterate()) !== undefined) { 128 + while ((node = iterate())) { 126 129 const fieldAlias = getFieldAlias(node); 127 130 const fieldValue = originalData[fieldAlias]; 128 - if (node.selectionSet !== undefined && fieldValue !== null) { 131 + if (node.selectionSet && fieldValue !== null) { 129 132 const fieldData = ensureData(fieldValue); 130 133 data[fieldAlias] = readRootField(ctx, getSelectionSet(node), fieldData); 131 134 } else { ··· 171 174 const fragments = getFragments(query); 172 175 const names = Object.keys(fragments); 173 176 const fragment = fragments[names[0]] as FragmentDefinitionNode; 174 - if (fragment === undefined) { 177 + if (!fragment) { 175 178 warn( 176 179 'readFragment(...) was called with an empty fragment.\n' + 177 180 'You have to call it with at least one fragment in your GraphQL document.', ··· 323 326 ctx 324 327 ); 325 328 326 - if (node.selectionSet !== undefined) { 329 + if (node.selectionSet) { 327 330 // When it has a selection set we are resolving an entity with a 328 331 // subselection. This can either be a list or an object. 329 332 dataFieldValue = resolveResolverResult( ··· 332 335 fieldName, 333 336 key, 334 337 getSelectionSet(node), 335 - (data[fieldAlias] || {}) as Data, 338 + data[fieldAlias] as Data, 336 339 dataFieldValue 337 340 ); 338 341 } ··· 346 349 // current field 347 350 return undefined; 348 351 } 349 - } else if (node.selectionSet === undefined) { 352 + } else if (!node.selectionSet) { 350 353 // The field is a scalar but isn't on the result, so it's retrieved from the cache 351 354 dataFieldValue = fieldValue; 352 355 } else if (resultValue !== undefined) {
+2 -2
exchanges/graphcache/src/operations/write.ts
··· 144 144 const fragments = getFragments(query); 145 145 const names = Object.keys(fragments); 146 146 const fragment = fragments[names[0]] as FragmentDefinitionNode; 147 - if (fragment === undefined) { 147 + if (!fragment) { 148 148 return warn( 149 149 'writeFragment(...) was called with an empty fragment.\n' + 150 150 'You have to call it with at least one fragment in your GraphQL document.', ··· 336 336 337 337 if ( 338 338 parentFieldKey && 339 - ctx.store.keys[data.__typename] === undefined && 339 + !ctx.store.keys[data.__typename] && 340 340 entityKey === null && 341 341 typeof typename === 'string' && 342 342 !KEYLESS_TYPE_RE.test(typename)