fix indefinite loading on remount with null data

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
 

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;
 

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,121 @@ 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<any, any> {
+      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.false;
+          done();
+        }
+        count ++;
+        return null;
+      }
+    };
+    const main = document.createElement('DIV');
+    main.id = 'main';
+    document.body.appendChild(main);
+
+    const render = (props) => {
+      ReactDOM.render((
+        <ProviderMock client={client}>
+          <Container {...props} />
+        </ProviderMock>
+      ), document.getElementById('main'));
+    };
+
+    // Initial render.
+    render(variables);
+
+    // Prop update: fetch.
+    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<any, any> {
+         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<any, any> {
+       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(<ProviderMock client={client}><Container /></ProviderMock>);
+   });
+
   it('executes a query with two root fields', (done) => {
     const query = gql`query people {
       allPeople(first: 1) { people { name } }

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() {