Ensure ObservableQuery fields have a stable identity

Make sure `refetch`, `fetchMore`, `updateQuery`, `startPolling`, `stopPolling`, and `subscribeToMore` maintain a stable identity when they're passed back alongside query results. Fixes #3317.
apollographql · Aug 28, 2019 · 8675334 · 8675334
1 parent 1dccd46
commit 8675334
Show file tree

Hide file tree

Showing 4 changed files with 98 additions and 20 deletions.
diff --git a/Changelog.md b/Changelog.md
@@ -8,6 +8,8 @@
   [@hwillson](https://github.com/hwillson) in [#3388](https://github.com/apollographql/react-apollo/pull/3388)
 - Adds support for the `skip` option when using `useSubscription`. <br/>
   [@n1ru4l](https://github.com/n1ru4l) in [#3356](https://github.com/apollographql/react-apollo/pull/3356)
+- Makes sure `refetch`, `fetchMore`, `updateQuery`, `startPolling`, `stopPolling`, and `subscribeToMore` maintain a stable identity when they're passed back alongside query results. <br/>
+  [@hwillson](https://github.com/hwillson) in [#3422](https://github.com/apollographql/react-apollo/pull/3422)
 - Documentation fixes. <br/>
   [@SeanRoberts](https://github.com/SeanRoberts) in [#3380](https://github.com/apollographql/react-apollo/pull/3380)
 

diff --git a/package.json b/package.json
@@ -92,7 +92,7 @@
     {
       "name": "@apollo/react-hooks",
       "path": "./packages/hooks/lib/react-hooks.cjs.min.js",
-      "maxSize": "3.93 kB"
+      "maxSize": "3.98 kB"
     },
     {
       "name": "@apollo/react-ssr",

diff --git a/packages/hooks/src/__tests__/useQuery.test.tsx b/packages/hooks/src/__tests__/useQuery.test.tsx
@@ -2,7 +2,7 @@ import React, { useState, useReducer } from 'react';
 import { DocumentNode, GraphQLError } from 'graphql';
 import gql from 'graphql-tag';
 import { MockedProvider, MockLink } from '@apollo/react-testing';
-import { render, cleanup } from '@testing-library/react';
+import { render, cleanup, wait } from '@testing-library/react';
 import { useQuery, ApolloProvider } from '@apollo/react-hooks';
 import { ApolloClient } from 'apollo-client';
 import { ApolloLink, Observable } from 'apollo-link';
@@ -77,6 +77,50 @@ describe('useQuery Hook', () => {
         </MockedProvider>
       );
     });
+
+    it('should ensure ObservableQuery fields have a stable identity', async () => {
+      let refetchFn: any;
+      let fetchMoreFn: any;
+      let updateQueryFn: any;
+      let startPollingFn: any;
+      let stopPollingFn: any;
+      let subscribeToMoreFn: any;
+      const Component = () => {
+        const {
+          loading,
+          refetch,
+          fetchMore,
+          updateQuery,
+          startPolling,
+          stopPolling,
+          subscribeToMore
+        } = useQuery(CAR_QUERY);
+        if (loading) {
+          refetchFn = refetch;
+          fetchMoreFn = fetchMore;
+          updateQueryFn = updateQuery;
+          startPollingFn = startPolling;
+          stopPollingFn = stopPolling;
+          subscribeToMoreFn = subscribeToMore;
+        } else {
+          expect(refetch).toBe(refetchFn);
+          expect(fetchMore).toBe(fetchMoreFn);
+          expect(updateQuery).toBe(updateQueryFn);
+          expect(startPolling).toBe(startPollingFn);
+          expect(stopPolling).toBe(stopPollingFn);
+          expect(subscribeToMore).toBe(subscribeToMoreFn);
+        }
+        return null;
+      };
+
+      render(
+        <MockedProvider mocks={CAR_MOCKS}>
+          <Component />
+        </MockedProvider>
+      );
+
+      await wait();
+    });
   });
 
   describe('Polling', () => {

diff --git a/packages/hooks/src/data/QueryData.ts b/packages/hooks/src/data/QueryData.ts
@@ -2,7 +2,11 @@ import {
   ApolloQueryResult,
   ObservableQuery,
   ApolloError,
-  NetworkStatus
+  NetworkStatus,
+  FetchMoreOptions,
+  FetchMoreQueryOptions,
+  UpdateQueryOptions,
+  SubscribeToMoreOptions
 } from 'apollo-client';
 import { equal as isEqual } from '@wry/equality';
 import {
@@ -201,20 +205,6 @@ export class QueryData<TData, TVariables> extends OperationData {
     };
   }
 
-  private observableQueryFields(
-    observable: ObservableQuery<TData, TVariables>
-  ): ObservableQueryFields<TData, TVariables> {
-    return {
-      variables: observable.variables,
-      refetch: observable.refetch.bind(observable),
-      fetchMore: observable.fetchMore.bind(observable),
-      updateQuery: observable.updateQuery.bind(observable),
-      startPolling: observable.startPolling.bind(observable),
-      stopPolling: observable.stopPolling.bind(observable),
-      subscribeToMore: observable.subscribeToMore.bind(observable)
-    } as ObservableQueryFields<TData, TVariables>;
-  }
-
   private initializeObservableQuery() {
     // See if there is an existing observable that was used to fetch the same
     // data and if so, use it instead since it will contain the proper queryId
@@ -314,9 +304,7 @@ export class QueryData<TData, TVariables> extends OperationData {
   }
 
   private getQueryResult(): QueryResult<TData, TVariables> {
-    let result: any = {
-      ...this.observableQueryFields(this.currentObservable.query!)
-    };
+    let result: any = this.observableQueryFields();
 
     // When skipping a query (ie. we're not querying for data but still want
     // to render children), make sure the `data` is cleared out and
@@ -434,4 +422,48 @@ export class QueryData<TData, TVariables> extends OperationData {
       delete this.currentObservable.subscription;
     }
   }
+
+  private obsRefetch = (variables?: TVariables) =>
+    this.currentObservable.query!.refetch(variables);
+
+  private obsFetchMore = <K extends keyof TVariables>(
+    fetchMoreOptions: FetchMoreQueryOptions<TVariables, K> &
+      FetchMoreOptions<TData, TVariables>
+  ) => this.currentObservable.query!.fetchMore(fetchMoreOptions);
+
+  private obsUpdateQuery = <TVars = TVariables>(
+    mapFn: (
+      previousQueryResult: TData,
+      options: UpdateQueryOptions<TVars>
+    ) => TData
+  ) => this.currentObservable.query!.updateQuery(mapFn);
+
+  private obsStartPolling = (pollInterval: number) =>
+    this.currentObservable.query!.startPolling(pollInterval);
+
+  private obsStopPolling = () => this.currentObservable.query!.stopPolling();
+
+  private obsSubscribeToMore = <
+    TSubscriptionData = TData,
+    TSubscriptionVariables = TVariables
+  >(
+    options: SubscribeToMoreOptions<
+      TData,
+      TSubscriptionVariables,
+      TSubscriptionData
+    >
+  ) => this.currentObservable.query!.subscribeToMore(options);
+
+  private observableQueryFields() {
+    const observable = this.currentObservable.query!;
+    return {
+      variables: observable.variables,
+      refetch: this.obsRefetch,
+      fetchMore: this.obsFetchMore,
+      updateQuery: this.obsUpdateQuery,
+      startPolling: this.obsStartPolling,
+      stopPolling: this.obsStopPolling,
+      subscribeToMore: this.obsSubscribeToMore
+    } as ObservableQueryFields<TData, TVariables>;
+  }
 }