From d7edb88547471f415a4c94027450c44b0e1f4e57 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 14 Jun 2021 14:07:50 -0400 Subject: [PATCH 1/4] Log non-fatal error when fields are missing from written results. Fixes #8331 and #6915, and should help with the underlying cause of https://github.com/apollographql/apollo-client/issues/7436#issuecomment-860631219 --- src/cache/inmemory/writeToStore.ts | 31 +++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/src/cache/inmemory/writeToStore.ts b/src/cache/inmemory/writeToStore.ts index 83d94f4ce7e..47f32692e0d 100644 --- a/src/cache/inmemory/writeToStore.ts +++ b/src/cache/inmemory/writeToStore.ts @@ -18,8 +18,8 @@ import { Reference, isReference, shouldInclude, - hasDirectives, cloneDeep, + addTypenameToDocument, } from '../../utilities'; import { NormalizedCache, ReadMergeModifyContext, MergeTree } from './types'; @@ -44,6 +44,7 @@ export interface WriteContext extends ReadMergeModifyContext { mergeTree: MergeTree; selections: Set; }>; + clientOnly: boolean; }; interface ProcessSelectionSetOptions { @@ -86,6 +87,7 @@ export class StoreWriter { fragmentMap: createFragmentMap(getFragmentDefinitions(query)), overwrite: !!overwrite, incomingById: new Map, + clientOnly: false, }; const ref = this.processSelectionSet({ @@ -231,7 +233,13 @@ export class StoreWriter { const resultFieldKey = resultKeyNameFromField(selection); const value = result[resultFieldKey]; - if (typeof value !== 'undefined') { + const wasClientOnly = context.clientOnly; + context.clientOnly = wasClientOnly || !!( + selection.directives && + selection.directives.some(d => d.name.value === "client") + ); + + if (value !== void 0) { const storeFieldName = policies.getStoreFieldName({ typename, fieldName: selection.name.value, @@ -303,17 +311,18 @@ export class StoreWriter { }); } else if ( - policies.usingPossibleTypes && - !hasDirectives(["defer", "client"], selection) + !context.clientOnly && + !addTypenameToDocument.added(selection) ) { - throw new InvariantError( - `Missing field '${resultFieldKey}' in ${JSON.stringify( - result, - null, - 2, - ).substring(0, 100)}`, - ); + invariant.error(`Missing field '${ + resultKeyNameFromField(selection) + }' while writing result ${ + JSON.stringify(result, null, 2) + }`.substring(0, 1000)); } + + context.clientOnly = wasClientOnly; + } else { // This is not a field, so it must be a fragment, either inline or named const fragment = getFragmentFromSelection( From d11ea159d66d73e99b1f51f15792372ca1a1b1cb Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 15 Jun 2021 11:48:23 -0400 Subject: [PATCH 2/4] New internal testing utility: withErrorSpy(it, "should...", ...) --- src/__tests__/__snapshots__/exports.ts.snap | 1 + src/utilities/testing/index.ts | 1 + src/utilities/testing/itAsync.ts | 17 +++++++++-------- src/utilities/testing/withErrorSpy.ts | 21 +++++++++++++++++++++ 4 files changed, 32 insertions(+), 8 deletions(-) create mode 100644 src/utilities/testing/withErrorSpy.ts diff --git a/src/__tests__/__snapshots__/exports.ts.snap b/src/__tests__/__snapshots__/exports.ts.snap index 82a7e0a1010..bf8dd514224 100644 --- a/src/__tests__/__snapshots__/exports.ts.snap +++ b/src/__tests__/__snapshots__/exports.ts.snap @@ -314,6 +314,7 @@ Array [ "mockSingleLink", "stripSymbols", "subscribeAndCount", + "withErrorSpy", ] `; diff --git a/src/utilities/testing/index.ts b/src/utilities/testing/index.ts index 45c2f3198f1..668557783e9 100644 --- a/src/utilities/testing/index.ts +++ b/src/utilities/testing/index.ts @@ -13,3 +13,4 @@ export { createMockClient } from './mocking/mockClient'; export { stripSymbols } from './stripSymbols'; export { default as subscribeAndCount } from './subscribeAndCount'; export { itAsync } from './itAsync'; +export { withErrorSpy } from './withErrorSpy'; diff --git a/src/utilities/testing/itAsync.ts b/src/utilities/testing/itAsync.ts index 17c8406321e..9d54b947671 100644 --- a/src/utilities/testing/itAsync.ts +++ b/src/utilities/testing/itAsync.ts @@ -14,12 +14,13 @@ function wrap(key?: "only" | "skip" | "todo") { } const wrappedIt = wrap(); -export function itAsync(...args: Parameters) { - return wrappedIt.apply(this, args); -} -export namespace itAsync { - export const only = wrap("only"); - export const skip = wrap("skip"); - export const todo = wrap("todo"); -} +export const itAsync = Object.assign(function ( + ...args: Parameters +) { + return wrappedIt.apply(this, args); +}, { + only: wrap("only"), + skip: wrap("skip"), + todo: wrap("todo"), +}); diff --git a/src/utilities/testing/withErrorSpy.ts b/src/utilities/testing/withErrorSpy.ts new file mode 100644 index 00000000000..2c39e8c1f91 --- /dev/null +++ b/src/utilities/testing/withErrorSpy.ts @@ -0,0 +1,21 @@ +export function withErrorSpy< + TArgs extends any[], + TResult, +>( + it: (...args: TArgs) => TResult, + ...args: TArgs +) { + const fn = args[1]; + args[1] = function () { + const args = arguments; + const errorSpy = jest.spyOn(console, 'error'); + errorSpy.mockImplementation(() => {}); + return new Promise(resolve => { + resolve(fn?.apply(this, args)); + }).finally(() => { + expect(errorSpy).toMatchSnapshot(); + errorSpy.mockReset(); + }); + }; + return it(...args); +} From 4b4a9b80e816e6d6ed5640b7c5437da7e24e5734 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 14 Jun 2021 17:05:19 -0400 Subject: [PATCH 3/4] Fix tests and capture/check invariant.error messages. --- src/__tests__/ApolloClient.ts | 68 ++++---- .../__snapshots__/ApolloClient.ts.snap | 39 +++++ src/__tests__/__snapshots__/client.ts.snap | 21 +++ .../__snapshots__/mutationResults.ts.snap | 20 +++ src/__tests__/client.ts | 35 ++-- .../local-state/__snapshots__/export.ts.snap | 141 ++++++++++++++++ .../local-state/__snapshots__/general.ts.snap | 38 +++++ src/__tests__/local-state/export.ts | 8 +- src/__tests__/local-state/general.ts | 6 +- src/__tests__/mutationResults.ts | 19 +-- .../__tests__/__snapshots__/policies.ts.snap | 156 ++++++++++++++++++ .../__snapshots__/readFromStore.ts.snap | 19 +++ .../__tests__/__snapshots__/roundtrip.ts.snap | 41 +++++ .../__snapshots__/writeToStore.ts.snap | 78 +++++++++ src/cache/inmemory/__tests__/cache.ts | 2 +- src/cache/inmemory/__tests__/entityStore.ts | 4 +- src/cache/inmemory/__tests__/policies.ts | 12 +- src/cache/inmemory/__tests__/readFromStore.ts | 3 +- src/cache/inmemory/__tests__/roundtrip.ts | 9 +- src/cache/inmemory/__tests__/writeToStore.ts | 43 +++-- src/core/__tests__/ObservableQuery.ts | 5 +- src/core/__tests__/QueryManager/index.ts | 14 +- src/core/__tests__/fetchPolicies.ts | 2 +- .../__tests__/client/Query.test.tsx | 4 +- .../client/__snapshots__/Query.test.tsx.snap | 16 ++ .../__snapshots__/useQuery.test.tsx.snap | 17 ++ .../useSubscription.test.tsx.snap | 47 ++++++ src/react/hooks/__tests__/useQuery.test.tsx | 12 +- .../hooks/__tests__/useSubscription.test.tsx | 10 +- 29 files changed, 768 insertions(+), 121 deletions(-) create mode 100644 src/__tests__/local-state/__snapshots__/export.ts.snap create mode 100644 src/cache/inmemory/__tests__/__snapshots__/roundtrip.ts.snap create mode 100644 src/react/hooks/__tests__/__snapshots__/useQuery.test.tsx.snap create mode 100644 src/react/hooks/__tests__/__snapshots__/useSubscription.test.tsx.snap diff --git a/src/__tests__/ApolloClient.ts b/src/__tests__/ApolloClient.ts index 5f195adcbec..5d59a8a7344 100644 --- a/src/__tests__/ApolloClient.ts +++ b/src/__tests__/ApolloClient.ts @@ -12,7 +12,7 @@ import { Observable } from '../utilities'; import { ApolloLink } from '../link/core'; import { HttpLink } from '../link/http'; import { InMemoryCache } from '../cache'; -import { stripSymbols } from '../testing'; +import { stripSymbols, withErrorSpy } from '../testing'; import { TypedDocumentNode } from '@graphql-typed-document-node/core'; describe('ApolloClient', () => { @@ -834,7 +834,7 @@ describe('ApolloClient', () => { }); }); - it('should warn when the data provided does not match the query shape', () => { + withErrorSpy(it, 'should warn when the data provided does not match the query shape', () => { const client = new ApolloClient({ link: ApolloLink.empty(), cache: new InMemoryCache({ @@ -843,28 +843,26 @@ describe('ApolloClient', () => { }), }); - expect(() => { - client.writeQuery({ - data: { - todos: [ - { - id: '1', - name: 'Todo 1', - __typename: 'Todo', - }, - ], - }, - query: gql` - query { - todos { - id - name - description - } + client.writeQuery({ + data: { + todos: [ + { + id: '1', + name: 'Todo 1', + __typename: 'Todo', + }, + ], + }, + query: gql` + query { + todos { + id + name + description } - `, - }); - }).toThrowError(/Missing field 'description' /); + } + `, + }); }); }); @@ -1119,7 +1117,7 @@ describe('ApolloClient', () => { }); }); - it('should warn when the data provided does not match the fragment shape', () => { + withErrorSpy(it, 'should warn when the data provided does not match the fragment shape', () => { const client = new ApolloClient({ link: ApolloLink.empty(), cache: new InMemoryCache({ @@ -1128,18 +1126,16 @@ describe('ApolloClient', () => { }), }); - expect(() => { - client.writeFragment({ - data: { __typename: 'Bar', i: 10 }, - id: 'bar', - fragment: gql` - fragment fragmentBar on Bar { - i - e - } - `, - }); - }).toThrowError(/Missing field 'e' /); + client.writeFragment({ + data: { __typename: 'Bar', i: 10 }, + id: 'bar', + fragment: gql` + fragment fragmentBar on Bar { + i + e + } + `, + }); }); describe('change will call observable next', () => { diff --git a/src/__tests__/__snapshots__/ApolloClient.ts.snap b/src/__tests__/__snapshots__/ApolloClient.ts.snap index d49b8df8c55..63f2d60b480 100644 --- a/src/__tests__/__snapshots__/ApolloClient.ts.snap +++ b/src/__tests__/__snapshots__/ApolloClient.ts.snap @@ -209,6 +209,25 @@ Object { } `; +exports[`ApolloClient writeFragment should warn when the data provided does not match the fragment shape 1`] = ` +[MockFunction] { + "calls": Array [ + Array [ + "Missing field 'e' while writing result { + \\"__typename\\": \\"Bar\\", + \\"i\\": 10 +}", + ], + ], + "results": Array [ + Object { + "type": "return", + "value": undefined, + }, + ], +} +`; + exports[`ApolloClient writeFragment will write some deeply nested data into the store at any id 1`] = ` Object { "__META": Object { @@ -359,6 +378,26 @@ Object { } `; +exports[`ApolloClient writeQuery should warn when the data provided does not match the query shape 1`] = ` +[MockFunction] { + "calls": Array [ + Array [ + "Missing field 'description' while writing result { + \\"id\\": \\"1\\", + \\"name\\": \\"Todo 1\\", + \\"__typename\\": \\"Todo\\" +}", + ], + ], + "results": Array [ + Object { + "type": "return", + "value": undefined, + }, + ], +} +`; + exports[`ApolloClient writeQuery will write some deeply nested data to the store 1`] = ` Object { "ROOT_QUERY": Object { diff --git a/src/__tests__/__snapshots__/client.ts.snap b/src/__tests__/__snapshots__/client.ts.snap index 4abceb872b6..4e2bfb9219d 100644 --- a/src/__tests__/__snapshots__/client.ts.snap +++ b/src/__tests__/__snapshots__/client.ts.snap @@ -41,3 +41,24 @@ Object { }, } `; + +exports[`client should warn if server returns wrong data 1`] = ` +[MockFunction] { + "calls": Array [ + Array [ + "Missing field 'description' while writing result { + \\"id\\": \\"1\\", + \\"name\\": \\"Todo 1\\", + \\"price\\": 100, + \\"__typename\\": \\"Todo\\" +}", + ], + ], + "results": Array [ + Object { + "type": "return", + "value": undefined, + }, + ], +} +`; diff --git a/src/__tests__/__snapshots__/mutationResults.ts.snap b/src/__tests__/__snapshots__/mutationResults.ts.snap index 211d8a2fa53..887a5147f14 100644 --- a/src/__tests__/__snapshots__/mutationResults.ts.snap +++ b/src/__tests__/__snapshots__/mutationResults.ts.snap @@ -1,5 +1,25 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`mutation results should warn when the result fields don't match the query fields 1`] = ` +[MockFunction] { + "calls": Array [ + Array [ + "Missing field 'description' while writing result { + \\"id\\": \\"2\\", + \\"name\\": \\"Todo 2\\", + \\"__typename\\": \\"createTodo\\" +}", + ], + ], + "results": Array [ + Object { + "type": "return", + "value": undefined, + }, + ], +} +`; + exports[`mutation results should write results to cache according to errorPolicy 1`] = `Object {}`; exports[`mutation results should write results to cache according to errorPolicy 2`] = ` diff --git a/src/__tests__/client.ts b/src/__tests__/client.ts index 9156269c2f3..c810a705fad 100644 --- a/src/__tests__/client.ts +++ b/src/__tests__/client.ts @@ -20,6 +20,7 @@ import { stripSymbols, subscribeAndCount, mockSingleLink, + withErrorSpy, } from '../testing'; describe('client', () => { @@ -2081,6 +2082,7 @@ describe('client', () => { resolve(); }); }); + itAsync('should allow errors to be returned from a mutation', (resolve, reject) => { const mutation = gql` mutation { @@ -2102,7 +2104,12 @@ describe('client', () => { const client = new ApolloClient({ link: mockSingleLink({ request: { query: mutation }, - result: { data, errors }, + result: { + errors, + data: { + newPerson: data, + }, + }, }).setOnError(reject), cache: new InMemoryCache({ addTypename: false }), }); @@ -2112,7 +2119,9 @@ describe('client', () => { expect(result.errors).toBeDefined(); expect(result.errors!.length).toBe(1); expect(result.errors![0].message).toBe(errors[0].message); - expect(result.data).toEqual(data); + expect(result.data).toEqual({ + newPerson: data, + }); resolve(); }) .catch((error: ApolloError) => { @@ -2132,9 +2141,11 @@ describe('client', () => { } `; const data = { - person: { - firstName: 'John', - lastName: 'Smith', + newPerson: { + person: { + firstName: 'John', + lastName: 'Smith', + }, }, }; const errors = [new Error('Some kind of GraphQL error.')]; @@ -2631,7 +2642,7 @@ describe('client', () => { }).then(resolve, reject); }); - itAsync('should warn if server returns wrong data', (resolve, reject) => { + withErrorSpy(itAsync, 'should warn if server returns wrong data', (resolve, reject) => { const query = gql` query { todos { @@ -2654,6 +2665,7 @@ describe('client', () => { ], }, }; + const link = mockSingleLink({ request: { query }, result, @@ -2666,14 +2678,9 @@ describe('client', () => { }), }); - return client.query({ query }).then( - result => { - fail("should have errored"); - }, - error => { - expect(error.message).toMatch(/Missing field 'description' /); - }, - ).then(resolve, reject); + return client.query({ query }).then(({ data }) => { + expect(data).toEqual(result.data); + }).then(resolve, reject); }); itAsync('runs a query with the connection directive and writes it to the store key defined in the directive', (resolve, reject) => { diff --git a/src/__tests__/local-state/__snapshots__/export.ts.snap b/src/__tests__/local-state/__snapshots__/export.ts.snap new file mode 100644 index 00000000000..4af22816166 --- /dev/null +++ b/src/__tests__/local-state/__snapshots__/export.ts.snap @@ -0,0 +1,141 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`@client @export tests should NOT refetch if an @export variable has not changed, the current fetch policy is not cache-only, and the query includes fields that need to be resolved remotely 1`] = ` +[MockFunction] { + "calls": Array [ + Array [ + "Missing field 'postCount' while writing result { + \\"currentAuthorId\\": 100 +}", + ], + ], + "results": Array [ + Object { + "type": "return", + "value": undefined, + }, + ], +} +`; + +exports[`@client @export tests should allow @client @export variables to be used with remote queries 1`] = ` +[MockFunction] { + "calls": Array [ + Array [ + "Missing field 'postCount' while writing result { + \\"currentAuthor\\": { + \\"name\\": \\"John Smith\\", + \\"authorId\\": 100, + \\"__typename\\": \\"Author\\" + } +}", + ], + ], + "results": Array [ + Object { + "type": "return", + "value": undefined, + }, + ], +} +`; + +exports[`@client @export tests should refetch if an @export variable changes, the current fetch policy is not cache-only, and the query includes fields that need to be resolved remotely 1`] = ` +[MockFunction] { + "calls": Array [ + Array [ + "Missing field 'postCount' while writing result { + \\"appContainer\\": { + \\"systemDetails\\": { + \\"currentAuthor\\": { + \\"name\\": \\"John Smith\\", + \\"authorId\\": 100, + \\"__typename\\": \\"Author\\" + }, + \\"__typename\\": \\"SystemDetails\\" + }, + \\"__typename\\": \\"AppContainer\\" + } +}", + ], + Array [ + "Missing field 'title' while writing result { + \\"loggedInReviewerId\\": 100, + \\"__typename\\": \\"Post\\", + \\"id\\": 10 +}", + ], + Array [ + "Missing field 'reviewerDetails' while writing result { + \\"postRequiringReview\\": { + \\"loggedInReviewerId\\": 100, + \\"__typename\\": \\"Post\\", + \\"id\\": 10 + } +}", + ], + Array [ + "Missing field 'id' while writing result { + \\"__typename\\": \\"Post\\" +}", + ], + Array [ + "Missing field 'title' while writing result { + \\"__typename\\": \\"Post\\" +}", + ], + Array [ + "Missing field 'reviewerDetails' while writing result { + \\"postRequiringReview\\": { + \\"__typename\\": \\"Post\\" + } +}", + ], + Array [ + "Missing field 'post' while writing result { + \\"primaryReviewerId\\": 100, + \\"secondaryReviewerId\\": 200 +}", + ], + Array [ + "Missing field 'postCount' while writing result { + \\"currentAuthorId\\": 100 +}", + ], + ], + "results": Array [ + Object { + "type": "return", + "value": undefined, + }, + Object { + "type": "return", + "value": undefined, + }, + Object { + "type": "return", + "value": undefined, + }, + Object { + "type": "return", + "value": undefined, + }, + Object { + "type": "return", + "value": undefined, + }, + Object { + "type": "return", + "value": undefined, + }, + Object { + "type": "return", + "value": undefined, + }, + Object { + "type": "return", + "value": undefined, + }, + ], +} +`; diff --git a/src/__tests__/local-state/__snapshots__/general.ts.snap b/src/__tests__/local-state/__snapshots__/general.ts.snap index 9747bcf8cd7..331aef47fe9 100644 --- a/src/__tests__/local-state/__snapshots__/general.ts.snap +++ b/src/__tests__/local-state/__snapshots__/general.ts.snap @@ -3,3 +3,41 @@ exports[`Combining client and server state/operations should correctly propagate an error from a client resolver 1`] = `"Illegal Query Operation Occurred"`; exports[`Combining client and server state/operations should correctly propagate an error from a client resolver 2`] = `"Illegal Mutation Operation Occurred"`; + +exports[`Combining client and server state/operations should handle a simple query with both server and client fields 1`] = ` +[MockFunction] { + "calls": Array [ + Array [ + "Missing field 'lastCount' while writing result { + \\"count\\": 0 +}", + ], + ], + "results": Array [ + Object { + "type": "return", + "value": undefined, + }, + ], +} +`; + +exports[`Combining client and server state/operations should support nested querying of both server and client fields 1`] = ` +[MockFunction] { + "calls": Array [ + Array [ + "Missing field 'lastName' while writing result { + \\"__typename\\": \\"User\\", + \\"id\\": 123, + \\"firstName\\": \\"John\\" +}", + ], + ], + "results": Array [ + Object { + "type": "return", + "value": undefined, + }, + ], +} +`; diff --git a/src/__tests__/local-state/export.ts b/src/__tests__/local-state/export.ts index a9e14a3a3c0..75d77a9e203 100644 --- a/src/__tests__/local-state/export.ts +++ b/src/__tests__/local-state/export.ts @@ -2,7 +2,7 @@ import gql from 'graphql-tag'; import { print } from 'graphql'; import { Observable } from '../../utilities'; -import { itAsync } from '../../testing'; +import { itAsync, withErrorSpy } from '../../testing'; import { ApolloLink } from '../../link/core'; import { ApolloClient } from '../../core'; import { InMemoryCache } from '../../cache'; @@ -179,7 +179,7 @@ describe('@client @export tests', () => { }, ); - it('should allow @client @export variables to be used with remote queries', done => { + withErrorSpy(it, 'should allow @client @export variables to be used with remote queries', done => { const query = gql` query currentAuthorPostCount($authorId: Int!) { currentAuthor @client { @@ -714,7 +714,7 @@ describe('@client @export tests', () => { }, ); - it( + withErrorSpy(it, 'should refetch if an @export variable changes, the current fetch ' + 'policy is not cache-only, and the query includes fields that need to ' + 'be resolved remotely', @@ -779,7 +779,7 @@ describe('@client @export tests', () => { } ); - it( + withErrorSpy(it, 'should NOT refetch if an @export variable has not changed, the ' + 'current fetch policy is not cache-only, and the query includes fields ' + 'that need to be resolved remotely', diff --git a/src/__tests__/local-state/general.ts b/src/__tests__/local-state/general.ts index 4c442f6b6ed..47f595440ec 100644 --- a/src/__tests__/local-state/general.ts +++ b/src/__tests__/local-state/general.ts @@ -6,7 +6,7 @@ import { ApolloLink } from '../../link/core'; import { Operation } from '../../link/core'; import { ApolloClient } from '../../core'; import { ApolloCache, InMemoryCache } from '../../cache'; -import { itAsync } from '../../testing'; +import { itAsync, withErrorSpy } from '../../testing'; describe('General functionality', () => { it('should not impact normal non-@client use', () => { @@ -885,7 +885,7 @@ describe('Combining client and server state/operations', () => { resolve(); }); - itAsync('should handle a simple query with both server and client fields', (resolve, reject) => { + withErrorSpy(itAsync, 'should handle a simple query with both server and client fields', (resolve, reject) => { const query = gql` query GetCount { count @client @@ -920,7 +920,7 @@ describe('Combining client and server state/operations', () => { }); }); - itAsync('should support nested querying of both server and client fields', (resolve, reject) => { + withErrorSpy(itAsync, 'should support nested querying of both server and client fields', (resolve, reject) => { const query = gql` query GetUser { user { diff --git a/src/__tests__/mutationResults.ts b/src/__tests__/mutationResults.ts index bc8ca258580..9a78a3255b8 100644 --- a/src/__tests__/mutationResults.ts +++ b/src/__tests__/mutationResults.ts @@ -6,7 +6,7 @@ import { ApolloClient } from '../core'; import { InMemoryCache } from '../cache'; import { ApolloLink } from '../link/core'; import { Observable, ObservableSubscription as Subscription } from '../utilities'; -import { itAsync, subscribeAndCount, mockSingleLink } from '../testing'; +import { itAsync, subscribeAndCount, mockSingleLink, withErrorSpy } from '../testing'; describe('mutation results', () => { const query = gql` @@ -400,7 +400,7 @@ describe('mutation results', () => { resolve(); }); - itAsync("should warn when the result fields don't match the query fields", (resolve, reject) => { + withErrorSpy(itAsync, "should warn when the result fields don't match the query fields", (resolve, reject) => { let handle: any; let subscriptionHandle: Subscription; @@ -482,16 +482,11 @@ describe('mutation results', () => { return newResults; }, }, - })).then( - () => { - subscriptionHandle.unsubscribe(); - fail("should have errored"); - }, - error => { - subscriptionHandle.unsubscribe(); - expect(error.message).toMatch(/Missing field 'description' /); - }, - ).then(resolve, reject); + })).finally( + () => subscriptionHandle.unsubscribe(), + ).then(result => { + expect(result).toEqual(mutationTodoResult); + }).then(resolve, reject); }); describe('InMemoryCache type/field policies', () => { diff --git a/src/cache/inmemory/__tests__/__snapshots__/policies.ts.snap b/src/cache/inmemory/__tests__/__snapshots__/policies.ts.snap index 296372a0cd1..d414466e566 100644 --- a/src/cache/inmemory/__tests__/__snapshots__/policies.ts.snap +++ b/src/cache/inmemory/__tests__/__snapshots__/policies.ts.snap @@ -50,6 +50,51 @@ Object { } `; +exports[`type policies complains about missing key fields 1`] = ` +[MockFunction] { + "calls": Array [ + Array [ + "Missing field 'title' while writing result { + \\"year\\": 2011, + \\"theInformationBookData\\": { + \\"__typename\\": \\"Book\\", + \\"isbn\\": \\"1400096235\\", + \\"title\\": \\"The Information\\", + \\"subtitle\\": \\"A History, a Theory, a Flood\\", + \\"author\\": { + \\"name\\": \\"James Gleick\\" + } + } +}", + ], + ], + "results": Array [ + Object { + "type": "return", + "value": undefined, + }, + ], +} +`; + +exports[`type policies field policies assumes keyArgs:false when read and merge function present 1`] = ` +[MockFunction] { + "calls": Array [ + Array [ + "Missing field 'a' while writing result { + \\"__typename\\": \\"TypeA\\" +}", + ], + ], + "results": Array [ + Object { + "type": "return", + "value": undefined, + }, + ], +} +`; + exports[`type policies field policies can handle Relay-style pagination 1`] = ` Object { "Artist:{\\"href\\":\\"/artist/jean-michel-basquiat\\"}": Object { @@ -749,6 +794,7 @@ Object { "cursor": "YXJyYXljb25uZWN0aW9uOjEx", "node": Object { "__typename": "SearchableItem", + "description": "", "displayLabel": "James Turrell: Light knows when we’re looking", }, }, @@ -1211,6 +1257,45 @@ Object { } `; +exports[`type policies field policies read and merge can cooperate through options.storage 1`] = ` +[MockFunction] { + "calls": Array [ + Array [ + "Missing field 'result' while writing result { + \\"__typename\\": \\"Job\\", + \\"name\\": \\"Job #1\\" +}", + ], + Array [ + "Missing field 'result' while writing result { + \\"__typename\\": \\"Job\\", + \\"name\\": \\"Job #2\\" +}", + ], + Array [ + "Missing field 'result' while writing result { + \\"__typename\\": \\"Job\\", + \\"name\\": \\"Job #3\\" +}", + ], + ], + "results": Array [ + Object { + "type": "return", + "value": undefined, + }, + Object { + "type": "return", + "value": undefined, + }, + Object { + "type": "return", + "value": undefined, + }, + ], +} +`; + exports[`type policies field policies read, merge, and modify functions can access options.storage 1`] = ` Object { "ROOT_QUERY": Object { @@ -1233,6 +1318,77 @@ Object { } `; +exports[`type policies field policies readField helper function calls custom read functions 1`] = ` +[MockFunction] { + "calls": Array [ + Array [ + "Missing field 'blockers' while writing result { + \\"__typename\\": \\"Task\\", + \\"id\\": 4, + \\"description\\": \\"grandchild task\\" +}", + ], + ], + "results": Array [ + Object { + "type": "return", + "value": undefined, + }, + ], +} +`; + +exports[`type policies field policies runs nested merge functions as well as ancestors 1`] = ` +[MockFunction] { + "calls": Array [ + Array [ + "Missing field 'time' while writing result { + \\"__typename\\": \\"Event\\", + \\"id\\": 123 +}", + ], + Array [ + "Missing field 'time' while writing result { + \\"__typename\\": \\"Event\\", + \\"id\\": 345, + \\"name\\": \\"Rooftop dog party\\", + \\"attendees\\": [ + { + \\"__typename\\": \\"Attendee\\", + \\"id\\": 456, + \\"name\\": \\"Inspector Beckett\\" + }, + { + \\"__typename\\": \\"Attendee\\", + \\"id\\": 234 + } + ] +}", + ], + Array [ + "Missing field 'name' while writing result { + \\"__typename\\": \\"Attendee\\", + \\"id\\": 234 +}", + ], + ], + "results": Array [ + Object { + "type": "return", + "value": undefined, + }, + Object { + "type": "return", + "value": undefined, + }, + Object { + "type": "return", + "value": undefined, + }, + ], +} +`; + exports[`type policies support inheritance 1`] = ` Object { "Cobra:{\\"tagId\\":\\"Egypt30BC\\"}": Object { diff --git a/src/cache/inmemory/__tests__/__snapshots__/readFromStore.ts.snap b/src/cache/inmemory/__tests__/__snapshots__/readFromStore.ts.snap index 46732d054ee..7de2c6014cb 100644 --- a/src/cache/inmemory/__tests__/__snapshots__/readFromStore.ts.snap +++ b/src/cache/inmemory/__tests__/__snapshots__/readFromStore.ts.snap @@ -1,5 +1,24 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`reading from the store propagates eviction signals to parent queries 1`] = ` +[MockFunction] { + "calls": Array [ + Array [ + "Missing field 'children' while writing result { + \\"__typename\\": \\"Deity\\", + \\"name\\": \\"Zeus\\" +}", + ], + ], + "results": Array [ + Object { + "type": "return", + "value": undefined, + }, + ], +} +`; + exports[`reading from the store returns === results for different queries 1`] = ` Object { "ROOT_QUERY": Object { diff --git a/src/cache/inmemory/__tests__/__snapshots__/roundtrip.ts.snap b/src/cache/inmemory/__tests__/__snapshots__/roundtrip.ts.snap new file mode 100644 index 00000000000..9f5f5e848ca --- /dev/null +++ b/src/cache/inmemory/__tests__/__snapshots__/roundtrip.ts.snap @@ -0,0 +1,41 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`roundtrip fragments should throw an error on two of the same inline fragment types 1`] = ` +[MockFunction] { + "calls": Array [ + Array [ + "Missing field 'rank' while writing result { + \\"__typename\\": \\"Jedi\\", + \\"name\\": \\"Luke Skywalker\\", + \\"side\\": \\"bright\\" +}", + ], + ], + "results": Array [ + Object { + "type": "return", + "value": undefined, + }, + ], +} +`; + +exports[`roundtrip fragments should throw on error on two of the same spread fragment types 1`] = ` +[MockFunction] { + "calls": Array [ + Array [ + "Missing field 'rank' while writing result { + \\"__typename\\": \\"Jedi\\", + \\"name\\": \\"Luke Skywalker\\", + \\"side\\": \\"bright\\" +}", + ], + ], + "results": Array [ + Object { + "type": "return", + "value": undefined, + }, + ], +} +`; diff --git a/src/cache/inmemory/__tests__/__snapshots__/writeToStore.ts.snap b/src/cache/inmemory/__tests__/__snapshots__/writeToStore.ts.snap index caa466e21f8..94ea2a95cb2 100644 --- a/src/cache/inmemory/__tests__/__snapshots__/writeToStore.ts.snap +++ b/src/cache/inmemory/__tests__/__snapshots__/writeToStore.ts.snap @@ -69,6 +69,46 @@ Object { } `; +exports[`writing to the store should not keep reference when type of mixed inlined field changes to non-inlined field 1`] = ` +[MockFunction] { + "calls": Array [ + Array [ + "Missing field 'price' while writing result { + \\"id\\": \\"1\\", + \\"name\\": \\"Todo 1\\", + \\"description\\": \\"Description 1\\", + \\"__typename\\": \\"ShoppingCartItem\\" +}", + ], + Array [ + "Missing field 'expensive' while writing result { + \\"id\\": 1 +}", + ], + Array [ + "Missing field 'id' while writing result { + \\"__typename\\": \\"Cat\\", + \\"name\\": \\"cat\\" +}", + ], + ], + "results": Array [ + Object { + "type": "return", + "value": undefined, + }, + Object { + "type": "return", + "value": undefined, + }, + Object { + "type": "return", + "value": undefined, + }, + ], +} +`; + exports[`writing to the store should respect id fields added by fragments 1`] = ` Object { "AType:a-id": Object { @@ -177,3 +217,41 @@ Object { }, } `; + +exports[`writing to the store writeResultToStore shape checking should warn when it receives the wrong data with non-union fragments 1`] = ` +[MockFunction] { + "calls": Array [ + Array [ + "Missing field 'description' while writing result { + \\"id\\": \\"1\\", + \\"name\\": \\"Todo 1\\" +}", + ], + ], + "results": Array [ + Object { + "type": "return", + "value": undefined, + }, + ], +} +`; + +exports[`writing to the store writeResultToStore shape checking should write the result data without validating its shape when a fragment matcher is not provided 1`] = ` +[MockFunction] { + "calls": Array [ + Array [ + "Missing field 'description' while writing result { + \\"id\\": \\"1\\", + \\"name\\": \\"Todo 1\\" +}", + ], + ], + "results": Array [ + Object { + "type": "return", + "value": undefined, + }, + ], +} +`; diff --git a/src/cache/inmemory/__tests__/cache.ts b/src/cache/inmemory/__tests__/cache.ts index 1db6fc073c6..ea199eb9d4c 100644 --- a/src/cache/inmemory/__tests__/cache.ts +++ b/src/cache/inmemory/__tests__/cache.ts @@ -2857,7 +2857,7 @@ describe("ReactiveVar and makeVar", () => { const query = gql` query { - onCall { + onCall @client { name } } diff --git a/src/cache/inmemory/__tests__/entityStore.ts b/src/cache/inmemory/__tests__/entityStore.ts index 396b683aede..2001417ace2 100644 --- a/src/cache/inmemory/__tests__/entityStore.ts +++ b/src/cache/inmemory/__tests__/entityStore.ts @@ -307,6 +307,7 @@ describe('EntityStore', () => { { __typename: 'Book', isbn: '9781451673319', + title: 'Fahrenheit 451', }, ], }, @@ -535,6 +536,7 @@ describe('EntityStore', () => { { __typename: 'Book', isbn: '0735211280', + title: "Spineless", }, ], }, @@ -1559,7 +1561,7 @@ describe('EntityStore', () => { expect(cache.identify(todoRef!)).toBe("Todo:123"); const taskRef = cache.writeFragment({ - fragment: gql`fragment TaskId on Task { id }`, + fragment: gql`fragment TaskId on Task { uuid }`, data: { __typename: "Task", uuid: "eb8cffcc-7a9e-4d8b-a517-7d987bf42138", diff --git a/src/cache/inmemory/__tests__/policies.ts b/src/cache/inmemory/__tests__/policies.ts index f4c98c30c6b..58fc8032af1 100644 --- a/src/cache/inmemory/__tests__/policies.ts +++ b/src/cache/inmemory/__tests__/policies.ts @@ -9,6 +9,7 @@ import { MockLink } from '../../../utilities/testing/mocking/mockLink'; import subscribeAndCount from '../../../utilities/testing/subscribeAndCount'; import { itAsync } from '../../../utilities/testing/itAsync'; import { FieldPolicy, StorageType } from "../policies"; +import { withErrorSpy } from "../../../testing"; function reverse(s: string) { return s.split("").reverse().join(""); @@ -253,7 +254,7 @@ describe("type policies", function () { checkAuthorName(cache); }); - it("complains about missing key fields", function () { + withErrorSpy(it, "complains about missing key fields", function () { const cache = new InMemoryCache({ typePolicies: { Book: { @@ -583,7 +584,7 @@ describe("type policies", function () { expect(result).toEqual(data); }); - it("assumes keyArgs:false when read and merge function present", function () { + withErrorSpy(it, "assumes keyArgs:false when read and merge function present", function () { const cache = new InMemoryCache({ typePolicies: { TypeA: { @@ -1281,7 +1282,7 @@ describe("type policies", function () { expect(cache.extract(true)).toEqual(expectedExtraction); }); - it("read and merge can cooperate through options.storage", function () { + withErrorSpy(it, "read and merge can cooperate through options.storage", function () { const cache = new InMemoryCache({ typePolicies: { Query: { @@ -1922,7 +1923,7 @@ describe("type policies", function () { }); }); - it("readField helper function calls custom read functions", function () { + withErrorSpy(it, "readField helper function calls custom read functions", function () { // Rather than writing ownTime data into the cache, we maintain it // externally in this object: const ownTimes: Record> = { @@ -3099,6 +3100,7 @@ describe("type policies", function () { node: { __typename: "SearchableItem", displayLabel: "James Turrell: Light knows when we’re looking", + description: "", }, }, ]; @@ -3528,7 +3530,7 @@ describe("type policies", function () { }); }); - it("runs nested merge functions as well as ancestors", function () { + withErrorSpy(it, "runs nested merge functions as well as ancestors", function () { let eventMergeCount = 0; let attendeeMergeCount = 0; diff --git a/src/cache/inmemory/__tests__/readFromStore.ts b/src/cache/inmemory/__tests__/readFromStore.ts index 40dedd5528b..5ac15d5f5be 100644 --- a/src/cache/inmemory/__tests__/readFromStore.ts +++ b/src/cache/inmemory/__tests__/readFromStore.ts @@ -21,6 +21,7 @@ import { jest.mock('optimism'); import { wrap } from 'optimism'; +import { withErrorSpy } from '../../../testing'; describe('resultCacheMaxSize', () => { const cache = new InMemoryCache(); @@ -1375,7 +1376,7 @@ describe('reading from the store', () => { }); }); - it("propagates eviction signals to parent queries", () => { + withErrorSpy(it, "propagates eviction signals to parent queries", () => { const cache = new InMemoryCache({ typePolicies: { Deity: { diff --git a/src/cache/inmemory/__tests__/roundtrip.ts b/src/cache/inmemory/__tests__/roundtrip.ts index 8604a09a883..7e1bd70a762 100644 --- a/src/cache/inmemory/__tests__/roundtrip.ts +++ b/src/cache/inmemory/__tests__/roundtrip.ts @@ -10,6 +10,7 @@ import { readQueryFromStore, withError, } from './helpers'; +import { withErrorSpy } from '../../../testing'; function assertDeeplyFrozen(value: any, stack: any[] = []) { if (value !== null && typeof value === 'object' && stack.indexOf(value) < 0) { @@ -315,7 +316,7 @@ describe('roundtrip', () => { // XXX this test is weird because it assumes the server returned an incorrect result // However, the user may have written this result with client.writeQuery. - it('should throw an error on two of the same inline fragment types', () => { + withErrorSpy(it, 'should throw an error on two of the same inline fragment types', () => { expect(() => { storeRoundtrip( gql` @@ -342,7 +343,7 @@ describe('roundtrip', () => { ], }, ); - }).toThrowError(/Missing field 'rank' /); + }).toThrowError(/Can't find field 'rank' /); }); it('should resolve fields it can on interface with non matching inline fragments', () => { @@ -455,7 +456,7 @@ describe('roundtrip', () => { }); }); - it('should throw on error on two of the same spread fragment types', () => { + withErrorSpy(it, 'should throw on error on two of the same spread fragment types', () => { expect(() => { storeRoundtrip( gql` @@ -486,7 +487,7 @@ describe('roundtrip', () => { ], }, ); - }).toThrowError(/Missing field 'rank' /); + }).toThrowError(/Can't find field 'rank' /); }); it('should resolve on @include and @skip with inline fragments', () => { diff --git a/src/cache/inmemory/__tests__/writeToStore.ts b/src/cache/inmemory/__tests__/writeToStore.ts index ee346b96b18..2abed99d149 100644 --- a/src/cache/inmemory/__tests__/writeToStore.ts +++ b/src/cache/inmemory/__tests__/writeToStore.ts @@ -20,6 +20,7 @@ import { itAsync } from '../../../utilities/testing/itAsync'; import { StoreWriter } from '../writeToStore'; import { defaultNormalizedCacheFactory, writeQueryToStore } from './helpers'; import { InMemoryCache } from '../inMemoryCache'; +import { withErrorSpy } from '../../../testing'; const getIdField = ({ id }: { id: string }) => id; @@ -1798,7 +1799,7 @@ describe('writing to the store', () => { } `; - it('should write the result data without validating its shape when a fragment matcher is not provided', () => { + withErrorSpy(it, 'should write the result data without validating its shape when a fragment matcher is not provided', () => { const result = { todos: [ { @@ -1823,7 +1824,7 @@ describe('writing to the store', () => { expect((newStore as any).lookup('1')).toEqual(result.todos[0]); }); - it('should warn when it receives the wrong data with non-union fragments', () => { + withErrorSpy(it, 'should warn when it receives the wrong data with non-union fragments', () => { const result = { todos: [ { @@ -1840,13 +1841,11 @@ describe('writing to the store', () => { }), ); - expect(() => { - writeQueryToStore({ - writer, - query, - result, - }); - }).toThrowError(/Missing field 'description' /); + writeQueryToStore({ + writer, + query, + result, + }); }); it('should warn when it receives the wrong data inside a fragment', () => { @@ -1893,13 +1892,11 @@ describe('writing to the store', () => { }), ); - expect(() => { - writeQueryToStore({ - writer, - query: queryWithInterface, - result, - }); - }).toThrowError(/Missing field 'price' /); + writeQueryToStore({ + writer, + query: queryWithInterface, + result, + }); }); it('should warn if a result is missing __typename when required', () => { @@ -1920,13 +1917,11 @@ describe('writing to the store', () => { }), ); - expect(() => { - writeQueryToStore({ - writer, - query: addTypenameToDocument(query), - result, - }); - }).toThrowError(/Missing field '__typename' /); + writeQueryToStore({ + writer, + query: addTypenameToDocument(query), + result, + }); }); it('should not warn if a field is null', () => { @@ -2190,7 +2185,7 @@ describe('writing to the store', () => { }); }); - it('should not keep reference when type of mixed inlined field changes to non-inlined field', () => { + withErrorSpy(it, 'should not keep reference when type of mixed inlined field changes to non-inlined field', () => { const store = defaultNormalizedCacheFactory(); const query = gql` diff --git a/src/core/__tests__/ObservableQuery.ts b/src/core/__tests__/ObservableQuery.ts index 7caa643e226..cc8ec7d8de8 100644 --- a/src/core/__tests__/ObservableQuery.ts +++ b/src/core/__tests__/ObservableQuery.ts @@ -1843,7 +1843,10 @@ describe('ObservableQuery', () => { value: 'oyez', }; } - client.writeQuery({ query, data }); + client.writeQuery({ + query: queryOptions.query, + data, + }); }, error(err) { expect(err.message).toMatch(/No more mocked responses/); diff --git a/src/core/__tests__/QueryManager/index.ts b/src/core/__tests__/QueryManager/index.ts index 6703a735ba6..8c30b7ced94 100644 --- a/src/core/__tests__/QueryManager/index.ts +++ b/src/core/__tests__/QueryManager/index.ts @@ -5741,7 +5741,11 @@ describe('QueryManager', () => { const queryManager = createQueryManager({ link: mockSingleLink({ request: { query }, - result: { author: { firstName: 'John' } }, + result: { + data: { + author: { firstName: 'John' }, + }, + }, }), }); @@ -5749,6 +5753,7 @@ describe('QueryManager', () => { expect(queryManager['inFlightLinkObservables'].size).toBe(1) }); + it('should allow overriding global queryDeduplication: true to false', () => { const query = gql` query { @@ -5757,10 +5762,15 @@ describe('QueryManager', () => { } } `; + const queryManager = createQueryManager({ link: mockSingleLink({ request: { query }, - result: { author: { firstName: 'John' } }, + result: { + data: { + author: { firstName: 'John' }, + }, + }, }), queryDeduplication: true, }); diff --git a/src/core/__tests__/fetchPolicies.ts b/src/core/__tests__/fetchPolicies.ts index 16cecb01b92..0e951595873 100644 --- a/src/core/__tests__/fetchPolicies.ts +++ b/src/core/__tests__/fetchPolicies.ts @@ -610,7 +610,7 @@ describe('cache-only', () => { })), }); - const query = gql`query { counter }`; + const query = gql`query { count }`; const observable = client.watchQuery({ query, diff --git a/src/react/components/__tests__/client/Query.test.tsx b/src/react/components/__tests__/client/Query.test.tsx index a95d18e5ad9..c6de0e546de 100644 --- a/src/react/components/__tests__/client/Query.test.tsx +++ b/src/react/components/__tests__/client/Query.test.tsx @@ -8,7 +8,7 @@ import { ApolloError } from '../../../../errors'; import { ApolloLink } from '../../../../link/core'; import { InMemoryCache as Cache } from '../../../../cache'; import { ApolloProvider } from '../../../context'; -import { itAsync, stripSymbols, MockedProvider, mockSingleLink } from '../../../../testing'; +import { itAsync, stripSymbols, MockedProvider, mockSingleLink, withErrorSpy } from '../../../../testing'; import { Query } from '../../Query'; const allPeopleQuery: DocumentNode = gql` @@ -1853,7 +1853,7 @@ describe('Query component', () => { console.warn = origConsoleWarn; }); - itAsync( + withErrorSpy(itAsync, 'should attempt a refetch when the query result was marked as being ' + 'partial, the returned data was reset to an empty Object by the ' + 'Apollo Client QueryManager (due to a cache miss), and the ' + diff --git a/src/react/components/__tests__/client/__snapshots__/Query.test.tsx.snap b/src/react/components/__tests__/client/__snapshots__/Query.test.tsx.snap index 8e19a2575ff..e4bafb7dc02 100644 --- a/src/react/components/__tests__/client/__snapshots__/Query.test.tsx.snap +++ b/src/react/components/__tests__/client/__snapshots__/Query.test.tsx.snap @@ -1,5 +1,21 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`Query component Partial refetching should attempt a refetch when the query result was marked as being partial, the returned data was reset to an empty Object by the Apollo Client QueryManager (due to a cache miss), and the \`partialRefetch\` prop is \`true\` 1`] = ` +[MockFunction] { + "calls": Array [ + Array [ + "Missing field 'allPeople' while writing result {}", + ], + ], + "results": Array [ + Object { + "type": "return", + "value": undefined, + }, + ], +} +`; + exports[`Query component calls the children prop: result in render prop 1`] = ` Object { "called": true, diff --git a/src/react/hooks/__tests__/__snapshots__/useQuery.test.tsx.snap b/src/react/hooks/__tests__/__snapshots__/useQuery.test.tsx.snap new file mode 100644 index 00000000000..45381278cbb --- /dev/null +++ b/src/react/hooks/__tests__/__snapshots__/useQuery.test.tsx.snap @@ -0,0 +1,17 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`useQuery Hook Partial refetching should attempt a refetch when the query result was marked as being partial, the returned data was reset to an empty Object by the Apollo Client QueryManager (due to a cache miss), and the \`partialRefetch\` prop is \`true\` 1`] = ` +[MockFunction] { + "calls": Array [ + Array [ + "Missing field 'allPeople' while writing result {}", + ], + ], + "results": Array [ + Object { + "type": "return", + "value": undefined, + }, + ], +} +`; diff --git a/src/react/hooks/__tests__/__snapshots__/useSubscription.test.tsx.snap b/src/react/hooks/__tests__/__snapshots__/useSubscription.test.tsx.snap new file mode 100644 index 00000000000..ad04394392b --- /dev/null +++ b/src/react/hooks/__tests__/__snapshots__/useSubscription.test.tsx.snap @@ -0,0 +1,47 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`useSubscription Hook should handle immediate completions gracefully 1`] = ` +[MockFunction] { + "calls": Array [ + Array [ + "Missing field 'car' while writing result {}", + ], + ], + "results": Array [ + Object { + "type": "return", + "value": undefined, + }, + ], +} +`; + +exports[`useSubscription Hook should handle immediate completions with multiple subscriptions gracefully 1`] = ` +[MockFunction] { + "calls": Array [ + Array [ + "Missing field 'car' while writing result {}", + ], + Array [ + "Missing field 'car' while writing result {}", + ], + Array [ + "Missing field 'car' while writing result {}", + ], + ], + "results": Array [ + Object { + "type": "return", + "value": undefined, + }, + Object { + "type": "return", + "value": undefined, + }, + Object { + "type": "return", + "value": undefined, + }, + ], +} +`; diff --git a/src/react/hooks/__tests__/useQuery.test.tsx b/src/react/hooks/__tests__/useQuery.test.tsx index 00201ec04b6..2f1bff39850 100644 --- a/src/react/hooks/__tests__/useQuery.test.tsx +++ b/src/react/hooks/__tests__/useQuery.test.tsx @@ -8,7 +8,7 @@ import { InMemoryCache } from '../../../cache'; import { ApolloProvider } from '../../context'; import { Observable, Reference, concatPagination } from '../../../utilities'; import { ApolloLink } from '../../../link/core'; -import { itAsync, MockLink, MockedProvider, mockSingleLink } from '../../../testing'; +import { itAsync, MockLink, MockedProvider, mockSingleLink, withErrorSpy } from '../../../testing'; import { useQuery } from '../useQuery'; import { useMutation } from '../useMutation'; import { QueryFunctionOptions } from '../..'; @@ -2022,7 +2022,7 @@ describe('useQuery Hook', () => { }); describe('Partial refetching', () => { - itAsync( + withErrorSpy(itAsync, 'should attempt a refetch when the query result was marked as being ' + 'partial, the returned data was reset to an empty Object by the ' + 'Apollo Client QueryManager (due to a cache miss), and the ' + @@ -2391,13 +2391,15 @@ describe('useQuery Hook', () => { let renderCount = 0; const Component = () => { const [mutate, { loading: mutationLoading }] = useMutation(mutation, { - optimisticResponse: carData, + optimisticResponse: { + addCar: carData, + }, update: (cache, { data }) => { cache.modify({ fields: { cars(existing, { readField }) { const newCarRef = cache.writeFragment({ - data, + data: data!.addCar, fragment: gql`fragment NewCar on Car { id make @@ -2405,7 +2407,7 @@ describe('useQuery Hook', () => { }`, }); if (existing.some( - (ref: Reference) => readField('id', ref) === data!.id + (ref: Reference) => readField('id', ref) === data!.addCar.id )) { return existing; } diff --git a/src/react/hooks/__tests__/useSubscription.test.tsx b/src/react/hooks/__tests__/useSubscription.test.tsx index 9bfe7ecbc7d..20a01f4eeed 100644 --- a/src/react/hooks/__tests__/useSubscription.test.tsx +++ b/src/react/hooks/__tests__/useSubscription.test.tsx @@ -5,7 +5,7 @@ import gql from 'graphql-tag'; import { ApolloClient, ApolloLink, concat } from '../../../core'; import { InMemoryCache as Cache } from '../../../cache'; import { ApolloProvider } from '../../context'; -import { MockSubscriptionLink } from '../../../testing'; +import { MockSubscriptionLink, withErrorSpy } from '../../../testing'; import { useSubscription } from '../useSubscription'; describe('useSubscription Hook', () => { @@ -441,7 +441,7 @@ describe('useSubscription Hook', () => { }); }); - it('should handle immediate completions gracefully', () => { + withErrorSpy(it, 'should handle immediate completions gracefully', () => { const subscription = gql` subscription { car { @@ -452,7 +452,7 @@ describe('useSubscription Hook', () => { const result = { result: { data: null }, - } + }; const link = new MockSubscriptionLink(); const client = new ApolloClient({ @@ -496,7 +496,7 @@ describe('useSubscription Hook', () => { }); }); - it('should handle immediate completions with multiple subscriptions gracefully', () => { + withErrorSpy(it, 'should handle immediate completions with multiple subscriptions gracefully', () => { const subscription = gql` subscription { car { @@ -507,7 +507,7 @@ describe('useSubscription Hook', () => { const result = { result: { data: null }, - } + }; const link = new MockSubscriptionLink(); const client = new ApolloClient({ From 1daffc2b097dff7f6547058da03cd0b20c5c4bb7 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 22 Jun 2021 12:17:48 -0400 Subject: [PATCH 4/4] Mention PR #8416 in CHANGELOG.md. --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index bfe0fa81f3f..e64edb79fab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,9 @@ - The TypeScript return types of the `getLastResult` and `getLastError` methods of `ObservableQuery` now correctly include the possibility of returning `undefined`. If you happen to be calling either of these methods directly, you may need to adjust how the calling code handles the methods' possibly-`undefined` results.
[@benjamn](https://github.com/benjamn) in [#8394](https://github.com/apollographql/apollo-client/pull/8394) +- Log non-fatal `invariant.error` message when fields are missing from result objects written into `InMemoryCache`, rather than throwing an exception. While this change relaxes an exception to be merely an error message, which is usually a backwards-compatible change, the error messages are logged in more cases now than the exception was previously thrown, and those new error messages may be worth investigating to discover potential problems in your application. The errors are not displayed for `@client`-only fields, so adding `@client` is one way to handle/hide the errors for local-only fields. Another general strategy is to use a more precise query to write specific subsets of data into the cache, rather than reusing a larger query that contains fields not present in the written `data`.
+ [@benjamn](https://github.com/benjamn) in [#8416](https://github.com/apollographql/apollo-client/pull/8416) + ### Improvements - `InMemoryCache` now _guarantees_ that any two result objects returned by the cache (from `readQuery`, `readFragment`, etc.) will be referentially equal (`===`) if they are deeply equal. Previously, `===` equality was often achievable for results for the same query, on a best-effort basis. Now, equivalent result objects will be automatically shared among the result trees of completely different queries. This guarantee is important for taking full advantage of optimistic updates that correctly guess the final data, and for "pure" UI components that can skip re-rendering when their input data are unchanged.