From e66027c5341dc7aaf71ee7ffcba1305b9a553525 Mon Sep 17 00:00:00 2001
From: Ben Newman <ben@apollographql.com>
Date: Sat, 20 Oct 2018 13:19:30 -0400
Subject: [PATCH] Snapshot observableQuery.lastResult to defend against
 mutation.

Should help with #3992.

Though deep cloning and isEqual testing are obviously more expensive than
relying on === equality, the reality is that lastResult may have been
modified since it was previously recorded, so === equality is useless for
determining isDifferentResult. A defensive copy is the only way to make
sure we get this right.
---
 packages/apollo-client/src/core/ObservableQuery.ts       | 9 ++++-----
 packages/apollo-client/src/core/QueryManager.ts          | 6 ++----
 .../apollo-client/src/core/__tests__/ObservableQuery.ts  | 3 +--
 3 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/packages/apollo-client/src/core/ObservableQuery.ts b/packages/apollo-client/src/core/ObservableQuery.ts
index d515f89d431..e89a8f08acd 100644
--- a/packages/apollo-client/src/core/ObservableQuery.ts
+++ b/packages/apollo-client/src/core/ObservableQuery.ts
@@ -1,4 +1,4 @@
-import { isEqual, tryFunctionOrLogError } from 'apollo-utilities';
+import { isEqual, tryFunctionOrLogError, cloneDeep } from 'apollo-utilities';
 import { GraphQLError } from 'graphql';
 import { NetworkStatus, isNetworkRequestInFlight } from './networkStatus';
 import { Observable, Observer, Subscription } from '../util/Observable';
@@ -201,7 +201,7 @@ export class ObservableQuery<
     }
 
     const result = {
-      data,
+      data: cloneDeep(data),
       loading: isNetworkRequestInFlight(networkStatus),
       networkStatus,
     } as ApolloQueryResult<TData>;
@@ -215,8 +215,7 @@ export class ObservableQuery<
     }
 
     if (!partial) {
-      const stale = false;
-      this.lastResult = { ...result, stale };
+      this.lastResult = { ...result, stale: false };
     }
 
     return { ...result, partial } as ApolloCurrentResult<TData>;
@@ -586,7 +585,7 @@ export class ObservableQuery<
 
     const observer: Observer<ApolloQueryResult<TData>> = {
       next: (result: ApolloQueryResult<TData>) => {
-        this.lastResult = result;
+        this.lastResult = cloneDeep(result);
         this.observers.forEach(obs => obs.next && obs.next(result));
       },
       error: (error: ApolloError) => {
diff --git a/packages/apollo-client/src/core/QueryManager.ts b/packages/apollo-client/src/core/QueryManager.ts
index 00657d19bad..d14652c5772 100644
--- a/packages/apollo-client/src/core/QueryManager.ts
+++ b/packages/apollo-client/src/core/QueryManager.ts
@@ -12,6 +12,7 @@ import {
   getQueryDefinition,
   isProduction,
   hasDirectives,
+  isEqual,
 } from 'apollo-utilities';
 
 import { QueryScheduler } from '../scheduler/scheduler';
@@ -609,10 +610,7 @@ export class QueryManager<TStore> {
               resultFromStore &&
               lastResult.networkStatus === resultFromStore.networkStatus &&
               lastResult.stale === resultFromStore.stale &&
-              // We can do a strict equality check here because we include a `previousResult`
-              // with `readQueryFromStore`. So if the results are the same they will be
-              // referentially equal.
-              lastResult.data === resultFromStore.data
+              isEqual(lastResult.data, resultFromStore.data)
             );
 
             if (isDifferentResult || previouslyHadError) {
diff --git a/packages/apollo-client/src/core/__tests__/ObservableQuery.ts b/packages/apollo-client/src/core/__tests__/ObservableQuery.ts
index 2456913c958..2d9204cecba 100644
--- a/packages/apollo-client/src/core/__tests__/ObservableQuery.ts
+++ b/packages/apollo-client/src/core/__tests__/ObservableQuery.ts
@@ -324,8 +324,7 @@ describe('ObservableQuery', () => {
           const current = observable.currentResult();
           expect(stripSymbols(current.data)).toEqual(data);
           const secondCurrent = observable.currentResult();
-          // ensure ref equality
-          expect(current.data).toBe(secondCurrent.data);
+          expect(current.data).toEqual(secondCurrent.data);
           done();
         }
       });