Merge pull request #376 from apollostack/358-skip-to-no-skip

Ensure that we always change options, even when unsubbed

src/graphql.tsx

@@ -201,7 +201,7 @@ export default function graphql(
         if (this.type === DocumentType.Mutation) return;
 
         if (!this.shouldSkip(this.props)) {
-          this.subscribeToQuery(this.props);
+          this.subscribeToQuery();
         }
       }
 
@@ -222,8 +222,8 @@ export default function graphql(
           return;
         }
 
-        // we got new props, we need to unsubscribe and re-subscribe with the new data
-        this.subscribeToQuery(nextProps);
+        this.updateQuery(nextProps);
+        this.subscribeToQuery();
       }
 
       shouldComponentUpdate(nextProps, nextState, nextContext) {
@@ -315,6 +315,24 @@ export default function graphql(
         }
       }
 
+      updateQuery(props) {
+        const opts = this.calculateOptions(props) as QueryOptions;
+
+        // if we skipped initially, we may not have yet created the observable
+        if (!this.queryObservable) {
+          this.createQuery(opts);
+        }
+
+        if (this.queryObservable._setOptionsNoResult) {
+          // Since we don't care about the result, use a hacky version to
+          // work around https://github.com/apollostack/apollo-client/pull/694
+          // This workaround is only present in Apollo Client 0.4.21
+          this.queryObservable._setOptionsNoResult(opts);
+        } else {
+          this.queryObservable.setOptions(opts);
+        }
+      }
+
       // For server-side rendering (see server.ts)
       fetchData(): Promise<ApolloQueryResult> | boolean {
         if (this.shouldSkip()) return false;
@@ -336,29 +354,11 @@ export default function graphql(
         }
       }
 
-      subscribeToQuery(props): boolean {
-        const opts = this.calculateOptions(props) as QueryOptions;
-
-        // We've subscribed already, just update with our new options and
-        // take the latest result
+      subscribeToQuery() {
         if (this.querySubscription) {
-          if (this.queryObservable._setOptionsNoResult) {
-            // Since we don't care about the result, use a hacky version to
-            // work around https://github.com/apollostack/apollo-client/pull/694
-            // This workaround is only present in Apollo Client 0.4.21
-            this.queryObservable._setOptionsNoResult(opts);
-          } else {
-            this.queryObservable.setOptions(opts);
-          }
-
           return;
         }
 
-        // if we skipped initially, we may not have yet created the observable
-        if (!this.queryObservable) {
-          this.createQuery(opts);
-        }
-
         const next = (results: any) => {
           if (this.type === DocumentType.Subscription) {
             // Subscriptions don't currently support `currentResult`, so we

test/react-web/client/graphql/queries.test.tsx

@@ -514,6 +514,61 @@ describe('queries', () => {
     renderer.create(<ApolloProvider client={client}><Parent /></ApolloProvider>);
   });
 
+  it('allows you to skip then unskip a query with new options (top-level syntax)', (done) => {
+    const query = gql`query people($first: Int) { allPeople(first: $first) { people { name } } }`;
+    const dataOne = { allPeople: { people: [ { name: 'Luke Skywalker' } ] } };
+    const dataTwo = { allPeople: { people: [ { name: 'Leia Skywalker' } ] } };
+    const networkInterface = mockNetworkInterface(
+      { request: { query, variables: { first: 1 } }, result: { data: dataOne } },
+      { request: { query, variables: { first: 2 } }, result: { data: dataTwo } },
+      { request: { query, variables: { first: 2 } }, result: { data: dataTwo } },
+    );
+    const client = new ApolloClient({ networkInterface, addTypename: false });
+
+    let hasSkipped = false;
+    @graphql(query, { skip: ({ skip }) => skip })
+    class Container extends React.Component<any, any> {8
+      componentWillReceiveProps(newProps) {
+        if (newProps.skip) {
+          hasSkipped = true;
+          // change back to skip: false, with a different variable
+          this.props.setState({ skip: false, first: 2 });
+        } else {
+          if (hasSkipped) {
+            if (!newProps.data.loading) {
+              expect(newProps.data.allPeople).toEqual(dataTwo.allPeople);
+              done();
+            }
+          } else {
+            expect(newProps.data.allPeople).toEqual(dataOne.allPeople);
+            this.props.setState({ skip: true });
+          }
+        }
+      }
+      render() {
+        return null;
+      }
+    };
+
+    class Parent extends React.Component<any, any> {
+      constructor() {
+        super();
+        this.state = { skip: false, first: 1 };
+      }
+      render() {
+        return (
+          <Container
+            skip={this.state.skip}
+            first={this.state.first}
+            setState={(state) => this.setState(state)}
+          />
+        );
+      }
+    };
+
+    renderer.create(<ApolloProvider client={client}><Parent /></ApolloProvider>);
+  });
+
   it('allows you to skip then unskip a query with opts syntax', (done) => {
     const query = gql`query people { allPeople(first: 1) { people { name } } }`;
     const data = { allPeople: { people: [ { name: 'Luke Skywalker' } ] } };