From f433edf6ef4e8e2bad196be6760c1a284e152247 Mon Sep 17 00:00:00 2001 From: Dirk-Jan Rutten Date: Fri, 16 Feb 2018 19:03:17 +0100 Subject: [PATCH 1/5] Mutation component (#1520) * Initial implementation for Mutation component * Updated the error message * Improved the component. Added some initial tests * Added more props to the mutation component. * Improved typescript typings * fixed listing issues. * Added onComplete and onError props * Implemented logic for handling changes in props. * Changelog * prop-types validation * The render prop only shows the result of the most recent mutation in flight * Only verify document if the mutation has updated * Moved the onCompletedMutation outside the try-catch * Improved the type for the update prop. * Allow for passing variables as options in the mutation function instead of through the props. --- Changelog.md | 1 + src/Mutation.tsx | 266 ++++++++++++ src/browser.ts | 3 + test/client/Mutation.test.tsx | 785 ++++++++++++++++++++++++++++++++++ 4 files changed, 1055 insertions(+) create mode 100644 src/Mutation.tsx create mode 100644 test/client/Mutation.test.tsx diff --git a/Changelog.md b/Changelog.md index 0e6df33906..190699864e 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,6 +1,7 @@ # Change log ### vNext +* Added `` component [#1520](https://github.com/apollographql/react-apollo/pull/1520) * HoC `props` result-mapping function now receives prior return value as second argument. * Fix errorPolicy when 'all' not passing data and errors diff --git a/src/Mutation.tsx b/src/Mutation.tsx new file mode 100644 index 0000000000..bbeb200a8c --- /dev/null +++ b/src/Mutation.tsx @@ -0,0 +1,266 @@ +import * as React from 'react'; +import * as PropTypes from 'prop-types'; +import ApolloClient, { + PureQueryOptions, + ApolloError +} from 'apollo-client'; +import { DataProxy } from "apollo-cache"; +const invariant = require('invariant'); +import { DocumentNode, GraphQLError } from 'graphql'; +const shallowEqual = require('fbjs/lib/shallowEqual'); + +import { OperationVariables } from './types'; +import { parser, DocumentType } from './parser'; + +export interface MutationResult { + data: TData; + error?: ApolloError; + loading: boolean; +} + +export interface ExecutionResult { + data?: T; + extensions?: { [key: string]: any }; + errors?: GraphQLError[]; +} + +// Improved MutationUpdaterFn type, need to port them back to Apollo Client +export declare type MutationUpdaterFn = (proxy: DataProxy, mutationResult: FetchResult) => void; + +export declare type FetchResult, E = Record> = ExecutionResult & { + extensions?: E; + context?: C; +}; + +export declare type MutationOptions = { + variables?: TVariables +}; + +export interface MutationProps { + mutation: DocumentNode; + optimisticResponse?: Object; + refetchQueries?: string[] | PureQueryOptions[]; + update?: MutationUpdaterFn; + children: ( + mutateFn: (options?: MutationOptions) => void, + result?: MutationResult, + ) => React.ReactNode; + onCompleted?: (data: TData) => void; + onError?: (error: ApolloError) => void; +} + +export interface MutationState { + notCalled: boolean; + error?: ApolloError; + data?: TData; + loading?: boolean; +} + +const initialState = { + notCalled: true, +}; + +class Mutation< + TData = any, + TVariables = OperationVariables +> extends React.Component< + MutationProps, + MutationState +> { + static contextTypes = { + client: PropTypes.object.isRequired, + }; + + static propTypes = { + mutation: PropTypes.object.isRequired, + variables: PropTypes.object, + optimisticResponse: PropTypes.object, + refetchQueries: PropTypes.oneOfType([ + PropTypes.arrayOf(PropTypes.string), + PropTypes.arrayOf(PropTypes.object) + ]), + update: PropTypes.func, + children: PropTypes.func.isRequired, + onCompleted: PropTypes.func, + onError: PropTypes.func + }; + + private client: ApolloClient; + private mostRecentMutationId: number; + + constructor(props: MutationProps, context: any) { + super(props, context); + + this.verifyContext(context); + this.client = context.client; + + this.verifyDocumentIsMutation(props.mutation); + + this.mostRecentMutationId = 0; + this.state = initialState; + } + + componentWillReceiveProps(nextProps, nextContext) { + if ( + shallowEqual(this.props, nextProps) && + this.client === nextContext.client + ) { + return; + } + + if (this.props.mutation !== nextProps.mutation) { + this.verifyDocumentIsMutation(nextProps.mutation); + } + + if (this.client !== nextContext.client) { + this.client = nextContext.client; + this.setState(initialState); + } + } + + render() { + const { children } = this.props; + const { loading, data, error, notCalled } = this.state; + + const result = notCalled + ? undefined + : { + loading, + data, + error, + }; + + return children(this.runMutation, result); + } + + private runMutation = async (options: MutationOptions = {}) => { + this.onStartMutation(); + + const mutationId = this.generateNewMutationId(); + + let response; + try { + response = await this.mutate(options); + } catch (e) { + this.onMutationError(e, mutationId); + return; + } + + this.onCompletedMutation(response, mutationId); + }; + + private mutate = (options: MutationOptions) => { + + const { + mutation, + optimisticResponse, + refetchQueries, + update, + } = this.props; + + const { variables } = options; + + return this.client.mutate({ + mutation, + variables, + optimisticResponse, + refetchQueries, + update, + }); + }; + + private onStartMutation = () => { + if (!this.state.loading) { + this.setState({ + loading: true, + error: undefined, + data: undefined, + notCalled: false, + }); + } + }; + + private onCompletedMutation = (response, mutationId) => { + + const { onCompleted } = this.props; + + const data = response.data as TData; + + const callOncomplete = () => { + if (onCompleted) { + onCompleted(data); + } + } + + if (this.isMostRecentMutation(mutationId)) { + this.setState( + { + loading: false, + data, + }, + () => { + callOncomplete(); + }, + ); + } else { + callOncomplete(); + } + }; + + private onMutationError = (error, mutationId) => { + const { onError } = this.props; + + let apolloError = error as ApolloError; + + const callOnError = () => { + if (onError) { + onError(apolloError); + } + } + + if (this.isMostRecentMutation(mutationId)) { + this.setState( + { + loading: false, + error: apolloError, + }, + () => { + callOnError(); + }, + ); + } + else { + callOnError(); + } + }; + + private generateNewMutationId = () => { + this.mostRecentMutationId = this.mostRecentMutationId + 1; + return this.mostRecentMutationId; + } + + private isMostRecentMutation = mutationId => { + return this.mostRecentMutationId === mutationId; + } + + private verifyDocumentIsMutation = mutation => { + const operation = parser(mutation); + invariant( + operation.type === DocumentType.Mutation, + `The component requires a graphql mutation, but got a ${ + operation.type === DocumentType.Query ? 'query' : 'subscription' + }.`, + ); + }; + + private verifyContext = context => { + invariant( + !!context.client, + `Could not find "client" in the context of Mutation. Wrap the root component in an `, + ); + }; +} + +export default Mutation; diff --git a/src/browser.ts b/src/browser.ts index 05b3d73b00..2f8c132e65 100644 --- a/src/browser.ts +++ b/src/browser.ts @@ -10,6 +10,9 @@ export * from './ApolloProvider'; export { default as Query } from './Query'; export * from './Query'; +export { default as Mutation } from './Mutation'; +export * from './Mutation'; + export { default as graphql } from './graphql'; export * from './graphql'; diff --git a/test/client/Mutation.test.tsx b/test/client/Mutation.test.tsx new file mode 100644 index 0000000000..f912ff0f95 --- /dev/null +++ b/test/client/Mutation.test.tsx @@ -0,0 +1,785 @@ +import * as React from 'react'; +import { mount } from 'enzyme'; +import ApolloClient from 'apollo-client'; +import { InMemoryCache as Cache } from 'apollo-cache-inmemory'; +import gql from 'graphql-tag'; + +import { ApolloProvider, Mutation, Query } from '../../src'; +import { MockedProvider, mockSingleLink } from '../../src/test-utils'; + +import stripSymbols from '../test-utils/stripSymbols'; + +const mutation = gql` + mutation createTodo($text: String!) { + createTodo { + id + text + completed + __typename + } + __typename + } +`; + +const data = { + createTodo: { + __typename: 'Todo', + id: '99', + text: 'This one was created with a mutation.', + completed: true, + }, + __typename: 'Mutation', +}; + +const data2 = { + createTodo: { + __typename: 'Todo', + id: '100', + text: 'This one was created with a mutation.', + completed: true, + }, + __typename: 'Mutation', +}; + +const mocks = [ + { + request: { query: mutation }, + result: { data }, + }, + { + request: { query: mutation }, + result: { data: data2 }, + } +]; + +const cache = new Cache({ addTypename: false }); + +it('performs a mutation', done => { + let count = 0; + const Component = () => ( + + {(createTodo, result) => { + if (count === 0) { + expect(result).toBeUndefined(); + setTimeout(() => { + createTodo(); + }); + } else if (count === 1) { + expect(result).toEqual({ + loading: true, + }); + } else if (count === 2) { + expect(result).toEqual({ + loading: false, + data, + }); + done(); + } + count++; + return
; + }} + + ); + + mount( + + + , + ); +}); + +it("performs a mutation with variables passed as an option", done => { + const variables = { + text: 'play tennis', + }; + + let count = 0; + const Component = () => ( + + {(createTodo, result) => { + if (count === 0) { + expect(result).toBeUndefined(); + setTimeout(() => { + createTodo({ variables }); + }); + } else if (count === 1) { + expect(result).toEqual({ + loading: true, + }); + } else if (count === 2) { + expect(result).toEqual({ + loading: false, + data, + }); + done(); + } + count++; + return
; + }} + + ); + + const mocks = [ + { + request: { query: mutation, variables }, + result: { data }, + } + ]; + + mount( + + + , + ); +}); + +it('only shows result for the latest mutation that is in flight', done => { + let count = 0; + + const onCompleted = dataMutation => { + if (count === 1) { + expect(dataMutation).toEqual(data); + } + else if (count === 3) { + expect(dataMutation).toEqual(data2); + done(); + } + } + const Component = () => ( + + {(createTodo, result) => { + if (count === 0) { + expect(result).toBeUndefined(); + setTimeout(() => { + createTodo(); + createTodo(); + }); + } else if (count === 1) { + expect(result).toEqual({ + loading: true, + }); + } else if (count === 2) { + expect(result).toEqual({ + loading: false, + data: data2, + }); + } + count++; + return
; + }} + + ); + + mount( + + + , + ); +}); + +it("only shows the error for the latest mutation in flight", done => { + let count = 0; + + const onError = error => { + if (count === 1) { + expect(error).toEqual(new Error("Network error: Error 1")); + } + else if (count === 3) { + expect(error).toEqual(new Error("Network error: Error 2")); + done(); + } + } + const Component = () => ( + + {(createTodo, result) => { + if (count === 0) { + expect(result).toBeUndefined(); + setTimeout(() => { + createTodo(); + createTodo(); + }); + } else if (count === 1) { + expect(result).toEqual({ + loading: true, + }); + } else if (count === 2) { + expect(result).toEqual({ + loading: false, + data: undefined, + error: new Error("Network error: Error 2") + }); + } + count++; + return
; + }} + + ); + + const mocksWithErrors = [ + { + request: { query: mutation }, + error: new Error('Error 2'), + }, + { + request: { query: mutation }, + error: new Error('Error 2'), + }, + ]; + + mount( + + + , + ); +}); + + +it('calls the onCompleted prop as soon as the mutation is complete', done => { + let onCompletedCalled = false; + + class Component extends React.Component { + state = { + mutationDone: false, + }; + + onCompleted = mutationData => { + expect(mutationData).toEqual(data); + onCompletedCalled = true; + this.setState({ + mutationDone: true, + }); + }; + + render() { + return ( + + {(createTodo, result) => { + if (!result) { + expect(this.state.mutationDone).toBe(false); + setTimeout(() => { + createTodo(); + }); + } + if (onCompletedCalled) { + expect(this.state.mutationDone).toBe(true); + done(); + } + return null; + }} + + ); + } + } + + mount( + + + , + ); +}); + +it('renders result of the children render prop', () => { + const Component = () => ( + + {(createTodo, result) => { + return
; + }} + + ); + + const wrapper = mount( + + + , + ); + expect(wrapper.find('div').exists()).toBe(true); +}); + +it('renders an error state', done => { + let count = 0; + const Component = () => ( + + {(createTodo, result) => { + if (count === 0) { + setTimeout(() => { + createTodo(); + }); + } else if (count === 1) { + expect(result.loading).toBeTruthy(); + } else if (count === 2) { + expect(result.error).toEqual( + new Error('Network error: error occurred'), + ); + done(); + } + count++; + return
; + }} + + ); + + const mockError = [ + { + request: { query: mutation }, + error: new Error('error occurred'), + }, + ]; + + mount( + + + , + ); +}); + +it('calls the onError prop if the mutation encounters an error', done => { + let onRenderCalled = false; + + class Component extends React.Component { + state = { + mutationError: false, + }; + + onError = error => { + expect(error).toEqual(new Error('Network error: error occurred')); + onRenderCalled = true; + this.setState({ + mutationError: true, + }); + }; + + render() { + const { mutationError } = this.state; + + return ( + + {(createTodo, result) => { + if (!result) { + expect(mutationError).toBe(false); + setTimeout(() => { + createTodo(); + }); + } + if (onRenderCalled) { + expect(mutationError).toBe(true); + done(); + } + return null; + }} + + ); + } + } + + const mockError = [ + { + request: { query: mutation }, + error: new Error('error occurred'), + }, + ]; + + mount( + + + , + ); +}); + +it('returns an optimistic response', done => { + const link = mockSingleLink(...mocks); + const client = new ApolloClient({ + link, + cache, + }); + + const optimisticResponse = { + createTodo: { + id: '99', + text: 'This is an optimistic response', + completed: false, + __typename: 'Todo', + }, + __typename: 'Mutation', + }; + + let count = 0; + const Component = () => ( + + {(createTodo, result) => { + if (count === 0) { + expect(result).toBeUndefined(); + setTimeout(() => { + createTodo(); + const dataInStore = client.cache.extract(true); + expect(dataInStore['Todo:99']).toEqual( + optimisticResponse.createTodo, + ); + }); + } else if (count === 1) { + expect(result).toEqual({ + loading: true, + }); + } else if (count === 2) { + expect(result).toEqual({ + loading: false, + data, + }); + done(); + } + count++; + return
; + }} + + ); + + mount( + + + , + ); +}); + +it('has refetchQueries in the props', done => { + const query = gql` + query getTodo { + todo { + id + text + completed + __typename + } + __typename + } + `; + + const queryData = { + todo: { + id: '1', + text: 'todo from query', + completed: false, + __typename: 'Todo', + }, + __typename: 'Query', + }; + + const mocksWithQuery = [ + ...mocks, + // TODO: Somehow apollo-client makes 3 request + // when refetch queries is enabled?? + { + request: { query }, + result: { data: queryData }, + }, + { + request: { query }, + result: { data: queryData }, + }, + { + request: { query }, + result: { data: queryData }, + }, + ]; + + const refetchQueries = [ + { + query, + }, + ]; + + let count = 0; + const Component = () => ( + + {(createTodo, resultMutation) => ( + + {resultQuery => { + if (count === 0) { + setTimeout(() => { + createTodo(); + }); + } else if (count === 1) { + expect(resultMutation.loading).toBe(true); + expect(resultQuery.loading).toBe(true); + } else if (count === 2) { + expect(resultMutation.loading).toBe(true); + expect(stripSymbols(resultQuery.data)).toEqual(queryData); + done(); + } + count++; + return null; + }} + + )} + + ); + + mount( + + + , + ); +}); + +it('has an update prop for updating the store after the mutation', done => { + const update = (proxy, response) => { + expect(response.data).toEqual(data); + done(); + }; + + let count = 0; + const Component = () => ( + + {(createTodo, result) => { + if (count === 0) { + setTimeout(() => { + createTodo(); + }); + } + count++; + return null; + }} + + ); + + mount( + + + , + ); +}); + +it('updates if the client changes', done => { + const link1 = mockSingleLink({ + request: { query: mutation }, + result: { data }, + }); + const client1 = new ApolloClient({ + link: link1, + cache: new Cache({ addTypename: false }), + }); + + const data2 = { + createTodo: { + __typename: 'Todo', + id: '100', + text: 'After updating client.', + completed: false, + }, + __typename: 'Mutation', + }; + + const link2 = mockSingleLink({ + request: { query: mutation }, + result: { data: data2 }, + }); + + const client2 = new ApolloClient({ + link: link2, + cache: new Cache({ addTypename: false }), + }); + + let count = 0; + class Component extends React.Component { + state = { + client: client1, + }; + + render() { + return ( + + + {(createTodo, result) => { + if (count === 0) { + expect(result).toBeUndefined(); + setTimeout(() => { + createTodo(); + }); + } else if (count === 2) { + expect(result.data).toEqual(data); + setTimeout(() => { + this.setState({ + client: client2, + }); + }); + } else if (count === 3) { + expect(result).toBeUndefined(); + setTimeout(() => { + createTodo(); + }); + } else if (count === 5) { + expect(result.data).toEqual(data2); + done(); + } + count++; + return null; + }} + + + ); + } + } + + mount(); +}); + +it('errors if a query is passed instead of a mutation', () => { + const query = gql` + query todos { + todos { + id + } + } + `; + + // Prevent error from being logged in console of test. + const errorLogger = console.error; + console.error = () => {}; // tslint:disable-line + + expect(() => { + mount( + + {() => null} + , + ); + }).toThrowError( + 'The component requires a graphql mutation, but got a query.', + ); + + console.log = errorLogger; +}); + +it('errors when changing from mutation to a query', done => { + const query = gql` + query todos { + todos { + id + } + } + `; + + class Component extends React.Component { + state = { + query: mutation, + }; + + componentDidCatch(e) { + expect(e).toEqual( + new Error( + 'The component requires a graphql mutation, but got a query.', + ), + ); + done(); + } + render() { + return ( + + {() => { + setTimeout(() => { + this.setState({ + query, + }); + }); + return null; + }} + + ); + } + } + + // Prevent error from being logged in console of test. + const errorLogger = console.error; + console.error = () => {}; // tslint:disable-line + + mount( + + + , + ); + + console.log = errorLogger; +}); + +it('errors if a subscription is passed instead of a mutation', () => { + const subscription = gql` + subscription todos { + todos { + id + } + } + `; + + // Prevent error from being logged in console of test. + const errorLogger = console.error; + console.error = () => {}; // tslint:disable-line + + expect(() => { + mount( + + {() => null} + , + ); + }).toThrowError( + 'The component requires a graphql mutation, but got a subscription.', + ); + + console.log = errorLogger; +}); + +it('errors when changing from mutation to a subscription', done => { + const subscription = gql` + subscription todos { + todos { + id + } + } + `; + + class Component extends React.Component { + state = { + query: mutation, + }; + + componentDidCatch(e) { + expect(e).toEqual( + new Error( + 'The component requires a graphql mutation, but got a subscription.', + ), + ); + done(); + } + render() { + return ( + + {() => { + setTimeout(() => { + this.setState({ + query: subscription, + }); + }); + return null; + }} + + ); + } + } + + // Prevent error from being logged in console of test. + const errorLogger = console.error; + console.error = () => {}; // tslint:disable-line + + mount( + + + , + ); + + console.log = errorLogger; +}); From 63b8b49212281209e8bbc738541a410fa62c28ea Mon Sep 17 00:00:00 2001 From: James Baxley Date: Fri, 16 Feb 2018 20:46:44 -0500 Subject: [PATCH 2/5] removed all types for mutation component --- src/Mutation.tsx | 122 +++++++++++++------------- test/client/Mutation.test.tsx | 160 +++++++++++++++++----------------- 2 files changed, 141 insertions(+), 141 deletions(-) diff --git a/src/Mutation.tsx b/src/Mutation.tsx index bbeb200a8c..405eeba506 100644 --- a/src/Mutation.tsx +++ b/src/Mutation.tsx @@ -1,10 +1,7 @@ import * as React from 'react'; import * as PropTypes from 'prop-types'; -import ApolloClient, { - PureQueryOptions, - ApolloError -} from 'apollo-client'; -import { DataProxy } from "apollo-cache"; +import ApolloClient, { PureQueryOptions, ApolloError } from 'apollo-client'; +import { DataProxy } from 'apollo-cache'; const invariant = require('invariant'); import { DocumentNode, GraphQLError } from 'graphql'; const shallowEqual = require('fbjs/lib/shallowEqual'); @@ -12,30 +9,39 @@ const shallowEqual = require('fbjs/lib/shallowEqual'); import { OperationVariables } from './types'; import { parser, DocumentType } from './parser'; -export interface MutationResult { - data: TData; +type DefaultData = Record; +export interface MutationResult { + data?: TData; error?: ApolloError; - loading: boolean; + loading?: boolean; +} +export interface MutationContext { + client: ApolloClient; } -export interface ExecutionResult { - data?: T; - extensions?: { [key: string]: any }; - errors?: GraphQLError[]; +export interface ExecutionResult { + data?: T; + extensions?: DefaultData; + errors?: GraphQLError[]; } // Improved MutationUpdaterFn type, need to port them back to Apollo Client -export declare type MutationUpdaterFn = (proxy: DataProxy, mutationResult: FetchResult) => void; + } +> = (proxy: DataProxy, mutationResult: FetchResult) => void; -export declare type FetchResult, E = Record> = ExecutionResult & { - extensions?: E; - context?: C; +export declare type FetchResult< + C = Record, + E = Record +> = ExecutionResult & { + extensions?: E; + context?: C; }; export declare type MutationOptions = { - variables?: TVariables + variables?: TVariables; }; export interface MutationProps { @@ -45,7 +51,7 @@ export interface MutationProps { update?: MutationUpdaterFn; children: ( mutateFn: (options?: MutationOptions) => void, - result?: MutationResult, + result?: MutationResult ) => React.ReactNode; onCompleted?: (data: TData) => void; onError?: (error: ApolloError) => void; @@ -72,24 +78,24 @@ class Mutation< static contextTypes = { client: PropTypes.object.isRequired, }; - + static propTypes = { mutation: PropTypes.object.isRequired, variables: PropTypes.object, optimisticResponse: PropTypes.object, refetchQueries: PropTypes.oneOfType([ PropTypes.arrayOf(PropTypes.string), - PropTypes.arrayOf(PropTypes.object) + PropTypes.arrayOf(PropTypes.object), ]), update: PropTypes.func, children: PropTypes.func.isRequired, onCompleted: PropTypes.func, - onError: PropTypes.func + onError: PropTypes.func, }; private client: ApolloClient; private mostRecentMutationId: number; - + constructor(props: MutationProps, context: any) { super(props, context); @@ -97,12 +103,15 @@ class Mutation< this.client = context.client; this.verifyDocumentIsMutation(props.mutation); - + this.mostRecentMutationId = 0; this.state = initialState; } - componentWillReceiveProps(nextProps, nextContext) { + componentWillReceiveProps( + nextProps: MutationProps, + nextContext: MutationContext + ) { if ( shallowEqual(this.props, nextProps) && this.client === nextContext.client @@ -137,9 +146,9 @@ class Mutation< private runMutation = async (options: MutationOptions = {}) => { this.onStartMutation(); - + const mutationId = this.generateNewMutationId(); - + let response; try { response = await this.mutate(options); @@ -147,21 +156,15 @@ class Mutation< this.onMutationError(e, mutationId); return; } - + this.onCompletedMutation(response, mutationId); }; private mutate = (options: MutationOptions) => { - - const { - mutation, - optimisticResponse, - refetchQueries, - update, - } = this.props; - + const { mutation, optimisticResponse, refetchQueries, update } = this.props; + const { variables } = options; - + return this.client.mutate({ mutation, variables, @@ -182,18 +185,20 @@ class Mutation< } }; - private onCompletedMutation = (response, mutationId) => { - + private onCompletedMutation = ( + response: ExecutionResult, + mutationId: number + ) => { const { onCompleted } = this.props; const data = response.data as TData; - + const callOncomplete = () => { if (onCompleted) { onCompleted(data); } - } - + }; + if (this.isMostRecentMutation(mutationId)) { this.setState( { @@ -202,23 +207,23 @@ class Mutation< }, () => { callOncomplete(); - }, + } ); } else { callOncomplete(); } }; - private onMutationError = (error, mutationId) => { + private onMutationError = (error: ApolloError, mutationId: number) => { const { onError } = this.props; let apolloError = error as ApolloError; - + const callOnError = () => { if (onError) { onError(apolloError); } - } + }; if (this.isMostRecentMutation(mutationId)) { this.setState( @@ -228,37 +233,36 @@ class Mutation< }, () => { callOnError(); - }, + } ); - } - else { + } else { callOnError(); } }; - - private generateNewMutationId = () => { + + private generateNewMutationId = (): number => { this.mostRecentMutationId = this.mostRecentMutationId + 1; return this.mostRecentMutationId; - } - - private isMostRecentMutation = mutationId => { + }; + + private isMostRecentMutation = (mutationId: number) => { return this.mostRecentMutationId === mutationId; - } + }; - private verifyDocumentIsMutation = mutation => { + private verifyDocumentIsMutation = (mutation: DocumentNode) => { const operation = parser(mutation); invariant( operation.type === DocumentType.Mutation, `The component requires a graphql mutation, but got a ${ operation.type === DocumentType.Query ? 'query' : 'subscription' - }.`, + }.` ); }; - private verifyContext = context => { + private verifyContext = (context: MutationContext) => { invariant( !!context.client, - `Could not find "client" in the context of Mutation. Wrap the root component in an `, + `Could not find "client" in the context of Mutation. Wrap the root component in an ` ); }; } diff --git a/test/client/Mutation.test.tsx b/test/client/Mutation.test.tsx index f912ff0f95..cb47b156ee 100644 --- a/test/client/Mutation.test.tsx +++ b/test/client/Mutation.test.tsx @@ -1,8 +1,10 @@ import * as React from 'react'; import { mount } from 'enzyme'; -import ApolloClient from 'apollo-client'; -import { InMemoryCache as Cache } from 'apollo-cache-inmemory'; import gql from 'graphql-tag'; +import { ApolloClient } from 'apollo-client'; +import { InMemoryCache as Cache } from 'apollo-cache-inmemory'; +import { DataProxy } from 'apollo-cache'; +import { ExecutionResult } from 'graphql'; import { ApolloProvider, Mutation, Query } from '../../src'; import { MockedProvider, mockSingleLink } from '../../src/test-utils'; @@ -21,7 +23,17 @@ const mutation = gql` } `; -const data = { +type Data = { + createTodo: { + __typename: string; + id: string; + text: string; + completed: boolean; + }; + __typename: string; +}; + +const data: Data = { createTodo: { __typename: 'Todo', id: '99', @@ -31,7 +43,7 @@ const data = { __typename: 'Mutation', }; -const data2 = { +const data2: Data = { createTodo: { __typename: 'Todo', id: '100', @@ -49,7 +61,7 @@ const mocks = [ { request: { query: mutation }, result: { data: data2 }, - } + }, ]; const cache = new Cache({ addTypename: false }); @@ -84,15 +96,15 @@ it('performs a mutation', done => { mount( - , + ); }); -it("performs a mutation with variables passed as an option", done => { +it('performs a mutation with variables passed as an option', done => { const variables = { text: 'play tennis', }; - + let count = 0; const Component = () => ( @@ -118,33 +130,32 @@ it("performs a mutation with variables passed as an option", done => { }} ); - - const mocks = [ + + const mocks1 = [ { request: { query: mutation, variables }, result: { data }, - } + }, ]; mount( - + - , + ); }); it('only shows result for the latest mutation that is in flight', done => { let count = 0; - - const onCompleted = dataMutation => { + + const onCompleted = (dataMutation: Data) => { if (count === 1) { expect(dataMutation).toEqual(data); - } - else if (count === 3) { + } else if (count === 3) { expect(dataMutation).toEqual(data2); done(); } - } + }; const Component = () => ( {(createTodo, result) => { @@ -173,22 +184,21 @@ it('only shows result for the latest mutation that is in flight', done => { mount( - , + ); }); -it("only shows the error for the latest mutation in flight", done => { +it('only shows the error for the latest mutation in flight', done => { let count = 0; - - const onError = error => { + + const onError = (error: Error) => { if (count === 1) { - expect(error).toEqual(new Error("Network error: Error 1")); - } - else if (count === 3) { - expect(error).toEqual(new Error("Network error: Error 2")); + expect(error).toEqual(new Error('Network error: Error 1')); + } else if (count === 3) { + expect(error).toEqual(new Error('Network error: Error 2')); done(); } - } + }; const Component = () => ( {(createTodo, result) => { @@ -206,7 +216,7 @@ it("only shows the error for the latest mutation in flight", done => { expect(result).toEqual({ loading: false, data: undefined, - error: new Error("Network error: Error 2") + error: new Error('Network error: Error 2'), }); } count++; @@ -214,7 +224,7 @@ it("only shows the error for the latest mutation in flight", done => { }} ); - + const mocksWithErrors = [ { request: { query: mutation }, @@ -229,11 +239,10 @@ it("only shows the error for the latest mutation in flight", done => { mount( - , + ); }); - it('calls the onCompleted prop as soon as the mutation is complete', done => { let onCompletedCalled = false; @@ -242,7 +251,7 @@ it('calls the onCompleted prop as soon as the mutation is complete', done => { mutationDone: false, }; - onCompleted = mutationData => { + onCompleted = (mutationData: Data) => { expect(mutationData).toEqual(data); onCompletedCalled = true; this.setState({ @@ -252,10 +261,7 @@ it('calls the onCompleted prop as soon as the mutation is complete', done => { render() { return ( - + {(createTodo, result) => { if (!result) { expect(this.state.mutationDone).toBe(false); @@ -277,23 +283,19 @@ it('calls the onCompleted prop as soon as the mutation is complete', done => { mount( - , + ); }); it('renders result of the children render prop', () => { const Component = () => ( - - {(createTodo, result) => { - return
; - }} - + {() =>
} ); const wrapper = mount( - , + ); expect(wrapper.find('div').exists()).toBe(true); }); @@ -307,11 +309,11 @@ it('renders an error state', done => { setTimeout(() => { createTodo(); }); - } else if (count === 1) { + } else if (count === 1 && result) { expect(result.loading).toBeTruthy(); - } else if (count === 2) { + } else if (count === 2 && result) { expect(result.error).toEqual( - new Error('Network error: error occurred'), + new Error('Network error: error occurred') ); done(); } @@ -331,7 +333,7 @@ it('renders an error state', done => { mount( - , + ); }); @@ -343,7 +345,7 @@ it('calls the onError prop if the mutation encounters an error', done => { mutationError: false, }; - onError = error => { + onError = (error: Error) => { expect(error).toEqual(new Error('Network error: error occurred')); onRenderCalled = true; this.setState({ @@ -384,7 +386,7 @@ it('calls the onError prop if the mutation encounters an error', done => { mount( - , + ); }); @@ -407,10 +409,7 @@ it('returns an optimistic response', done => { let count = 0; const Component = () => ( - + {(createTodo, result) => { if (count === 0) { expect(result).toBeUndefined(); @@ -418,7 +417,7 @@ it('returns an optimistic response', done => { createTodo(); const dataInStore = client.cache.extract(true); expect(dataInStore['Todo:99']).toEqual( - optimisticResponse.createTodo, + optimisticResponse.createTodo ); }); } else if (count === 1) { @@ -441,7 +440,7 @@ it('returns an optimistic response', done => { mount( - , + ); }); @@ -494,10 +493,7 @@ it('has refetchQueries in the props', done => { let count = 0; const Component = () => ( - + {(createTodo, resultMutation) => ( {resultQuery => { @@ -505,10 +501,10 @@ it('has refetchQueries in the props', done => { setTimeout(() => { createTodo(); }); - } else if (count === 1) { + } else if (count === 1 && resultMutation) { expect(resultMutation.loading).toBe(true); expect(resultQuery.loading).toBe(true); - } else if (count === 2) { + } else if (count === 2 && resultMutation) { expect(resultMutation.loading).toBe(true); expect(stripSymbols(resultQuery.data)).toEqual(queryData); done(); @@ -524,12 +520,12 @@ it('has refetchQueries in the props', done => { mount( - , + ); }); it('has an update prop for updating the store after the mutation', done => { - const update = (proxy, response) => { + const update = (_proxy: DataProxy, response: ExecutionResult) => { expect(response.data).toEqual(data); done(); }; @@ -537,7 +533,7 @@ it('has an update prop for updating the store after the mutation', done => { let count = 0; const Component = () => ( - {(createTodo, result) => { + {createTodo => { if (count === 0) { setTimeout(() => { createTodo(); @@ -552,7 +548,7 @@ it('has an update prop for updating the store after the mutation', done => { mount( - , + ); }); @@ -566,7 +562,7 @@ it('updates if the client changes', done => { cache: new Cache({ addTypename: false }), }); - const data2 = { + const data3 = { createTodo: { __typename: 'Todo', id: '100', @@ -578,7 +574,7 @@ it('updates if the client changes', done => { const link2 = mockSingleLink({ request: { query: mutation }, - result: { data: data2 }, + result: { data: data3 }, }); const client2 = new ApolloClient({ @@ -602,7 +598,7 @@ it('updates if the client changes', done => { setTimeout(() => { createTodo(); }); - } else if (count === 2) { + } else if (count === 2 && result) { expect(result.data).toEqual(data); setTimeout(() => { this.setState({ @@ -614,8 +610,8 @@ it('updates if the client changes', done => { setTimeout(() => { createTodo(); }); - } else if (count === 5) { - expect(result.data).toEqual(data2); + } else if (count === 5 && result) { + expect(result.data).toEqual(data3); done(); } count++; @@ -647,10 +643,10 @@ it('errors if a query is passed instead of a mutation', () => { mount( {() => null} - , + ); }).toThrowError( - 'The component requires a graphql mutation, but got a query.', + 'The component requires a graphql mutation, but got a query.' ); console.log = errorLogger; @@ -670,11 +666,11 @@ it('errors when changing from mutation to a query', done => { query: mutation, }; - componentDidCatch(e) { + componentDidCatch(e: Error) { expect(e).toEqual( new Error( - 'The component requires a graphql mutation, but got a query.', - ), + 'The component requires a graphql mutation, but got a query.' + ) ); done(); } @@ -701,7 +697,7 @@ it('errors when changing from mutation to a query', done => { mount( - , + ); console.log = errorLogger; @@ -724,10 +720,10 @@ it('errors if a subscription is passed instead of a mutation', () => { mount( {() => null} - , + ); }).toThrowError( - 'The component requires a graphql mutation, but got a subscription.', + 'The component requires a graphql mutation, but got a subscription.' ); console.log = errorLogger; @@ -747,11 +743,11 @@ it('errors when changing from mutation to a subscription', done => { query: mutation, }; - componentDidCatch(e) { + componentDidCatch(e: Error) { expect(e).toEqual( new Error( - 'The component requires a graphql mutation, but got a subscription.', - ), + 'The component requires a graphql mutation, but got a subscription.' + ) ); done(); } @@ -778,7 +774,7 @@ it('errors when changing from mutation to a subscription', done => { mount( - , + ); console.log = errorLogger; From fbbe3270223f059234253106faaba012a05c2b01 Mon Sep 17 00:00:00 2001 From: James Baxley Date: Fri, 16 Feb 2018 21:12:14 -0500 Subject: [PATCH 3/5] return result from mutate call --- src/Mutation.tsx | 23 ++++++++++++----------- test/client/Mutation.test.tsx | 6 ++++-- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/Mutation.tsx b/src/Mutation.tsx index 405eeba506..7f3a38c0cd 100644 --- a/src/Mutation.tsx +++ b/src/Mutation.tsx @@ -50,7 +50,9 @@ export interface MutationProps { refetchQueries?: string[] | PureQueryOptions[]; update?: MutationUpdaterFn; children: ( - mutateFn: (options?: MutationOptions) => void, + mutateFn: ( + options?: MutationOptions + ) => Promise, result?: MutationResult ) => React.ReactNode; onCompleted?: (data: TData) => void; @@ -144,20 +146,19 @@ class Mutation< return children(this.runMutation, result); } - private runMutation = async (options: MutationOptions = {}) => { + private runMutation = (options: MutationOptions = {}) => { this.onStartMutation(); const mutationId = this.generateNewMutationId(); - let response; - try { - response = await this.mutate(options); - } catch (e) { - this.onMutationError(e, mutationId); - return; - } - - this.onCompletedMutation(response, mutationId); + return this.mutate(options) + .then(response => { + this.onCompletedMutation(response, mutationId); + return response; + }) + .catch(e => { + this.onMutationError(e, mutationId); + }); }; private mutate = (options: MutationOptions) => { diff --git a/test/client/Mutation.test.tsx b/test/client/Mutation.test.tsx index cb47b156ee..f058c0a8af 100644 --- a/test/client/Mutation.test.tsx +++ b/test/client/Mutation.test.tsx @@ -527,7 +527,6 @@ it('has refetchQueries in the props', done => { it('has an update prop for updating the store after the mutation', done => { const update = (_proxy: DataProxy, response: ExecutionResult) => { expect(response.data).toEqual(data); - done(); }; let count = 0; @@ -536,7 +535,10 @@ it('has an update prop for updating the store after the mutation', done => { {createTodo => { if (count === 0) { setTimeout(() => { - createTodo(); + createTodo().then(response => { + expect(response!.data).toEqual(data); + done(); + }); }); } count++; From d57e2b5782a7cf6bf2f8903f294b96a46098777c Mon Sep 17 00:00:00 2001 From: James Baxley Date: Fri, 16 Feb 2018 21:16:17 -0500 Subject: [PATCH 4/5] remove default data type --- src/Mutation.tsx | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Mutation.tsx b/src/Mutation.tsx index 7f3a38c0cd..c91901518d 100644 --- a/src/Mutation.tsx +++ b/src/Mutation.tsx @@ -9,8 +9,7 @@ const shallowEqual = require('fbjs/lib/shallowEqual'); import { OperationVariables } from './types'; import { parser, DocumentType } from './parser'; -type DefaultData = Record; -export interface MutationResult { +export interface MutationResult> { data?: TData; error?: ApolloError; loading?: boolean; @@ -19,9 +18,9 @@ export interface MutationContext { client: ApolloClient; } -export interface ExecutionResult { +export interface ExecutionResult> { data?: T; - extensions?: DefaultData; + extensions?: Record; errors?: GraphQLError[]; } From eb6582f918003d35b5ccddac6a548c6dcc87da6f Mon Sep 17 00:00:00 2001 From: James Baxley Date: Fri, 16 Feb 2018 21:21:17 -0500 Subject: [PATCH 5/5] bundle size increase for mutation component --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 98040f0f8d..b3f2e06549 100644 --- a/package.json +++ b/package.json @@ -36,7 +36,7 @@ "bundlesize": [ { "path": "./lib/react-apollo.browser.umd.js", - "maxSize": "6.5 KB" + "maxSize": "7 KB" } ], "lint-staged": {