diff --git a/src/__tests__/ApolloClient.ts b/src/__tests__/ApolloClient.ts index b854ee46c8b..052d51738f9 100644 --- a/src/__tests__/ApolloClient.ts +++ b/src/__tests__/ApolloClient.ts @@ -6,7 +6,6 @@ import { ApolloLink } from '../link/core/ApolloLink'; import { HttpLink } from '../link/http/HttpLink'; import { InMemoryCache } from '../cache/inmemory/inMemoryCache'; import { stripSymbols } from '../utilities/testing/stripSymbols'; -import { withWarning } from '../utilities/testing/wrap'; import { ApolloClient } from '../'; import { DefaultOptions } from '../ApolloClient'; import { FetchPolicy, QueryOptions } from '../core/watchQueryOptions'; @@ -827,7 +826,7 @@ describe('ApolloClient', () => { }), }); - return withWarning(() => { + expect(() => { client.writeQuery({ data: { todos: [ @@ -848,7 +847,7 @@ describe('ApolloClient', () => { } `, }); - }, /Missing field description/); + }).toThrowError(/Missing field description/); }); }); @@ -1109,7 +1108,7 @@ describe('ApolloClient', () => { }), }); - return withWarning(() => { + expect(() => { client.writeFragment({ data: { __typename: 'Bar', i: 10 }, id: 'bar', @@ -1120,7 +1119,7 @@ describe('ApolloClient', () => { } `, }); - }, /Missing field e/); + }).toThrowError(/Missing field e/); }); describe('change will call observable next', () => { diff --git a/src/__tests__/client.ts b/src/__tests__/client.ts index 1517e0cbf7d..59294e1d65b 100644 --- a/src/__tests__/client.ts +++ b/src/__tests__/client.ts @@ -11,7 +11,6 @@ import { WatchQueryOptions, FetchPolicy, WatchQueryFetchPolicy } from '../core/w import { ApolloError } from '../errors/ApolloError'; import { ApolloClient } from '..'; import subscribeAndCount from '../utilities/testing/subscribeAndCount'; -import { withWarning } from '../utilities/testing/wrap'; import { itAsync } from '../utilities/testing/itAsync'; import { mockSingleLink } from '../utilities/testing/mocking/mockLink'; import { NetworkStatus, ObservableQuery } from '../core'; @@ -625,41 +624,6 @@ describe('client', () => { }); }); - itAsync.skip('should pass a network error correctly on a query using an observable network interface with a warning', (resolve, reject) => { - withWarning(() => { - const query = gql` - query people { - allPeople(first: 1) { - people { - name - } - } - } - `; - - const networkError = new Error('Some kind of network error.'); - - const link = ApolloLink.from([ - () => { - return new Observable(_ => { - throw networkError; - }); - }, - ]); - - const client = new ApolloClient({ - link, - cache: new InMemoryCache({ addTypename: false }), - }); - - client.query({ query }).catch((error: ApolloError) => { - expect(error.networkError).toBeDefined(); - expect(error.networkError!.message).toEqual(networkError.message); - resolve(); - }); - }, /deprecated/); - }); - itAsync('should pass a network error correctly on a query with apollo-link network interface', (resolve, reject) => { const query = gql` query people { @@ -2620,9 +2584,13 @@ describe('client', () => { }), }); - return withWarning( - () => client.query({ query }), - /Missing field description/, + return client.query({ query }).then( + result => { + fail("should have errored"); + }, + error => { + expect(error.message).toMatch(/Missing field description /); + }, ).then(resolve, reject); }); diff --git a/src/__tests__/mutationResults.ts b/src/__tests__/mutationResults.ts index 56ef7ad3488..1103d45781e 100644 --- a/src/__tests__/mutationResults.ts +++ b/src/__tests__/mutationResults.ts @@ -6,7 +6,6 @@ import { ApolloLink } from '../link/core/ApolloLink'; import { mockSingleLink } from '../utilities/testing/mocking/mockLink'; import { ApolloClient } from '..'; import { InMemoryCache } from '../cache/inmemory/inMemoryCache'; -import { withWarning } from '../utilities/testing/wrap'; import { itAsync } from '../utilities/testing/itAsync'; describe('mutation results', () => { @@ -358,45 +357,50 @@ describe('mutation results', () => { }, }; - return withWarning(() => { - const { client, obsQuery } = setupObsQuery( - reject, - { - request: { query: queryTodos }, - result: queryTodosResult, - }, - { - request: { query: mutationTodo }, - result: mutationTodoResult, - }, - ); + const { client, obsQuery } = setupObsQuery( + reject, + { + request: { query: queryTodos }, + result: queryTodosResult, + }, + { + request: { query: mutationTodo }, + result: mutationTodoResult, + }, + ); - return obsQuery.result().then(() => { - // we have to actually subscribe to the query to be able to update it - return new Promise(resolve => { - handle = client.watchQuery({ query: queryTodos }); - subscriptionHandle = handle.subscribe({ - next(res: any) { - counter++; - resolve(res); - }, - }); - }); - }).then(() => client.mutate({ - mutation: mutationTodo, - updateQueries: { - todos: (prev, { mutationResult }) => { - const newTodo = (mutationResult as any).data.createTodo; - const newResults = { - todos: [...(prev as any).todos, newTodo], - }; - return newResults; + return obsQuery.result().then(() => { + // we have to actually subscribe to the query to be able to update it + return new Promise(resolve => { + handle = client.watchQuery({ query: queryTodos }); + subscriptionHandle = handle.subscribe({ + next(res: any) { + counter++; + resolve(res); }, + }); + }); + }).then(() => client.mutate({ + mutation: mutationTodo, + updateQueries: { + todos: (prev, { mutationResult }) => { + const newTodo = (mutationResult as any).data.createTodo; + const newResults = { + todos: [...(prev as any).todos, newTodo], + }; + return newResults; }, - })).then( - () => subscriptionHandle.unsubscribe() - ).then(resolve, reject); - }, /Missing field description/); + }, + })).then( + () => { + subscriptionHandle.unsubscribe(); + fail("should have errored"); + }, + error => { + subscriptionHandle.unsubscribe(); + expect(error.message).toMatch(/Missing field description /); + }, + ).then(resolve, reject); }); describe('updateQueries', () => { diff --git a/src/cache/inmemory/__tests__/roundtrip.ts b/src/cache/inmemory/__tests__/roundtrip.ts index ba6a75796d0..c56afc07868 100644 --- a/src/cache/inmemory/__tests__/roundtrip.ts +++ b/src/cache/inmemory/__tests__/roundtrip.ts @@ -2,7 +2,6 @@ import { DocumentNode } from 'graphql'; import gql from 'graphql-tag'; import { withError } from './diffAgainstStore'; -import { withWarning } from './writeToStore'; import { EntityStore } from '../entityStore'; import { StoreReader } from '../readFromStore'; import { StoreWriter } from '../writeToStore'; @@ -311,7 +310,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', () => { - return withWarning(() => expect(() => { + expect(() => { storeRoundtrip( gql` query { @@ -337,7 +336,7 @@ describe('roundtrip', () => { ], }, ); - }).toThrowError(/Can\'t find field rank on object/)); + }).toThrowError(/Missing field rank /); }); it('should resolve fields it can on interface with non matching inline fragments', () => { @@ -451,7 +450,7 @@ describe('roundtrip', () => { }); it('should throw on error on two of the same spread fragment types', () => { - withWarning(() => expect(() => { + expect(() => { storeRoundtrip( gql` fragment jediSide on Jedi { @@ -481,7 +480,7 @@ describe('roundtrip', () => { ], }, ); - }).toThrowError(/Can\'t find field rank on object/)); + }).toThrowError(/Missing 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 c481cc9703b..665a1721225 100644 --- a/src/cache/inmemory/__tests__/writeToStore.ts +++ b/src/cache/inmemory/__tests__/writeToStore.ts @@ -20,21 +20,6 @@ import { defaultNormalizedCacheFactory } from '../entityStore'; import { InMemoryCache } from '../inMemoryCache'; import { Policies } from '../policies'; -export function withWarning(func: Function, regex?: RegExp) { - let message: string = null as never; - const oldWarn = console.warn; - - console.warn = (m: string) => (message = m); - - return Promise.resolve(func()).then(val => { - if (regex) { - expect(message).toMatch(regex); - } - console.warn = oldWarn; - return val; - }); -} - const getIdField = ({ id }: { id: string }) => id; describe('writing to the store', () => { @@ -1563,14 +1548,12 @@ describe('writing to the store', () => { }), }); - return withWarning(() => { - const newStore = writer.writeQueryToStore({ + expect(() => { + writer.writeQueryToStore({ query, result, }); - - expect((newStore as any).lookup('1')).toEqual(result.todos[0]); - }, /Missing field description/); + }).toThrowError(/Missing field description/); }); it('should warn when it receives the wrong data inside a fragment', () => { @@ -1617,14 +1600,12 @@ describe('writing to the store', () => { }), }); - return withWarning(() => { - const newStore = writer.writeQueryToStore({ + expect(() => { + writer.writeQueryToStore({ query: queryWithInterface, result, }); - - expect((newStore as any).lookup('1')).toEqual(result.todos[0]); - }, /Missing field price/); + }).toThrowError(/Missing field price/); }); it('should warn if a result is missing __typename when required', () => { @@ -1645,14 +1626,12 @@ describe('writing to the store', () => { }), }); - return withWarning(() => { - const newStore = writer.writeQueryToStore({ + expect(() => { + writer.writeQueryToStore({ query: addTypenameToDocument(query), result, }); - - expect((newStore as any).lookup('1')).toEqual(result.todos[0]); - }, /Missing field __typename/); + }).toThrowError(/Missing field __typename/); }); it('should not warn if a field is null', () => { diff --git a/src/cache/inmemory/writeToStore.ts b/src/cache/inmemory/writeToStore.ts index de617f5dddf..8b4ea9572be 100644 --- a/src/cache/inmemory/writeToStore.ts +++ b/src/cache/inmemory/writeToStore.ts @@ -1,5 +1,5 @@ import { SelectionSetNode, FieldNode, DocumentNode } from 'graphql'; -import { invariant } from 'ts-invariant'; +import { InvariantError } from 'ts-invariant'; import { createFragmentMap, @@ -22,7 +22,7 @@ import { StoreObject, } from '../../utilities/graphql/storeUtils'; -import { shouldInclude } from '../../utilities/graphql/directives'; +import { shouldInclude, hasDirectives } from '../../utilities/graphql/directives'; import { cloneDeep } from '../../utilities/common/cloneDeep'; import { Policies, ReadMergeContext } from './policies'; @@ -229,18 +229,9 @@ export class StoreWriter { } else if ( policies.usingPossibleTypes && - !( - selection.directives && - selection.directives.some( - ({ name }) => - name && (name.value === 'defer' || name.value === 'client'), - ) - ) + !hasDirectives(["defer", "client"], selection) ) { - // XXX We'd like to throw an error, but for backwards compatibility's sake - // we just print a warning for the time being. - //throw new WriteError(`Missing field ${resultFieldKey} in ${JSON.stringify(result, null, 2).substring(0, 100)}`); - invariant.warn( + throw new InvariantError( `Missing field ${resultFieldKey} in ${JSON.stringify( result, null, diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index 240021df4d4..22c4b90c929 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -239,14 +239,21 @@ export class QueryManager { self.mutationStore.markMutationResult(mutationId); if (fetchPolicy !== 'no-cache') { - markMutationResult({ - mutationId, - result, - document: mutation, - variables, - queryUpdatersById: generateUpdateQueriesInfo(), - update: updateWithProxyFn, - }, self.cache); + try { + markMutationResult({ + mutationId, + result, + document: mutation, + variables, + queryUpdatersById: generateUpdateQueriesInfo(), + update: updateWithProxyFn, + }, self.cache); + } catch (e) { + error = new ApolloError({ + networkError: e, + }); + return; + } } storeResult = result as FetchResult; diff --git a/src/utilities/graphql/directives.ts b/src/utilities/graphql/directives.ts index 31658f1be16..9f5d10748f9 100644 --- a/src/utilities/graphql/directives.ts +++ b/src/utilities/graphql/directives.ts @@ -8,6 +8,7 @@ import { DocumentNode, ArgumentNode, ValueNode, + ASTNode, } from 'graphql'; import { visit } from 'graphql/language/visitor'; @@ -42,10 +43,10 @@ export function shouldInclude( }); } -export function getDirectiveNames(doc: DocumentNode) { +export function getDirectiveNames(root: ASTNode) { const names: string[] = []; - visit(doc, { + visit(root, { Directive(node) { names.push(node.name.value); }, @@ -54,8 +55,8 @@ export function getDirectiveNames(doc: DocumentNode) { return names; } -export function hasDirectives(names: string[], doc: DocumentNode) { - return getDirectiveNames(doc).some( +export function hasDirectives(names: string[], root: ASTNode) { + return getDirectiveNames(root).some( (name: string) => names.indexOf(name) > -1, ); } diff --git a/src/utilities/testing/wrap.ts b/src/utilities/testing/wrap.ts index 6e635a29903..6c020a3ef86 100644 --- a/src/utilities/testing/wrap.ts +++ b/src/utilities/testing/wrap.ts @@ -11,19 +11,6 @@ export default ( } }; -export function withWarning(func: Function, regex: RegExp) { - let message: string = null as never; - const oldWarn = console.warn; - - console.warn = (m: string) => (message = m); - - return Promise.resolve(func()).then(val => { - expect(message).toMatch(regex); - console.warn = oldWarn; - return val; - }); -} - export function withError(func: Function, regex: RegExp) { let message: string = null as never; const oldError = console.error;