Revert "Avoid network after incomplete optimistic cache results. (#6419…

…)" (#6493) This reverts commit 9450938. The logic of diff.optimistic needs rethinking. Any query could read from the cache at a time when optimistic updates are in progress, and that possibility should not affect the network behavior of the query. I had hoped that the dependency tracking system would serve to protect queries whose field dependencies were unrelated to the optimistic updates, but I have now observed at least one case where this hope was badly mistaken, while debugging an endless loading spinner issue with the latest versions of @apollo/client (rc3-rc.8) in the studio.apollographql.com application. As I mentioned in #6419, I was not able to reproduce the original scenario using an ordinary optimistic mutation, so I think we should wait until someone can provide a more realistic reproduction of the problem: #6419 (comment) I have left the test in place, disabled with itAsync.skip, so that we can revisit this functionality in the future.
apollographql · Jun 25, 2020 · 0530ee0 · 0530ee0
1 parent dcd897d
commit 0530ee0
Show file tree

Hide file tree

Showing 8 changed files with 3 additions and 39 deletions.
diff --git a/src/cache/core/types/DataProxy.ts b/src/cache/core/types/DataProxy.ts
@@ -81,7 +81,6 @@ export namespace DataProxy {
     result?: T;
     complete?: boolean;
     missing?: MissingFieldError[];
-    optimistic?: boolean;
   }
 }
 

diff --git a/src/cache/inmemory/__tests__/cache.ts b/src/cache/inmemory/__tests__/cache.ts
@@ -1460,7 +1460,6 @@ describe("InMemoryCache#broadcastWatches", function () {
         },
       },
       complete: true,
