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.

(core) - Fix formatDocument mutating original document (#880)

* Fix formatDocument mutating original document

- Fix the formatDocument mutating parts of the original document
- Fix a formatted document from creating a different hash in createRequest

The latter works because `formatDocument` can be assumed to be called
by caches, like the cacheExchange, so they shouldn't ever alter the
resulting hash.

* Add changeset

* Update offlineExchange.test.ts

authored by

Phil Plückthun and committed by
GitHub
c6fb74ed 481ff4ca

+58 -19
+5
.changeset/six-hornets-smile.md
··· 1 + --- 2 + '@urql/core': patch 3 + --- 4 + 5 + Fix `formatDocument` mutating parts of the `DocumentNode` which may be shared by other documents and queries. Also ensure that a formatted document will always generate the same key in `createRequest` as the original document.
+4 -2
exchanges/graphcache/src/offlineExchange.test.ts
··· 1 - import gql from 'graphql-tag'; 2 1 import { 3 2 createClient, 4 3 ExchangeIO, 5 4 Operation, 6 5 OperationResult, 6 + formatDocument, 7 7 } from '@urql/core'; 8 + 9 + import gql from 'graphql-tag'; 8 10 import { pipe, map, makeSubject, tap, publish } from 'wonka'; 9 11 import { offlineExchange } from './offlineExchange'; 10 12 ··· 263 265 expect(dispatchOperationSpy).toHaveBeenCalledTimes(1); 264 266 expect((dispatchOperationSpy.mock.calls[0][0] as any).key).toEqual(1); 265 267 expect((dispatchOperationSpy.mock.calls[0][0] as any).query).toEqual( 266 - mutationOp.query 268 + formatDocument(mutationOp.query) 267 269 ); 268 270 }); 269 271 });
+3 -4
packages/core/src/utils/request.ts
··· 10 10 const hashQuery = (q: string): number => hash(q.replace(/[\s,]+/g, ' ').trim()); 11 11 12 12 const docs: Documents = Object.create(null); 13 - const keyProp = '__key'; 14 13 15 14 export const createRequest = ( 16 15 q: string | DocumentNode, ··· 21 20 if (typeof q === 'string') { 22 21 key = hashQuery(q); 23 22 query = docs[key] !== undefined ? docs[key] : parse(q); 24 - } else if ((q as any)[keyProp] !== undefined) { 25 - key = (q as any)[keyProp]; 23 + } else if ((q as any).__key !== undefined) { 24 + key = (q as any).__key; 26 25 query = q; 27 26 } else { 28 27 key = hashQuery(print(q)); ··· 30 29 } 31 30 32 31 docs[key] = query; 33 - (query as any)[keyProp] = key; 32 + (query as any).__key = key; 34 33 35 34 return { 36 35 key: vars ? phash(key, stringifyVariables(vars)) >>> 0 : key,
+25 -1
packages/core/src/utils/typenames.test.ts
··· 1 1 import { parse, print } from 'graphql'; 2 2 import { collectTypesFromResponse, formatDocument } from './typenames'; 3 + import { createRequest } from './request'; 3 4 4 5 const formatTypeNames = (query: string) => { 5 6 const typedNode = formatDocument(parse(query)); ··· 8 9 9 10 describe('formatTypeNames', () => { 10 11 it('creates a new instance when adding typenames', () => { 11 - const doc = parse(`{ todos { id } }`) as any; 12 + const doc = parse(`{ id todos { id } }`) as any; 12 13 const newDoc = formatDocument(doc) as any; 13 14 expect(doc).not.toBe(newDoc); 14 15 expect(doc.definitions).not.toBe(newDoc.definitions); ··· 23 24 expect(doc.definitions[0].selectionSet.selections[0]).toBe( 24 25 newDoc.definitions[0].selectionSet.selections[0] 25 26 ); 27 + // Not equal again: 28 + expect(doc.definitions[0].selectionSet.selections[1]).not.toBe( 29 + newDoc.definitions[0].selectionSet.selections[1] 30 + ); 31 + expect(doc.definitions[0].selectionSet.selections[1].selectionSet).not.toBe( 32 + newDoc.definitions[0].selectionSet.selections[1].selectionSet 33 + ); 34 + // Equal again: 35 + expect( 36 + doc.definitions[0].selectionSet.selections[1].selectionSet.selections[0] 37 + ).toBe( 38 + newDoc.definitions[0].selectionSet.selections[1].selectionSet 39 + .selections[0] 40 + ); 41 + }); 42 + 43 + it('preserves the hashed key of the resulting query', () => { 44 + const doc = parse(`{ id todos { id } }`) as any; 45 + const expectedKey = createRequest(doc).key; 46 + const formattedDoc = formatDocument(doc); 47 + expect(formattedDoc).not.toBe(doc); 48 + const actualKey = createRequest(formattedDoc).key; 49 + expect(expectedKey).toBe(actualKey); 26 50 }); 27 51 28 52 it('adds typenames to a query string', () => {
+21 -12
packages/core/src/utils/typenames.ts
··· 40 40 node => node.kind === Kind.FIELD && node.name.value === '__typename' 41 41 ) 42 42 ) { 43 - // NOTE: It's fine to mutate here as long as we return the node, 44 - // which will instruct visit() to clone the AST upwards 45 - (node.selectionSet.selections as SelectionNode[]).push({ 46 - kind: Kind.FIELD, 47 - name: { 48 - kind: Kind.NAME, 49 - value: '__typename', 43 + return { 44 + ...node, 45 + selectionSet: { 46 + ...node.selectionSet, 47 + selections: [ 48 + ...(node.selectionSet.selections as SelectionNode[]), 49 + { 50 + kind: Kind.FIELD, 51 + name: { 52 + kind: Kind.NAME, 53 + value: '__typename', 54 + }, 55 + }, 56 + ], 50 57 }, 51 - }); 52 - 53 - return node; 58 + }; 54 59 } 55 60 }; 56 61 57 - export const formatDocument = (node: DocumentNode) => { 58 - return visit(node, { 62 + export const formatDocument = (node: DocumentNode): DocumentNode => { 63 + const result = visit(node, { 59 64 Field: formatNode, 60 65 InlineFragment: formatNode, 61 66 }); 67 + 68 + // Ensure that the hash of the resulting document won't suddenly change 69 + result.__key = (node as any).__key; 70 + return result; 62 71 };