Merge pull request #5398 from apollographql/react-updates

React updates (from React Apollo 3.1.2)

src/react/data/OperationData.ts

@@ -26,8 +26,11 @@ export abstract class OperationData<TOptions = any> {
     return this.options;
   }
 
-  public setOptions(newOptions: CommonOptions<TOptions>) {
-    if (!isEqual(this.options, newOptions)) {
+  public setOptions(
+    newOptions: CommonOptions<TOptions>,
+    storePrevious: boolean = false
+  ) {
+    if (storePrevious && !isEqual(this.options, newOptions)) {
       this.previousOptions = this.options;
     }
     this.options = newOptions;

src/react/data/QueryData.ts

@@ -48,7 +48,7 @@ export class QueryData<TData, TVariables> extends OperationData {
   public execute(): QueryResult<TData, TVariables> {
     this.refreshClient();
 
-    const { skip, query, ssr } = this.getOptions();
+    const { skip, query } = this.getOptions();
     if (skip || query !== this.previousData.query) {
       this.removeQuerySubscription();
       this.previousData.query = query;
@@ -58,9 +58,7 @@ export class QueryData<TData, TVariables> extends OperationData {
 
     if (this.isMounted) this.startQuerySubscription();
 
-    const ssrDisabled = ssr === false;
-
-    return this.getExecuteSsrResult(ssrDisabled) || this.getExecuteResult();
+    return this.getExecuteSsrResult() || this.getExecuteResult();
   }
 
   public executeLazy(): QueryTuple<TData, TVariables> {
@@ -90,6 +88,7 @@ export class QueryData<TData, TVariables> extends OperationData {
 
   public afterExecute({ lazy = false }: { lazy?: boolean } = {}) {
     this.isMounted = true;
+
     if (!lazy || this.runLazy) {
       this.handleErrorOrCompleted();
 
@@ -103,6 +102,7 @@ export class QueryData<TData, TVariables> extends OperationData {
       });
     }
 
+    this.previousOptions = this.getOptions();
     return this.unmount.bind(this);
   }
 
@@ -114,25 +114,24 @@ export class QueryData<TData, TVariables> extends OperationData {
 
   public getOptions() {
     const options = super.getOptions();
-    const lazyOptions = this.lazyOptions || {};
-    const updatedOptions = {
-      ...options,
-      variables: {
+
+    if (this.lazyOptions) {
+      options.variables = {
         ...options.variables,
-        ...lazyOptions.variables
-      },
-      context: {
+        ...this.lazyOptions.variables
+      };
+      options.context = {
         ...options.context,
-        ...lazyOptions.context
-      }
-    };
+        ...this.lazyOptions.context
+      };
+    }
 
     // skip is not supported when using lazy query execution.
     if (this.runLazy) {
-      delete updatedOptions.skip;
+      delete options.skip;
     }
 
-    return updatedOptions;
+    return options;
   }
 
   private runLazyQuery = (options?: QueryLazyOptions<TVariables>) => {
@@ -149,29 +148,31 @@ export class QueryData<TData, TVariables> extends OperationData {
     return result;
   };
 
-  private getExecuteSsrResult(ssrDisabled: boolean) {
-    let result;
-
-    if (this.context && this.context.renderPromises) {
-      const ssrLoading = {
-        loading: true,
-        networkStatus: NetworkStatus.loading,
-        called: true,
-        data: undefined
-      } as QueryResult<TData, TVariables>;
-
-      // SSR is disabled, so just return the loading event and leave it in that state.
-      if (ssrDisabled) {
-        return ssrLoading;
-      }
+  private getExecuteSsrResult() {
+    const treeRenderingInitiated = this.context && this.context.renderPromises;
+    const ssrDisabled = this.getOptions().ssr === false;
+    const fetchDisabled = this.refreshClient().client.disableNetworkFetches;
+
+    const ssrLoading = {
+      loading: true,
+      networkStatus: NetworkStatus.loading,
+      called: true,
+      data: undefined
+    } as QueryResult<TData, TVariables>;
+
+    // If SSR has been explicitly disabled, and this function has been called
+    // on the server side, return the default loading state.
+    if (ssrDisabled && (treeRenderingInitiated || fetchDisabled)) {
+      return ssrLoading;
+    }
 
-      result = this.context.renderPromises.addQueryPromise(
-        this,
-        this.getExecuteResult
-      );
-      if (!result) {
-        result = ssrLoading as QueryResult<TData, TVariables>;
-      }
+    let result;
+    if (treeRenderingInitiated) {
+      result =
+        this.context.renderPromises!.addQueryPromise(
+          this,
+          this.getExecuteResult
+        ) || ssrLoading;
     }
 
     return result;
@@ -196,7 +197,7 @@ export class QueryData<TData, TVariables> extends OperationData {
     return {
       ...options,
       displayName,
-      context: options.context || {},
+      context: options.context,
       metadata: { reactComponent: { displayName } }
     };
   }
@@ -213,13 +214,14 @@ export class QueryData<TData, TVariables> extends OperationData {
 
     if (!this.currentObservable.query) {
       const observableQueryOptions = this.prepareObservableQueryOptions();
+
       this.previousData.observableQueryOptions = {
         ...observableQueryOptions,
         children: null
       };
-      this.currentObservable.query = this.refreshClient().client.watchQuery(
-        observableQueryOptions
-      );
+      this.currentObservable.query = this.refreshClient().client.watchQuery({
+        ...observableQueryOptions
+      });
 
       if (this.context && this.context.renderPromises) {
         this.context.renderPromises.registerSSRObservable(
@@ -267,30 +269,14 @@ export class QueryData<TData, TVariables> extends OperationData {
       next: ({ loading, networkStatus, data }) => {
         const previousResult = this.previousData.result;
 
-        if (previousResult) {
-          // Calls to `ObservableQuery.fetchMore` return a result before the
-          // `updateQuery` function fully finishes. This can lead to an
-          // extra un-necessary re-render since the initially returned data is
-          // the same as data that has already been rendered. We'll
-          // prevent the un-necessary render from happening, making sure
-          // `fetchMore` results are only rendered when `updateQuery` has
-          // completed.
-          if (
-            previousResult.loading &&
-            previousResult.networkStatus === NetworkStatus.fetchMore &&
-            isEqual(previousResult.data, data)
-          ) {
-            return;
-          }
-
-          // Make sure we're not attempting to re-render similar results
-          if (
-            previousResult.loading === loading &&
-            previousResult.networkStatus === networkStatus &&
-            isEqual(previousResult.data, data)
-          ) {
-            return;
-          }
+        // Make sure we're not attempting to re-render similar results
+        if (
+          previousResult &&
+          previousResult.loading === loading &&
+          previousResult.networkStatus === networkStatus &&
+          isEqual(previousResult.data, data)
+        ) {
+          return;
         }
 
         this.forceUpdate();

src/react/hooks/__tests__/useLazyQuery.test.tsx

@@ -417,7 +417,7 @@ describe('useLazyQuery Hook', () => {
           expect(loading).toEqual(false);
           expect(data).toEqual(CAR_RESULT_DATA);
           setTimeout(() => {
-            execute();
+            execute({ variables: { someProp: 'someValue' } });
           });
           break;
         case 3:

src/react/hooks/__tests__/useQuery.test.tsx

@@ -851,6 +851,11 @@ describe('useQuery Hook', () => {
               break;
             case 3:
               expect(loading).toBeFalsy();
+              expect(data).toEqual({
+                cars: [carResults.cars[0]]
+              });
+              break;
+            case 4:
               expect(data).toEqual({
                 cars: [carResults.cars[0], moreCarResults.cars[0]]
               });
@@ -869,7 +874,7 @@ describe('useQuery Hook', () => {
         );
 
         return wait(() => {
-          expect(renderCount).toBe(4);
+          expect(renderCount).toBe(5);
         });
       }
     );

src/react/hooks/useSubscription.ts

@@ -33,7 +33,7 @@ export function useSubscription<TData = any, TVariables = OperationVariables>(
   }
 
   const subscriptionData = getSubscriptionDataRef();
-  subscriptionData.setOptions(updatedOptions);
+  subscriptionData.setOptions(updatedOptions, true);
   subscriptionData.context = context;
 
   useEffect(() => subscriptionData.afterExecute());

src/react/testing/mocks/mockLink.ts

@@ -87,13 +87,15 @@ export class MockLink extends ApolloLink {
 
     this.mockedResponsesByKey[key].splice(responseIndex, 1);
 
-    const { result, error, delay, newData } = response;
+    const { newData } = response;
 
     if (newData) {
       response.result = newData();
       this.mockedResponsesByKey[key].push(response);
     }
 
+    const { result, error, delay } = response;
+
     if (!result && !error) {
       throw new Error(
         `Mocked response should contain either result or error: ${key}`