-      optimistic: false,
     }];
 
     expect(receivedCallbackResults).toEqual([
@@ -1482,7 +1481,6 @@ describe("InMemoryCache#broadcastWatches", function () {
         },
       },
       complete: true,
-      optimistic: false,
     }];
 
     expect(receivedCallbackResults).toEqual([
@@ -1504,7 +1502,6 @@ describe("InMemoryCache#broadcastWatches", function () {
         },
       },
       complete: true,
-      optimistic: false,
     }];
 
     const received4 = [id4, 1, {
@@ -1514,7 +1511,6 @@ describe("InMemoryCache#broadcastWatches", function () {
         },
       },
       complete: true,
-      optimistic: false,
     }];
 
     expect(receivedCallbackResults).toEqual([
@@ -1535,7 +1531,6 @@ describe("InMemoryCache#broadcastWatches", function () {
         },
       },
       complete: true,
-      optimistic: false,
     }];
 
     expect(receivedCallbackResults).toEqual([
@@ -2113,12 +2108,10 @@ describe("InMemoryCache#modify", () => {
     function makeResult(
       __typename: string,
       value: number,
-      complete = true,
-      optimistic = false,
+      complete: boolean = true,
     ) {
       return {
         complete,
-        optimistic,
         result: {
           [__typename.toLowerCase()]: {
             __typename,

diff --git a/src/cache/inmemory/__tests__/entityStore.ts b/src/cache/inmemory/__tests__/entityStore.ts
@@ -1084,7 +1084,6 @@ describe('EntityStore', () => {
       },
     })).toEqual({
       complete: false,
-      optimistic: false,
       result: {
         authorOfBook: tedWithoutHobby,
       },
@@ -1687,7 +1686,6 @@ describe('EntityStore', () => {
     })).toEqual({
       complete: false,
       missing,
-      optimistic: true,
       result: {
         book: {
           __typename: "Book",
@@ -1719,7 +1717,6 @@ describe('EntityStore', () => {
     })).toEqual({
       complete: false,
       missing,
-      optimistic: false,
       result: {
         book: {
           __typename: "Book",
@@ -1736,7 +1733,6 @@ describe('EntityStore', () => {
       returnPartialData: true,
     })).toEqual({
       complete: true,
-      optimistic: false,
       result: {
         book: {
           __typename: "Book",
@@ -1937,7 +1933,6 @@ describe('EntityStore', () => {
 
     expect(cuckoosCallingDiffResult).toEqual({
       complete: false,
-      optimistic: false,
       result: {
         book: {
           __typename: "Book",

diff --git a/src/cache/inmemory/__tests__/policies.ts b/src/cache/inmemory/__tests__/policies.ts
@@ -1056,7 +1056,6 @@ describe("type policies", function () {
           }],
         },
         complete: false,
-        optimistic: false,
         missing: [
           makeMissingError(1),
           makeMissingError(2),
@@ -1108,7 +1107,6 @@ describe("type policies", function () {
           }],
         },
         complete: false,
-        optimistic: false,
         missing: [
           makeMissingError(1),
           makeMissingError(3),
@@ -1166,7 +1164,6 @@ describe("type policies", function () {
           }],
         },
         complete: false,
-        optimistic: false,
         missing: [
           makeMissingError(1),
           makeMissingError(3),
@@ -1201,7 +1198,6 @@ describe("type policies", function () {
           }],
         },
         complete: true,
-        optimistic: false,
       });
 
       expect(cache.readQuery({ query })).toEqual({

diff --git a/src/cache/inmemory/__tests__/readFromStore.ts b/src/cache/inmemory/__tests__/readFromStore.ts
@@ -1033,7 +1033,6 @@ describe('reading from the store', () => {
 
     expect(diffChickens()).toEqual({
       complete: true,
-      optimistic: false,
       result: {
         chickens: [
           { __typename: "Chicken", id: 1, inCoop: true },
@@ -1052,7 +1051,6 @@ describe('reading from the store', () => {
 
     expect(diffChickens()).toEqual({
       complete: false,
-      optimistic: false,
       missing: [
         expect.anything(),
         expect.anything(),
@@ -1075,7 +1073,6 @@ describe('reading from the store', () => {
 
     expect(diffDucks()).toEqual({
       complete: true,
-      optimistic: false,
       result: {
         ducks: [
           { __typename: "Duck", id: 1, quacking: true },
@@ -1097,7 +1094,6 @@ describe('reading from the store', () => {
     // diff, and without altering the positions of later elements.
     expect(diffDucks()).toEqual({
       complete: true,
-      optimistic: false,
       result: {
         ducks: [
           { __typename: "Duck", id: 1, quacking: true },
@@ -1116,7 +1112,6 @@ describe('reading from the store', () => {
 
     expect(diffOxen()).toEqual({
       complete: true,
-      optimistic: false,
       result: {
         oxen: [
           { __typename: "Ox", id: 1, gee: true, haw: false },
@@ -1134,7 +1129,6 @@ describe('reading from the store', () => {
 
     expect(diffOxen()).toEqual({
       complete: true,
-      optimistic: false,
       result: {
         oxen: [
           { __typename: "Ox", id: 2, gee: false, haw: true },
@@ -1244,7 +1238,6 @@ describe('reading from the store', () => {
         },
       },
       complete: true,
-      optimistic: false,
     };
 
     // We already have one diff because of the immediate:true above.
@@ -1268,7 +1261,6 @@ describe('reading from the store', () => {
         },
       },
       complete: true,
-      optimistic: false,
     };
 
     expect(diffs).toEqual([
@@ -1296,7 +1288,6 @@ describe('reading from the store', () => {
         },
       },
       complete: true,
-      optimistic: false,
     };
 
     expect(diffs).toEqual([
@@ -1338,7 +1329,6 @@ describe('reading from the store', () => {
 
     const diffWithChildrenOfZeus = {
       complete: true,
-      optimistic: false,
       result: {
         ...diffWithoutDevouredSons.result,
         ruler: {
@@ -1375,7 +1365,6 @@ describe('reading from the store', () => {
 
     const diffWithZeusAsRuler = {
       complete: true,
-      optimistic: false,
       result: {
         ruler: {
           __typename: "Deity",
@@ -1554,7 +1543,6 @@ describe('reading from the store', () => {
 
     const diffWithApolloAsRuler = {
       complete: true,
-      optimistic: false,
       result: apolloRulerResult,
     };
 

diff --git a/src/cache/inmemory/readFromStore.ts b/src/cache/inmemory/readFromStore.ts
@@ -35,7 +35,7 @@ import {
   NormalizedCache,
   ReadMergeModifyContext,
 } from './types';
-import { supportsResultCaching, EntityStore } from './entityStore';
+import { supportsResultCaching } from './entityStore';
 import { getTypenameFromStoreObject } from './helpers';
 import { Policies } from './policies';
 import { InMemoryCache } from './inMemoryCache';
@@ -154,7 +154,6 @@ export class StoreReader {
       result: execResult.result,
       missing: execResult.missing,
       complete: !hasMissingFields,
-      optimistic: !(store instanceof EntityStore.Root),
     };
   }
 

diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts
@@ -1011,12 +1011,6 @@ export class QueryManager<TStore> {
         ];
       }
 
-      if (diff.optimistic) {
-        return returnPartialData ? [
-          resultsFromCache(diff, queryInfo.markReady()),
-        ] : [];
-      }
-
       if (returnPartialData) {
         return [
           resultsFromCache(diff),

diff --git a/src/core/__tests__/fetchPolicies.ts b/src/core/__tests__/fetchPolicies.ts
@@ -325,7 +325,7 @@ describe('no-cache', () => {
 });
 
 describe('cache-first', () => {
-  itAsync('does not trigger network request during optimistic update', (resolve, reject) => {
+  itAsync.skip('does not trigger network request during optimistic update', (resolve, reject) => {
     const results: any[] = [];
     const client = new ApolloClient({
       link: new ApolloLink((operation, forward) => {