From 8623c7341a798c522c5a61fa363f9d15924711ef Mon Sep 17 00:00:00 2001 From: James Baxley Date: Wed, 11 May 2016 20:10:02 -0400 Subject: [PATCH] Performance improvements, and better lifecycle support (#51) --- Changelog.md | 8 +- package.json | 2 +- src/connect.tsx | 92 +++++++-- test/connect/mutations.tsx | 199 ++++++++++++++++++- test/connect/queries.tsx | 398 +++++++++++++++++++++++++++++++++++++ 5 files changed, 680 insertions(+), 19 deletions(-) diff --git a/Changelog.md b/Changelog.md index 89637b3cb6..7566168572 100644 --- a/Changelog.md +++ b/Changelog.md @@ -2,7 +2,13 @@ Expect active development and potentially significant breaking changes in the `0.x` track. We'll try to be diligent about releasing a `1.0` version in a timely fashion (ideally within 1 or 2 months), so that we can take advantage of SemVer to signify breaking changes from that point on. -### v0.3.2 +### v0.3.4 + +Bug: Fix bug where state / props weren't accurate when executing mutations. +Perf: Increase performance by limiting re-renders and re-execution of queries. +Chore: Split tests to make them easier to maintain. + +### v0.3.2 || v0.3.3 (publish fix) Feature: add `startPolling` and `stopPolling` to the prop object for queries Bug: Fix bug where full options were not being passed to watchQuery diff --git a/package.json b/package.json index 8554a7ea20..bccbab0f67 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "react-apollo", - "version": "0.3.3", + "version": "0.3.4", "description": "React data container for Apollo Client", "main": "index.js", "scripts": { diff --git a/src/connect.tsx b/src/connect.tsx index f965852821..1c10f5bf0b 100644 --- a/src/connect.tsx +++ b/src/connect.tsx @@ -27,6 +27,7 @@ import ApolloClient, { readQueryFromStore } from 'apollo-client'; import { GraphQLResult, + Document, } from 'graphql'; export declare interface MapQueriesToPropsOptions { @@ -43,9 +44,9 @@ export declare interface ConnectOptions { mapStateToProps?: IMapStateToProps; mapDispatchToProps?: IMapDispatchToProps; options?: IConnectOptions; - mergeProps?(stateProps: any, dispatchProps: any, ownProps: any): any; - mapQueriesToProps?(opts: MapQueriesToPropsOptions): any; // WatchQueryHandle - mapMutationsToProps?(opts: MapMutationsToPropsOptions): any; // Mutation Handle + mergeProps?(stateProps: Object, dispatchProps: Object, ownProps: Object): Object; + mapQueriesToProps?(opts: MapQueriesToPropsOptions): Object; // WatchQueryHandle + mapMutationsToProps?(opts: MapMutationsToPropsOptions): Object; // Mutation Handle }; const defaultMapQueriesToProps = opts => ({ }); @@ -113,15 +114,18 @@ export default function connect(opts?: ConnectOptions) { client: PropTypes.object.isRequired, }; - // react and react dev tools (HMR) needs + // react / redux and react dev tools (HMR) needs public state: any; // redux state public props: any; // passed props public version: number; + private unsubscribeFromStore: Function; // data storage private store: Store; private client: ApolloClient; // apollo client - private data: any; // apollo data + private data: Object; // apollo data + private previousState: Object; + private previousQueries: Object; // request / action storage private queryHandles: any; @@ -131,6 +135,7 @@ export default function connect(opts?: ConnectOptions) { private haveOwnPropsChanged: boolean; private hasQueryDataChanged: boolean; private hasMutationDataChanged: boolean; + private hasOwnStateChanged: boolean; // the element to render private renderedElement: any; @@ -150,15 +155,17 @@ export default function connect(opts?: ConnectOptions) { const storeState = this.store.getState(); this.state = assign({}, storeState); + this.previousState = storeState; this.data = {}; this.mutations = {}; } componentWillMount() { - const { props, state } = this; - this.subscribeToAllQueries(props, state); - this.createAllMutationHandles(props, state); + const { props } = this; + this.subscribeToAllQueries(props); + this.createAllMutationHandles(props); + this.bindStoreUpdates(); } componentWillReceiveProps(nextProps) { @@ -168,22 +175,53 @@ export default function connect(opts?: ConnectOptions) { // to avoid rebinding queries if nothing has changed if (!isEqual(this.props, nextProps)) { this.haveOwnPropsChanged = true; - this.unsubcribeAllQueries(); - this.subscribeToAllQueries(nextProps, this.state); + this.createAllMutationHandles(nextProps); + this.subscribeToAllQueries(nextProps); + } } shouldComponentUpdate(nextProps, nextState) { return this.haveOwnPropsChanged || + this.hasOwnStateChanged || this.hasQueryDataChanged || this.hasMutationDataChanged; } componentWillUnmount() { this.unsubcribeAllQueries(); + + if (this.unsubscribeFromStore) { + this.unsubscribeFromStore(); + this.unsubscribeFromStore = null; + } + } + + bindStoreUpdates(): void { + const { store, props } = this; + const { reduxRootKey } = this.client; + + this.unsubscribeFromStore = store.subscribe(() => { + let newState = assign({}, store.getState()); + let oldState = assign({}, this.previousState); + + // we remove the apollo key from the store + // because incomming data would trigger unneccesary + // queries and mutations rebuilds + delete newState[reduxRootKey]; + delete oldState[reduxRootKey]; + + if (!isEqual(oldState, newState)) { + this.previousState = newState; + this.hasOwnStateChanged = true; + + this.subscribeToAllQueries(props); + this.createAllMutationHandles(props); + } + }); } - subscribeToAllQueries(props: any, state: any) { + subscribeToAllQueries(props: any) { const { watchQuery, reduxRootKey } = this.client; const { store } = this; @@ -192,6 +230,17 @@ export default function connect(opts?: ConnectOptions) { ownProps: props, }); + const oldQueries = assign({}, this.previousQueries); + this.previousQueries = assign({}, queryHandles); + + // don't re run queries if nothing has changed + if (isEqual(oldQueries, queryHandles)) { + return; + } else if (oldQueries) { + // unsubscribe from previous queries + this.unsubcribeAllQueries(); + } + if (isObject(queryHandles) && Object.keys(queryHandles).length) { this.queryHandles = queryHandles; @@ -316,10 +365,11 @@ export default function connect(opts?: ConnectOptions) { }); } - createAllMutationHandles(props: any, state: any): void { + createAllMutationHandles(props: any): void { + const mutations = mapMutationsToProps({ - state, ownProps: props, + state: this.store.getState(), }); if (isObject(mutations) && Object.keys(mutations).length) { @@ -340,7 +390,10 @@ export default function connect(opts?: ConnectOptions) { } } - createMutationHandle(key: string, method: () => { mutation: string, variables?: any }): () => Promise { + createMutationHandle( + key: string, + method: () => { mutation: Document, variables?: any } + ): () => Promise { const { mutate } = this.client; const { store } = this; @@ -377,6 +430,14 @@ export default function connect(opts?: ConnectOptions) { }; return (...args) => { + const stateAndProps = { + state: store.getState(), + ownProps: this.props, + }; + + // add props and state as last argument of method + args.push(stateAndProps); + const { mutation, variables } = method.apply(this.client, args); return new Promise((resolve, reject) => { this.data[key] = assign(this.data[key], { @@ -403,6 +464,7 @@ export default function connect(opts?: ConnectOptions) { render() { const { haveOwnPropsChanged, + hasOwnStateChanged, hasQueryDataChanged, hasMutationDataChanged, renderedElement, @@ -412,6 +474,7 @@ export default function connect(opts?: ConnectOptions) { } = this; this.haveOwnPropsChanged = false; + this.hasOwnStateChanged = false; this.hasQueryDataChanged = false; this.hasMutationDataChanged = false; @@ -427,6 +490,7 @@ export default function connect(opts?: ConnectOptions) { const mergedPropsAndData = assign({}, props, data, clientProps); if ( !haveOwnPropsChanged && + !hasOwnStateChanged && !hasQueryDataChanged && !hasMutationDataChanged && renderedElement diff --git a/test/connect/mutations.tsx b/test/connect/mutations.tsx index f92ee6b8db..1a9885bbb2 100644 --- a/test/connect/mutations.tsx +++ b/test/connect/mutations.tsx @@ -3,7 +3,7 @@ import * as React from 'react'; import * as chai from 'chai'; import { mount } from 'enzyme'; -import { createStore, combineReducers, applyMiddleware } from 'redux'; +import { createStore, combineReducers, applyMiddleware, ReducersMapObject } from 'redux'; import { connect as ReactReduxConnect } from 'react-redux'; import gql from 'apollo-client/gql'; import assign = require('object-assign'); @@ -371,9 +371,9 @@ describe('mutations', () => { }); function mapMutationsToProps({ ownProps }) { - expect(ownProps.listId).to.equal('1'); return { makeListPrivate: () => { + expect(ownProps.listId).to.equal('1'); return { mutation, variables: { @@ -442,9 +442,10 @@ describe('mutations', () => { }); function mapMutationsToProps({ state }) { - expect(state.listId).to.equal('1'); + return { makeListPrivate: () => { + expect(state.listId).to.equal('1'); return { mutation, variables: { @@ -482,6 +483,198 @@ describe('mutations', () => { ); }); + it('gets the correct state at muation execution', (done) => { + + const mutation = gql` + mutation makeListPrivate($listId: ID!) { + makeListPrivate(id: $listId) + } + `; + + const variables1 = { + listId: '1', + }; + + const data1 = { + makeListPrivate: true, + }; + + const variables2 = { + listId: '2', + }; + + const data2 = { + makeListPrivate: false, + }; + + const networkInterface = mockNetworkInterface( + { + request: { query: mutation, variables: variables1 }, + result: { data: data1 }, + }, + { + request: { query: mutation, variables: variables2 }, + result: { data: data2 }, + } + ); + + const client = new ApolloClient({ + networkInterface, + }); + + function counter(state = 1, action) { + switch (action.type) { + case 'INCREMENT': + return state + 1 + default: + return state + } + } + + // Typscript workaround + const apolloReducer = client.reducer() as () => any; + + const store = createStore( + combineReducers({ + counter, + apollo: apolloReducer + }), + applyMiddleware(client.middleware()) + ); + + function mapMutationsToProps({ state }) { + return { + makeListPrivate: () => { + expect(state.counter).to.equal(2); + done(); + return { + mutation, + variables: { + listId: state.counter, + }, + }; + }, + }; + }; + + let hasFiredOnce = false; + @connect({ mapMutationsToProps }) + class Container extends React.Component { + componentDidMount() { + // trigger a store change + this.props.dispatch({ type: 'INCREMENT' }); + this.props.mutations.makeListPrivate(); + } + + render() { + return ; + } + }; + + mount( + + + + ); + }); + + it('gets the correct props at muation execution', (done) => { + + const mutation = gql` + mutation makeListPrivate($listId: ID!) { + makeListPrivate(id: $listId) + } + `; + + const variables1 = { + listId: '1', + }; + + const data1 = { + makeListPrivate: true, + }; + + const variables2 = { + listId: '2', + }; + + const data2 = { + makeListPrivate: false, + }; + + const networkInterface = mockNetworkInterface( + { + request: { query: mutation, variables: variables1 }, + result: { data: data1 }, + }, + { + request: { query: mutation, variables: variables2 }, + result: { data: data2 }, + } + ); + + const client = new ApolloClient({ + networkInterface, + }); + + function mapMutationsToProps({ ownProps }) { + return { + makeListPrivate: () => { + expect(ownProps.listId).to.equal(2); + done(); + return { + mutation, + variables: { + listId: ownProps.listId, + }, + }; + }, + }; + }; + + let hasFiredOnce = false; + @connect({ mapMutationsToProps }) + class Container extends React.Component { + + componentDidUpdate() { + if (this.props.listId === 2 && !hasFiredOnce) { + hasFiredOnce = true; + this.props.mutations.makeListPrivate(); + } + } + + render() { + return ; + } + }; + + class ChangingProps extends React.Component { + + state = { + listId: 1 + } + + componentDidMount() { + setTimeout(() => { + this.setState({ + listId: 2, + }); + }, 50); + } + + render() { + return + } + + } + + mount( + + + + ); + }); + it('should allow passing custom arugments to mutation handle', (done) => { const store = createStore(() => ({ })); diff --git a/test/connect/queries.tsx b/test/connect/queries.tsx index e38c692fb4..72d808668e 100644 --- a/test/connect/queries.tsx +++ b/test/connect/queries.tsx @@ -26,6 +26,120 @@ import { import connect from '../../src/connect'; describe('queries', () => { + it('doesn\'t rerun the query if it doesn\'t change', (done) => { + const query = gql` + query people($person: Int!) { + allPeople(first: $person) { + people { + name + } + } + } + `; + + const data1 = { + allPeople: { + people: [ + { + name: 'Luke Skywalker', + }, + ], + }, + }; + + const data2 = { + allPeople: { + people: [ + { + name: 'Leia Skywalker', + }, + ], + }, + }; + + const variables1 = { + person: 1 + } + + const variables2 = { + person: 2 + } + + const networkInterface = mockNetworkInterface( + { + request: { query, variables: variables1 }, + result: { data: data1 }, + }, + { + request: { query, variables: variables2 }, + result: { data: data2 }, + } + ); + + const client = new ApolloClient({ + networkInterface, + }); + + function mapQueriesToProps({ state }) { + return { + foobar: { + query, + variables: { + person: 1, + } + }, + }; + }; + + function counter(state = 1, action) { + switch (action.type) { + case 'INCREMENT': + return state + 1 + default: + return state + } + } + + // Typscript workaround + const apolloReducer = client.reducer() as () => any; + + const store = createStore( + combineReducers({ + counter, + apollo: apolloReducer + }), + applyMiddleware(client.middleware()) + ); + + let hasDispatched = false; + let count = 0; + @connect({ mapQueriesToProps }) + class Container extends React.Component { + + componentWillReceiveProps(nextProps) { + count++; + if (nextProps.foobar.allPeople && !hasDispatched) { + hasDispatched = true; + this.props.dispatch({ type: 'INCREMENT' }); + } + } + render() { + return ; + } + }; + + const wrapper = mount( + + + + ); + + setTimeout(() => { + expect(count).to.equal(2); + done(); + }, 250); + }); + it('binds a query to props', () => { const store = createStore(() => ({ foo: 'bar', @@ -87,6 +201,290 @@ describe('queries', () => { expect(props.people.loading).to.be.true; }); + it('rebuilds the queries on state change', (done) => { + const query = gql` + query people { + allPeople(first: 1) { + people { + name + } + } + } + `; + + const data = { + allPeople: { + people: [ + { + name: 'Luke Skywalker', + }, + ], + }, + }; + + const networkInterface = mockNetworkInterface({ + request: { query }, + result: { data }, + }); + + const client = new ApolloClient({ + networkInterface, + }); + let wrapper; + let firstRun = true; + function mapQueriesToProps({ state }) { + if (!firstRun) { + expect(state.counter).to.equal(2); + wrapper.unmount(); + done(); + } else { + firstRun = false; + } + return { + people: { query }, + }; + }; + + function counter(state = 1, action) { + switch (action.type) { + case 'INCREMENT': + return state + 1 + default: + return state + } + } + + // Typscript workaround + const apolloReducer = client.reducer() as () => any; + + const store = createStore( + combineReducers({ + counter, + apollo: apolloReducer + }), + applyMiddleware(client.middleware()) + ); + + let hasDispatched = false; + @connect({ mapQueriesToProps }) + class Container extends React.Component { + componentWillReceiveProps(nextProps) { + if (nextProps.people.allPeople && !hasDispatched) { + hasDispatched = true; + this.props.dispatch({ type: 'INCREMENT' }); + } + } + render() { + return ; + } + }; + + wrapper = mount( + + + + ) as any; + + }); + + it('rebuilds the queries on prop change', (done) => { + const query = gql` + query people { + allPeople(first: 1) { + people { + name + } + } + } + `; + + const data = { + allPeople: { + people: [ + { + name: 'Luke Skywalker', + }, + ], + }, + }; + + const networkInterface = mockNetworkInterface({ + request: { query }, + result: { data }, + }); + + const client = new ApolloClient({ + networkInterface, + }); + + let firstRun = true; + function mapQueriesToProps({ ownProps }) { + if (!firstRun) { + expect(ownProps.listId).to.equal(2); + done(); + } else { + firstRun = false; + } + return { + people: { query }, + }; + }; + + + let hasDispatched = false; + @connect({ mapQueriesToProps }) + class Container extends React.Component { + render() { + return ; + } + }; + + class ChangingProps extends React.Component { + + state = { + listId: 1 + } + + componentDidMount() { + setTimeout(() => { + this.setState({ + listId: 2, + }); + }, 50); + } + + render() { + return + } + + } + + mount( + + + + ); + + }); + + it('does rerun the query if it changes', (done) => { + const query = gql` + query people($person: Int!) { + allPeople(first: $person) { + people { + name + } + } + } + `; + + const data1 = { + allPeople: { + people: [ + { + name: 'Luke Skywalker', + }, + ], + }, + }; + + const data2 = { + allPeople: { + people: [ + { + name: 'Leia Skywalker', + }, + ], + }, + }; + + const variables1 = { + person: 1 + } + + const variables2 = { + person: 2 + } + + const networkInterface = mockNetworkInterface( + { + request: { query, variables: variables1 }, + result: { data: data1 }, + }, + { + request: { query, variables: variables2 }, + result: { data: data2 }, + } + ); + + const client = new ApolloClient({ + networkInterface, + }); + + let firstRun = true; + function mapQueriesToProps({ state }) { + return { + people: { + query, + variables: { + person: state.counter, + } + }, + }; + }; + + function counter(state = 1, action) { + switch (action.type) { + case 'INCREMENT': + return state + 1 + default: + return state + } + } + + // Typscript workaround + const apolloReducer = client.reducer() as () => any; + + const store = createStore( + combineReducers({ + counter, + apollo: apolloReducer + }), + applyMiddleware(client.middleware()) + ); + + let hasDispatched = false; + let count = 0; + @connect({ mapQueriesToProps }) + class Container extends React.Component { + componentDidMount(){ + count++; // increase for the loading + } + + componentWillReceiveProps(nextProps) { + count++; + if (nextProps.people.allPeople && !hasDispatched) { + hasDispatched = true; + this.props.dispatch({ type: 'INCREMENT' }); + } + } + render() { + return ; + } + }; + + mount( + + + + ); + + setTimeout(() => { + expect(count).to.equal(3); + done(); + }, 250); + }); + it('stops the query after unmounting', () => { const query = gql` query people {