Merge pull request #7997 from apollographql/reinstate-pr-6891

Check structural equality in QueryInfo#setDiff, again.
apollographql · May 12, 2021 · ea3a05c · ea3a05c
2 parents 1d57263 + e3ec93d
commit ea3a05c
Show file tree

Hide file tree

Showing 3 changed files with 89 additions and 7 deletions.
diff --git a/src/core/QueryInfo.ts b/src/core/QueryInfo.ts
@@ -197,7 +197,8 @@ export class QueryInfo {
     const oldDiff = this.lastDiff && this.lastDiff.diff;
     this.updateLastDiff(diff);
     if (!this.dirty &&
-        (diff && diff.result) !== (oldDiff && oldDiff.result)) {
+        !equal(oldDiff && oldDiff.result,
+               diff && diff.result)) {
       this.dirty = true;
       if (!this.notifyTimeout) {
         this.notifyTimeout = setTimeout(() => this.notify(), 0);

diff --git a/src/core/__tests__/ObservableQuery.ts b/src/core/__tests__/ObservableQuery.ts
@@ -1,5 +1,6 @@
 import gql from 'graphql-tag';
 import { GraphQLError } from 'graphql';
+import { TypedDocumentNode } from '@graphql-typed-document-node/core';
 
 import { ApolloClient, NetworkStatus } from '../../core';
 import { ObservableQuery } from '../ObservableQuery';
@@ -39,7 +40,11 @@ export const mockFetchQuery = (queryManager: QueryManager<any>) => {
 
 describe('ObservableQuery', () => {
   // Standard data for all these tests
-  const query = gql`
+  const query: TypedDocumentNode<{
+    people_one: {
+      name: string;
+    };
+  }> = gql`
     query query($id: ID!) {
       people_one(id: $id) {
         name
@@ -1918,6 +1923,79 @@ describe('ObservableQuery', () => {
     });
   });
 
+  itAsync("QueryInfo does not notify for !== but deep-equal results", (resolve, reject) => {
+    const queryManager = mockQueryManager(reject, {
+      request: { query, variables },
+      result: { data: dataOne },
+    });
+
+    const observable = queryManager.watchQuery({
+      query,
+      variables,
+      // If we let the cache return canonical results, it will be harder to
+      // write this test, because any two results that are deeply equal will
+      // also be !==, making the choice of equality test in queryInfo.setDiff
+      // less visible/important.
+      canonizeResults: false,
+    });
+
+    const queryInfo = observable["queryInfo"];
+    const cache = queryInfo["cache"];
+    const setDiffSpy = jest.spyOn(queryInfo, "setDiff");
+    const notifySpy = jest.spyOn(queryInfo, "notify");
+
+    subscribeAndCount(reject, observable, (count, result) => {
+      if (count === 1) {
+        expect(result).toEqual({
+          loading: false,
+          networkStatus: NetworkStatus.ready,
+          data: dataOne,
+        });
+
+        let invalidateCount = 0;
+        let onDirtyCount = 0;
+
+        cache.batch({
+          optimistic: true,
+          transaction(cache) {
+            cache.modify({
+              fields: {
+                people_one(value, { INVALIDATE }) {
+                  expect(value).toEqual(dataOne.people_one);
+                  ++invalidateCount;
+                  return INVALIDATE;
+                },
+              },
+            });
+          },
+          // Verify that the cache.modify operation did trigger a cache broadcast.
+          onDirty(watch, diff) {
+            expect(watch.watcher).toBe(queryInfo);
+            expect(diff).toEqual({
+              complete: true,
+              result: {
+                people_one: {
+                  name: "Luke Skywalker",
+                },
+              },
+            });
+            ++onDirtyCount;
+          },
+        });
+
+        new Promise(resolve => setTimeout(resolve, 100)).then(() => {
+          expect(setDiffSpy).toHaveBeenCalledTimes(1);
+          expect(notifySpy).not.toHaveBeenCalled();
+          expect(invalidateCount).toBe(1);
+          expect(onDirtyCount).toBe(1);
+          queryManager.stop();
+        }).then(resolve, reject);
+      } else {
+        reject("too many results");
+      }
+    });
+  });
+
   itAsync("ObservableQuery#map respects Symbol.species", (resolve, reject) => {
     const observable = mockWatchQuery(reject, {
       request: { query, variables },

diff --git a/src/utilities/testing/subscribeAndCount.ts b/src/utilities/testing/subscribeAndCount.ts
@@ -1,20 +1,23 @@
 import { ObservableQuery } from '../../core/ObservableQuery';
-import { ApolloQueryResult } from '../../core/types';
+import { ApolloQueryResult, OperationVariables } from '../../core/types';
 import { ObservableSubscription } from '../../utilities/observables/Observable';
 import { asyncMap } from '../../utilities/observables/asyncMap';
 
-export default function subscribeAndCount(
+export default function subscribeAndCount<
+  TData,
+  TVariables = OperationVariables,
+>(
   reject: (reason: any) => any,
-  observable: ObservableQuery<any>,
-  cb: (handleCount: number, result: ApolloQueryResult<any>) => any,
+  observable: ObservableQuery<TData, TVariables>,
+  cb: (handleCount: number, result: ApolloQueryResult<TData>) => any,
 ): ObservableSubscription {
   // Use a Promise queue to prevent callbacks from being run out of order.
   let queue = Promise.resolve();
   let handleCount = 0;
 
   const subscription = asyncMap(
     observable,
-    (result: ApolloQueryResult<any>) => {
+    (result: ApolloQueryResult<TData>) => {
       // All previous asynchronous callbacks must complete before cb can
       // be invoked with this result.
       return queue = queue.then(() => {