From 9a6d36154e802a4914ea17b85145b71a2063c282 Mon Sep 17 00:00:00 2001 From: Quentin Cuvillier Date: Sun, 29 Apr 2018 18:57:03 -0700 Subject: [PATCH 01/10] Mark Query context client as optional and always use props client over context client. --- src/Query.tsx | 45 ++++++++++++++++++------------- test/client/Query.test.tsx | 55 +++++++++++++++++++++----------------- 2 files changed, 57 insertions(+), 43 deletions(-) diff --git a/src/Query.tsx b/src/Query.tsx index c47d0ae910..97a12e33bd 100644 --- a/src/Query.tsx +++ b/src/Query.tsx @@ -116,7 +116,7 @@ export interface QueryProps { } export interface QueryContext { - client: ApolloClient; + client?: ApolloClient; operations?: Map; } @@ -124,11 +124,12 @@ export default class Query extends QueryProps > { static contextTypes = { - client: PropTypes.object.isRequired, + client: PropTypes.object, operations: PropTypes.object, }; static propTypes = { + client: PropTypes.object, children: PropTypes.func.isRequired, fetchPolicy: PropTypes.string, notifyOnNetworkStatusChange: PropTypes.bool, @@ -160,11 +161,7 @@ export default class Query extends constructor(props: QueryProps, context: QueryContext) { super(props, context); - this.client = props.client || context.client; - invariant( - !!this.client, - `Could not find "client" in the context of Query or as passed props. Wrap the root component in an `, - ); + this.client = this.getClient(props, context); this.initializeQueryObservable(props); } @@ -208,26 +205,20 @@ export default class Query extends return; } - const { client } = nextProps; - if ( - shallowEqual(this.props, nextProps) && - (this.client === client || this.client === nextContext.client) - ) { + const nextClient = this.getClient(nextProps, nextContext); + + if (shallowEqual(this.props, nextProps) && this.client === nextClient) { return; } - if (this.client !== client && this.client !== nextContext.client) { - if (client) { - this.client = client; - } else { - this.client = nextContext.client; - } + if (this.client !== nextClient) { + this.client = nextClient; this.removeQuerySubscription(); this.queryObservable = null; this.previousData = {}; - this.updateQuery(nextProps); } + if (this.props.query !== nextProps.query) { this.removeQuerySubscription(); } @@ -351,6 +342,22 @@ export default class Query extends Object.assign(this.queryObservable!, { lastError, lastResult }); } + private getClient( + props: QueryProps, + context: QueryContext, + ): ApolloClient { + const client = props.client || context.client; + + invariant( + !!client, + 'Could not find "client" in the context of Query or as passed props.', + 'Wrap the root component in an ', + ); + + // fixme: TS doesn't infer that the type cannot be undefined after the invariant. + return client as ApolloClient; + } + private updateCurrentData = () => { // force a rerender that goes through shouldComponentUpdate if (this.hasMounted) this.forceUpdate(); diff --git a/test/client/Query.test.tsx b/test/client/Query.test.tsx index c4c76cfbcb..7eec945246 100644 --- a/test/client/Query.test.tsx +++ b/test/client/Query.test.tsx @@ -889,53 +889,60 @@ describe('Query component', () => { }); it('if the client changes in the context', done => { + function newClient(data: Object) { + const link = mockSingleLink({ + request: { query: allPeopleQuery }, + result: { data }, + }); + + return new ApolloClient({ + link, + cache: new Cache({ addTypename: false }), + }); + } + const data1 = { allPeople: { people: [{ name: 'Luke Skywalker' }] } }; - const link1 = mockSingleLink({ - request: { query: allPeopleQuery }, - result: { data: data1 }, - }); - const client1 = new ApolloClient({ - link: link1, - cache: new Cache({ addTypename: false }), - }); + const client1 = newClient(data1); const data2 = { allPeople: { people: [{ name: 'Han Solo' }] } }; - const link2 = mockSingleLink({ - request: { query: allPeopleQuery }, - result: { data: data2 }, - }); - const client2 = new ApolloClient({ - link: link2, - cache: new Cache({ addTypename: false }), - }); + const client2 = newClient(data2); + + const data3 = { allPeople: { people: [{ name: 'Frodo' }] } }; + const client3 = newClient(data3); let count = 0; + class Component extends React.Component { state = { - client: client1, + propClient: undefined, + providerClient: client1, }; render() { return ( - - + + {result => { if (result.loading) { return null; } + catchAsyncError(done, () => { if (count === 0) { expect(stripSymbols(result.data)).toEqual(data1); - setTimeout(() => { - this.setState({ - client: client2, - }); - }, 0); + setTimeout(() => this.setState({ providerClient: client2 }), 0); } + if (count === 1) { expect(stripSymbols(result.data)).toEqual(data2); + setTimeout(() => this.setState({ propClient: client3 }), 0); + } + + if (count === 2) { + expect(stripSymbols(result.data)).toEqual(data3); done(); } + count++; }); From a9512d179694545acc3aacd78110db303545cafd Mon Sep 17 00:00:00 2001 From: Quentin Cuvillier Date: Sun, 29 Apr 2018 21:37:24 -0700 Subject: [PATCH 02/10] write test as an array of steps --- test/client/Query.test.tsx | 75 ++++++++++++++++++++++---------------- 1 file changed, 44 insertions(+), 31 deletions(-) diff --git a/test/client/Query.test.tsx b/test/client/Query.test.tsx index 7eec945246..8f213b4504 100644 --- a/test/client/Query.test.tsx +++ b/test/client/Query.test.tsx @@ -889,61 +889,74 @@ describe('Query component', () => { }); it('if the client changes in the context', done => { - function newClient(data: Object) { + function newClient(name: string) { const link = mockSingleLink({ request: { query: allPeopleQuery }, - result: { data }, + result: { data: { allPeople: { people: [{ name }] } } }, }); - return new ApolloClient({ - link, - cache: new Cache({ addTypename: false }), - }); + return new ApolloClient({ link, cache: new Cache({ addTypename: false }) }); } - const data1 = { allPeople: { people: [{ name: 'Luke Skywalker' }] } }; - const client1 = newClient(data1); - - const data2 = { allPeople: { people: [{ name: 'Han Solo' }] } }; - const client2 = newClient(data2); + const AckbarClient = newClient('Admiral Ackbar'); + const LukeClient = newClient('Luke Skywalker'); + const HanClient = newClient('Han Solo'); - const data3 = { allPeople: { people: [{ name: 'Frodo' }] } }; - const client3 = newClient(data3); + const steps = [ + { + name: null, + nextState: { query: null, provider: AckbarClient }, + }, + { + name: 'Admiral Ackbar', + nextState: { query: null, provider: LukeClient }, + }, + { + name: 'Luke Skywalker', + nextState: { query: HanClient, provider: LukeClient }, + }, + { + name: 'Han Solo', + nextState: { query: null, provider: AckbarClient }, + }, + { + name: 'Admiral Ackbar', + nextState: null, + }, + ]; let count = 0; - class Component extends React.Component { - state = { - propClient: undefined, - providerClient: client1, - }; + class Component extends React.Component { + componentDidMount() { + this.setState(steps[0].nextState); + } render() { + if (!this.state) { + return null; + } + return ( - - + + {result => { if (result.loading) { return null; } catchAsyncError(done, () => { - if (count === 0) { - expect(stripSymbols(result.data)).toEqual(data1); - setTimeout(() => this.setState({ providerClient: client2 }), 0); - } + const step = steps[count++]; - if (count === 1) { - expect(stripSymbols(result.data)).toEqual(data2); - setTimeout(() => this.setState({ propClient: client3 }), 0); + if (step.name) { + expect(result.data.allPeople.people[0].name).toEqual(step.name); } - if (count === 2) { - expect(stripSymbols(result.data)).toEqual(data3); + if (step.nextState) { + setTimeout(() => this.setState(step.nextState), 0); + } else { done(); } - - count++; }); return null; From 41cca48dee5b14ac4a04b4989a75cd831217c733 Mon Sep 17 00:00:00 2001 From: Quentin Cuvillier Date: Sat, 26 May 2018 02:58:36 -0700 Subject: [PATCH 03/10] better test --- test/client/Query.test.tsx | 98 +++++++++++++++++++------------------- 1 file changed, 50 insertions(+), 48 deletions(-) diff --git a/test/client/Query.test.tsx b/test/client/Query.test.tsx index 8f213b4504..d3e7d164e7 100644 --- a/test/client/Query.test.tsx +++ b/test/client/Query.test.tsx @@ -8,6 +8,7 @@ import catchAsyncError from '../test-utils/catchAsyncError'; import stripSymbols from '../test-utils/stripSymbols'; import { DocumentNode } from 'graphql'; import gql from 'graphql-tag'; +import { wrap } from 'module'; const allPeopleQuery: DocumentNode = gql` query people { @@ -888,7 +889,9 @@ describe('Query component', () => { ); }); - it('if the client changes in the context', done => { + it('use client from props or context', done => { + jest.useFakeTimers(); + function newClient(name: string) { const link = mockSingleLink({ request: { query: allPeopleQuery }, @@ -898,76 +901,75 @@ describe('Query component', () => { return new ApolloClient({ link, cache: new Cache({ addTypename: false }) }); } - const AckbarClient = newClient('Admiral Ackbar'); - const LukeClient = newClient('Luke Skywalker'); - const HanClient = newClient('Han Solo'); - - const steps = [ + // simulate prop & context client changes + // and assert that the component refresh correctly. + const propsChanges = [ { - name: null, - nextState: { query: null, provider: AckbarClient }, + propsClient: null, + contextClient: newClient('Admiral Ackbar'), + renderedResult: (result: any) => + expect(result.data.allPeople.people[0].name).toEqual('Admiral Ackbar'), }, { - name: 'Admiral Ackbar', - nextState: { query: null, provider: LukeClient }, + propsClient: null, + contextClient: newClient('Luke Skywalker'), + renderedResult: (result: any) => + expect(result.data.allPeople.people[0].name).toEqual('Luke Skywalker'), }, { - name: 'Luke Skywalker', - nextState: { query: HanClient, provider: LukeClient }, + propsClient: newClient('Han Solo'), + contextClient: newClient('Luke Skywalker'), + renderedResult: (result: any) => + expect(result.data.allPeople.people[0].name).toEqual('Han Solo'), }, { - name: 'Han Solo', - nextState: { query: null, provider: AckbarClient }, + propsClient: null, + contextClient: newClient('Admiral Ackbar'), + renderedResult: (result: any) => + expect(result.data.allPeople.people[0].name).toEqual('Admiral Ackbar'), }, { - name: 'Admiral Ackbar', - nextState: null, + propsClient: newClient('Luke Skywalker'), + contextClient: null, + renderedResult: (result: any) => + expect(result.data.allPeople.people[0].name).toEqual('Luke Skywalker'), }, ]; - let count = 0; - class Component extends React.Component { - componentDidMount() { - this.setState(steps[0].nextState); - } - render() { - if (!this.state) { + if (Object.keys(this.props).length === 0) { return null; } - return ( - - - {result => { - if (result.loading) { - return null; - } - - catchAsyncError(done, () => { - const step = steps[count++]; + const query = ( + + {result => { + if (result.data && result.data.allPeople) { + this.props.renderedResult(result); + } - if (step.name) { - expect(result.data.allPeople.people[0].name).toEqual(step.name); - } + return null; + }} + + ); - if (step.nextState) { - setTimeout(() => this.setState(step.nextState), 0); - } else { - done(); - } - }); + if (this.props.contextClient) { + return {query}; + } - return null; - }} - - - ); + return query; } } - wrapper = mount(); + const component = mount(); + + propsChanges.forEach(props => { + component.setProps(props); + jest.runAllTimers(); + }); + + done(); }); it('with data while loading', done => { From f4570e076c297b4cf3547d47529f004eb0c6310b Mon Sep 17 00:00:00 2001 From: Quentin Cuvillier Date: Sat, 26 May 2018 03:06:03 -0700 Subject: [PATCH 04/10] cleanup --- test/client/Query.test.tsx | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/test/client/Query.test.tsx b/test/client/Query.test.tsx index 5d9bb59b18..30803a79e9 100644 --- a/test/client/Query.test.tsx +++ b/test/client/Query.test.tsx @@ -8,7 +8,6 @@ import catchAsyncError from '../test-utils/catchAsyncError'; import stripSymbols from '../test-utils/stripSymbols'; import { DocumentNode } from 'graphql'; import gql from 'graphql-tag'; -import { wrap } from 'module'; const allPeopleQuery: DocumentNode = gql` query people { @@ -901,38 +900,35 @@ describe('Query component', () => { return new ApolloClient({ link, cache: new Cache({ addTypename: false }) }); } - // simulate prop & context client changes - // and assert that the component refresh correctly. + const skywalker = newClient('Luke Skywalker'); + const ackbar = newClient('Admiral Ackbar'); + const solo = newClient('Han Solo'); + const propsChanges = [ { propsClient: null, - contextClient: newClient('Admiral Ackbar'), - renderedResult: (result: any) => - expect(result.data.allPeople.people[0].name).toEqual('Admiral Ackbar'), + contextClient: ackbar, + renderedName: (name: string) => expect(name).toEqual('Admiral Ackbar'), }, { propsClient: null, - contextClient: newClient('Luke Skywalker'), - renderedResult: (result: any) => - expect(result.data.allPeople.people[0].name).toEqual('Luke Skywalker'), + contextClient: skywalker, + renderedName: (name: string) => expect(name).toEqual('Luke Skywalker'), }, { - propsClient: newClient('Han Solo'), - contextClient: newClient('Luke Skywalker'), - renderedResult: (result: any) => - expect(result.data.allPeople.people[0].name).toEqual('Han Solo'), + propsClient: solo, + contextClient: skywalker, + renderedName: (name: string) => expect(name).toEqual('Han Solo'), }, { propsClient: null, - contextClient: newClient('Admiral Ackbar'), - renderedResult: (result: any) => - expect(result.data.allPeople.people[0].name).toEqual('Admiral Ackbar'), + contextClient: ackbar, + renderedName: (name: string) => expect(name).toEqual('Admiral Ackbar'), }, { - propsClient: newClient('Luke Skywalker'), + propsClient: skywalker, contextClient: null, - renderedResult: (result: any) => - expect(result.data.allPeople.people[0].name).toEqual('Luke Skywalker'), + renderedName: (name: string) => expect(name).toEqual('Luke Skywalker'), }, ]; @@ -946,7 +942,7 @@ describe('Query component', () => { {result => { if (result.data && result.data.allPeople) { - this.props.renderedResult(result); + this.props.renderedName(result.data.allPeople.people[0].name); } return null; From 0b1251ad60e7b30035c9eab4db354032488fb6ec Mon Sep 17 00:00:00 2001 From: Quentin Cuvillier Date: Mon, 28 May 2018 20:53:09 -0700 Subject: [PATCH 05/10] Change to share client same behaviour and interface of --- src/Mutation.tsx | 14 +++--- src/Query.tsx | 21 ++------- src/component-utils.tsx | 26 +++++++++++ test/client/Mutation.test.tsx | 87 ++++++++++++++++++++++++++++++++++- 4 files changed, 123 insertions(+), 25 deletions(-) create mode 100644 src/component-utils.tsx diff --git a/src/Mutation.tsx b/src/Mutation.tsx index 5a3acfa4ba..b1b13686f5 100644 --- a/src/Mutation.tsx +++ b/src/Mutation.tsx @@ -8,6 +8,7 @@ const shallowEqual = require('fbjs/lib/shallowEqual'); import { OperationVariables, RefetchQueriesProviderFn } from './types'; import { parser, DocumentType } from './parser'; +import { getClient } from './component-utils'; export interface MutationResult> { data?: TData; @@ -16,7 +17,7 @@ export interface MutationResult> { called: boolean; } export interface MutationContext { - client: ApolloClient; + client?: ApolloClient; operations: Map; } @@ -52,6 +53,7 @@ export declare type MutationFn = ( ) => Promise>; export interface MutationProps { + client?: ApolloClient; mutation: DocumentNode; ignoreResults?: boolean; optimisticResponse?: Object; @@ -114,7 +116,7 @@ class Mutation extends React.Compo super(props, context); this.verifyContext(context); - this.client = context.client; + this.client = getClient(props, context); this.verifyDocumentIsMutation(props.mutation); @@ -134,7 +136,8 @@ class Mutation extends React.Compo nextProps: MutationProps, nextContext: MutationContext, ) { - if (shallowEqual(this.props, nextProps) && this.client === nextContext.client) { + const nextClient = getClient(nextProps, nextContext); + if (shallowEqual(this.props, nextProps) && this.client === nextClient) { return; } @@ -142,8 +145,8 @@ class Mutation extends React.Compo this.verifyDocumentIsMutation(nextProps.mutation); } - if (this.client !== nextContext.client) { - this.client = nextContext.client; + if (this.client !== nextClient) { + this.client = nextClient; this.setState(initialState); } } @@ -164,7 +167,6 @@ class Mutation extends React.Compo private runMutation = (options: MutationOptions = {}) => { this.onStartMutation(); - const mutationId = this.generateNewMutationId(); return this.mutate(options) diff --git a/src/Query.tsx b/src/Query.tsx index 97a12e33bd..cbfeef93db 100644 --- a/src/Query.tsx +++ b/src/Query.tsx @@ -12,6 +12,7 @@ import { DocumentNode } from 'graphql'; import { ZenObservable } from 'zen-observable-ts'; import { OperationVariables, GraphqlQueryControls } from './types'; import { parser, DocumentType, IDocumentDefinition } from './parser'; +import { getClient } from './component-utils'; const shallowEqual = require('fbjs/lib/shallowEqual'); const invariant = require('invariant'); @@ -161,7 +162,7 @@ export default class Query extends constructor(props: QueryProps, context: QueryContext) { super(props, context); - this.client = this.getClient(props, context); + this.client = getClient(props, context); this.initializeQueryObservable(props); } @@ -205,7 +206,7 @@ export default class Query extends return; } - const nextClient = this.getClient(nextProps, nextContext); + const nextClient = getClient(nextProps, nextContext); if (shallowEqual(this.props, nextProps) && this.client === nextClient) { return; @@ -342,22 +343,6 @@ export default class Query extends Object.assign(this.queryObservable!, { lastError, lastResult }); } - private getClient( - props: QueryProps, - context: QueryContext, - ): ApolloClient { - const client = props.client || context.client; - - invariant( - !!client, - 'Could not find "client" in the context of Query or as passed props.', - 'Wrap the root component in an ', - ); - - // fixme: TS doesn't infer that the type cannot be undefined after the invariant. - return client as ApolloClient; - } - private updateCurrentData = () => { // force a rerender that goes through shouldComponentUpdate if (this.hasMounted) this.forceUpdate(); diff --git a/src/component-utils.tsx b/src/component-utils.tsx new file mode 100644 index 0000000000..7f3798caba --- /dev/null +++ b/src/component-utils.tsx @@ -0,0 +1,26 @@ +import ApolloClient from 'apollo-client'; +const invariant = require('invariant'); + +export interface CommonComponentProps { + client?: ApolloClient; +} + +export interface CommonComponentContext { + client?: ApolloClient; +} + +export function getClient( + props: CommonComponentProps, + context: CommonComponentContext, +): ApolloClient { + const client = props.client || context.client; + + invariant( + !!client, + 'Could not find "client" in the context of Query or as passed props.', + 'Wrap the root component in an ', + ); + + // fixme: TS doesn't infer that the type cannot be undefined after the invariant. + return client as ApolloClient; +} diff --git a/test/client/Mutation.test.tsx b/test/client/Mutation.test.tsx index 38ea9bec05..ee94726b26 100644 --- a/test/client/Mutation.test.tsx +++ b/test/client/Mutation.test.tsx @@ -7,7 +7,7 @@ import { DataProxy } from 'apollo-cache'; import { ExecutionResult } from 'graphql'; import { ApolloProvider, Mutation, Query } from '../../src'; -import { MockedProvider, mockSingleLink } from '../../src/test-utils'; +import { MockedProvider, MockLink, mockSingleLink } from '../../src/test-utils'; import stripSymbols from '../test-utils/stripSymbols'; @@ -66,6 +66,91 @@ const mocks = [ const cache = new Cache({ addTypename: false }); +it('pick prop client over context client', async done => { + const mock = (text: string) => [ + { + request: { query: mutation }, + result: { + data: { + createTodo: { + __typename: 'Todo', + id: '99', + text, + completed: true, + }, + __typename: 'Mutation', + }, + }, + }, + { + request: { query: mutation }, + result: { + data: { + createTodo: { + __typename: 'Todo', + id: '100', + text, + completed: true, + }, + __typename: 'Mutation', + }, + }, + }, + ]; + + const mocksProps = mock('This is the result of the prop client mutation.'); + const mocksContext = mock('This is the result of the context client mutation.'); + + function mockClient(m: any) { + return new ApolloClient({ + link: new MockLink(m, false), + cache: new Cache({ addTypename: false }), + }); + } + + const contextClient = mockClient(mocksContext); + const propsClient = mockClient(mocksProps); + const spy = jest.fn(); + + const Component = (props: any) => { + return ( + + + {createTodo =>