From aedad5a6e2c6e02b5f3816e9d42ec4647fcd319b Mon Sep 17 00:00:00 2001 From: James Baxley Date: Mon, 5 Sep 2016 12:13:08 -0400 Subject: [PATCH 1/4] create failing test scenario --- test/react-web/client/graphql/queries.tsx | 49 +++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/test/react-web/client/graphql/queries.tsx b/test/react-web/client/graphql/queries.tsx index 0a708cc1a5..4a8b833c80 100644 --- a/test/react-web/client/graphql/queries.tsx +++ b/test/react-web/client/graphql/queries.tsx @@ -1,5 +1,6 @@ import * as React from 'react'; +import * as ReactDOM from 'react-dom'; import * as chai from 'chai'; import { mount } from 'enzyme'; import gql from 'graphql-tag'; @@ -233,6 +234,54 @@ describe('queries', () => { wrapper = mount(render(1)); }); + it('correctly sets loading state on remounted component with changed variables (alt)', (done) => { + const query = gql` + query remount($name: String) { allPeople(name: $name) { people { name } } } + `; + const data = { allPeople: null }; + const variables = { name: 'does-not-exist' }; + const variables2 = { name: 'nothing-either' }; + const networkInterface = mockNetworkInterface( + { request: { query, variables }, result: { data }, delay: 10 }, + { request: { query, variables: variables2 }, result: { data }, delay: 10 } + ); + const client = new ApolloClient({ networkInterface }); + let count = 0; + + @graphql(query) + class Container extends React.Component { + render() { + const { loading } = this.props.data; + if (count === 0) expect(loading).to.be.true; + if (count === 1) expect(loading).to.be.false; + if (count === 2) expect(loading).to.be.true; + if (count === 3) { + expect(loading).to.be.true; + done(); + } + count ++; + return null; + } + }; + const main = document.createElement('DIV'); + main.id = 'main'; + document.body.appendChild(main); + + const render = (props) => { + ReactDOM.render(( + + + + ), document.getElementById('main')); + }; + + // Initial render. + render(variables); + + // Prop update: fetch. + setTimeout(() => render(variables2), 1000); + }); + it('executes a query with two root fields', (done) => { const query = gql`query people { allPeople(first: 1) { people { name } } From 80f447107d2a102726dc34a7f4a7ea6636ed394c Mon Sep 17 00:00:00 2001 From: James Baxley Date: Mon, 5 Sep 2016 19:37:56 -0400 Subject: [PATCH 2/4] fix remount loading state on new variables --- src/graphql.tsx | 15 +++++++++++++-- test/react-web/client/graphql/queries.tsx | 2 +- test/react-web/client/libraries/redux.tsx | 6 ++++++ 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/graphql.tsx b/src/graphql.tsx index f5dabed099..8ff08ebe05 100644 --- a/src/graphql.tsx +++ b/src/graphql.tsx @@ -384,7 +384,19 @@ export default function graphql( this.forceRenderChildren(); (this.queryObservable as ObservableQuery).refetch(assign( {}, (this.previousOpts as WatchQueryOptions).variables, opts.variables - )); + )) + .then((result) => { + this.data = assign(this.data, result.data, { loading: false }); + this.hasOperationDataChanged = true; + this.forceRenderChildren(); + return result; + }) + .catch(error => { + this.data = assign(this.data, { loading: false, error }); + this.hasOperationDataChanged = true; + this.forceRenderChildren(); + return error; + }); this.previousOpts = opts; return; } @@ -427,7 +439,6 @@ export default function graphql( oldData = {}; const next = ({ data = oldData, loading, error }: any) => { - const { queryId } = observableQuery; let initialVariables = this.store.getState()[reduxRootKey].queries[queryId].variables; diff --git a/test/react-web/client/graphql/queries.tsx b/test/react-web/client/graphql/queries.tsx index 4a8b833c80..091fd43f70 100644 --- a/test/react-web/client/graphql/queries.tsx +++ b/test/react-web/client/graphql/queries.tsx @@ -256,7 +256,7 @@ describe('queries', () => { if (count === 1) expect(loading).to.be.false; if (count === 2) expect(loading).to.be.true; if (count === 3) { - expect(loading).to.be.true; + expect(loading).to.be.false; done(); } count ++; diff --git a/test/react-web/client/libraries/redux.tsx b/test/react-web/client/libraries/redux.tsx index 9f884726b2..3ef922c432 100644 --- a/test/react-web/client/libraries/redux.tsx +++ b/test/react-web/client/libraries/redux.tsx @@ -74,6 +74,7 @@ describe('redux integration', () => { if (nextProps.data.loading) return; expect(nextProps.data.allPeople).to.deep.equal(data2.allPeople); done(); + wrapper.unmount(); } } render() { @@ -134,6 +135,7 @@ describe('redux integration', () => { expect(nextProps.data.loading).to.be.false; expect(nextProps.data.allPeople).to.deep.equal(data.allPeople); done(); + wrapper.unmount(); } render() { @@ -205,6 +207,7 @@ describe('redux integration', () => { expect(value).to.equal(data.allPeople.people[0].name); done(); + // wrapper.unmount(); } render() { @@ -277,6 +280,7 @@ describe('redux integration', () => { if (nextProps.data.loading) return; expect(nextProps.data.allPeople).to.deep.equal(data2.allPeople); done(); + wrapper.unmount(); } } render() { @@ -338,6 +342,7 @@ describe('redux integration', () => { if (nextProps.data.loading) return; expect(nextProps.data.allPeople).to.deep.equal(data2.allPeople); done(); + wrapper.unmount(); } } render() { @@ -394,6 +399,7 @@ describe('redux integration', () => { if (nextProps.data.loading) return; expect(nextProps.data.allPeople).to.deep.equal(data2.allPeople); done(); + wrapper.unmount(); } } render() { From d64ea351bb17329b12249157f852be8003bc406e Mon Sep 17 00:00:00 2001 From: James Baxley Date: Mon, 5 Sep 2016 19:40:10 -0400 Subject: [PATCH 3/4] pull in test from 176 --- test/react-web/client/graphql/queries.tsx | 67 +++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/test/react-web/client/graphql/queries.tsx b/test/react-web/client/graphql/queries.tsx index 091fd43f70..d2df554af0 100644 --- a/test/react-web/client/graphql/queries.tsx +++ b/test/react-web/client/graphql/queries.tsx @@ -282,6 +282,73 @@ describe('queries', () => { setTimeout(() => render(variables2), 1000); }); + it('correctly sets loading state on component with changed variables and unchanged result', (done) => { + const query = gql` + query remount($first: Int) { allPeople(first: $first) { people { name } } } + `; + const data = { allPeople: null }; + const variables = { first: 1 }; + const variables2 = { first: 2 }; + const networkInterface = mockNetworkInterface( + { request: { query, variables }, result: { data }, delay: 10 }, + { request: { query, variables: variables2 }, result: { data }, delay: 10 } + ); + const client = new ApolloClient({ networkInterface }); + let count = 0; + + const connect = (component) : any => { + return class Container extends React.Component { + constructor(props) { + super(props); + + this.state = { + first: 1, + }; + this.setFirst = this.setFirst.bind(this); + } + + setFirst(first) { + this.setState({first}); + } + + render() { + return React.createElement(component, { + first: this.state.first, + setFirst: this.setFirst + }); + } + } + } + + @connect + @graphql(query, { options: ({ first }) => ({ variables: { first }})}) + class Container extends React.Component { + componentWillReceiveProps(props) { + if (count === 0) { // has data + setTimeout(() => { + this.props.setFirst(2); + }, 5); + } + + if (count === 1) { + expect(this.props.data.loading).to.be.true; // on variables change + } + + if (count === 2) { + // new data after fetch + expect(props.data.loading).to.be.false; + done(); + } + count++; + } + render() { + return null; + } + }; + + mount(); + }); + it('executes a query with two root fields', (done) => { const query = gql`query people { allPeople(first: 1) { people { name } } From 6644d69ae7cceed2ee1a868a7429eff0bf0b3191 Mon Sep 17 00:00:00 2001 From: James Baxley Date: Mon, 5 Sep 2016 19:43:30 -0400 Subject: [PATCH 4/4] update changelog --- Changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/Changelog.md b/Changelog.md index 302dac41db..0423ca6814 100644 --- a/Changelog.md +++ b/Changelog.md @@ -9,6 +9,7 @@ Expect active development and potentially significant breaking changes in the `0 - Bug: Fixed ssr fragment issue [#178](https://github.com/apollostack/react-apollo/pull/178) - Feature: Removed client as a prop and fixed warnings when not using ApolloProvider [#189](https://github.com/apollostack/react-apollo/pull/189) - Bug: Fixed loading state for skipped queries [#190](https://github.com/apollostack/react-apollo/pull/190) +- Bug: Fixed loading state on remounted component with different variables ### v0.4.7