Repository: apollographql/apollo-client

Problem statement:
[QueryManager] getCurrentQueryResult behavior appears inconsistent
From what I can tell the return value of `getCurrentQueryResult` is inconsistent. 

This is a bit tricky to explain, so I've annotated the method below in a vein attempt to convey what I believe I'm observing.

```javascript
  // Copied from QueryManager commit d185f41.
  // https://github.com/apollographql/apollo-client/blob/d185f41fb13eeaae38aa66582d3b2ea1ebf48d43/packages/apollo-client/src/core/QueryManager.ts#L991-L1016
  public getCurrentQueryResult<T>(
    observableQuery: ObservableQuery<T>,
    optimistic: boolean = true,
  ) {
    const { variables, query } = observableQuery.options;
    const lastResult = observableQuery.getLastResult();

    // newData is non null when either:
    //   a) a query watcher emits a new diff, see line 775
    //   b) the fetch has completed, see line 1137
    const { newData } = this.getQuery(observableQuery.queryId);
    
    // This is where I think this starts to become inconsistent with my expectations.
    // Unlike the condition below, by virtue of the watcher, the result in newData may
    // be incomplete if a query or mutation selecting similar fields completes first.
    if (newData) {

      // It's not 100% clear to me the intention of the partial field, but it led to some
      // confusion when I received incomplete results and the value was false.
      return { data: newData.result, partial: false };
    } else {
      try {
        // The read method appears to throw when fields the query is trying to select are
        // unavailable, meaning that in this condition the return value will never be incomplete.
        const data = this.dataStore.getCache().read({
          query,
          variables,
          previousResult: lastResult ? lastResult.data : undefined,
          optimistic,
        });

        return { data, partial: false };
      } catch (e) {
        return { data: {}, partial: true };
      }
    }
  }
```

This combination of the method in some cases returning complete results and in other rare cases returning incomplete results has led to some strange behavior. Consider, when the query is paired with a listener and a cache policy that is not "no-cache" you can observe:

1. watcher emits a new but incomplete result from the cache.
1. application calls `getCurrentQueryResult` and receives incomplete results.
1. next tick query listener fires and [nullifies](https://github.com/apollographql/apollo-client/blob/d185f41fb13eeaae38aa66582d3b2ea1ebf48d43/packages/apollo-client/src/core/QueryManager.ts#L774-L776) the `newData` value.
1. application calls `getCurrentQueryResult` again, this time the method fetches from the cache and throws. Empty result set is received by application.

If you have multiple queries on the page/view/activity the end-user could see content flicker in and out of existence until all fetches have completed. This is assuming it didn't already crash when the view attempted to access an incomplete result set.

**Intended outcome:**

I think ideally `getCurrentQueryResult` would be consistent by returning only complete or incomplete results, but not mixture of the two.

**Actual outcome:**

Depending on the state of the query, the method can return either complete or incomplete result sets.

**How to reproduce the issue:**

For a slightly more concrete example imagine the following (albeit contrived) scenario:

```javascript
const queryA = gql`
query A {
  user {
    name
  }
}
`;
const queryB = gql`
query B {
  # NOTE: using the same root query field.
  user {
    dogs
  }
`;

<Query query={queryA} cachePolicy="cache-and-network">
  <p>{resultA.user && resultA.user.name}</p>
  <Query query={queryB} cachePolicy="cache-and-network">
    {loading && <span>Loading...</span>}
    <p>{resultB.user && resultB.user.dogs.join(", ")}</p>
  </Query>
</Query>
```

Given:

- queryA takes 100ms to complete
- queryB takes 500ms to complete

Timeline:

1. components are mounted and fetches start.
1. `queryA` completes.
1. (From what I've observed) the watcher for `queryB` fires as there are new results associated with the root `user` field. queryB's `newData` field is set.
1. the first query container rerenders.
1. as updates cascade down the second query container rerenders.
1. the second query container's render method asks for the current query result receiving the incomplete result set that was retrieved by the watcher.
1. (From what I've observed) the query listener for `queryB` fires causing the second query container to rerender again, this time when it asks for the current query result it receives an empty set.
1. Eventually `queryB` completes and the query container rerenders with the complete set of results.

Concern:

Currently there is no way to determine if I've received an incomplete result set or not, this means that as a consumer, I would need a lot of (seemingly unnecessary) conditionals in my components to ensure that I am not accessing fields that have not yet been fulfilled.

**Versions:**

- `apollo-client` `=2.4.2`


Hints:
I hope what I've provided is clear; happy to provide more details or help patch.

Patch:
diff --git a/packages/apollo-client/src/core/QueryManager.ts b/packages/apollo-client/src/core/QueryManager.ts
--- a/packages/apollo-client/src/core/QueryManager.ts
+++ b/packages/apollo-client/src/core/QueryManager.ts
@@ -996,7 +996,7 @@ export class QueryManager<TStore> {
     const lastResult = observableQuery.getLastResult();
     const { newData } = this.getQuery(observableQuery.queryId);
     // XXX test this
-    if (newData) {
+    if (newData && newData.complete) {
       return { data: newData.result, partial: false };
     } else {
       try {


Test patch:
diff --git a/packages/apollo-client/src/core/__tests__/QueryManager/index.ts b/packages/apollo-client/src/core/__tests__/QueryManager/index.ts
--- a/packages/apollo-client/src/core/__tests__/QueryManager/index.ts
+++ b/packages/apollo-client/src/core/__tests__/QueryManager/index.ts
@@ -2336,6 +2336,87 @@ describe('QueryManager', () => {
     // We have an unhandled error warning from the `subscribe` above, which has no `error` cb
   });
 
+  it('does not return incomplete data when two queries for the same item are executed', () => {
+    const queryA = gql`
+      query queryA {
+        person(id: "abc") {
+          __typename
+          id
+          firstName
+          lastName
+        }
+      }
+    `;
+    const queryB = gql`
+      query queryB {
+        person(id: "abc") {
+          __typename
+          id
+          lastName
+          age
+        }
+      }
+    `;
+    const dataA = {
+      person: {
+        __typename: 'Person',
+        id: 'abc',
+        firstName: 'Luke',
+        lastName: 'Skywalker',
+      },
+    };
+    const dataB = {
+      person: {
+        __typename: 'Person',
+        id: 'abc',
+        lastName: 'Skywalker',
+        age: '32',
+      },
+    };
+    const queryManager = new QueryManager<NormalizedCacheObject>({
+      link: mockSingleLink(
+        { request: { query: queryA }, result: { data: dataA } },
+        { request: { query: queryB }, result: { data: dataB }, delay: 20 },
+      ),
+      store: new DataStore(new InMemoryCache({})),
+      ssrMode: true,
+    });
+
+    const observableA = queryManager.watchQuery({
+      query: queryA,
+    });
+    const observableB = queryManager.watchQuery({
+      query: queryB,
+    });
+
+    return Promise.all([
+      observableToPromise({ observable: observableA }, () => {
+        expect(
+          stripSymbols(queryManager.getCurrentQueryResult(observableA)),
+        ).toEqual({
+          data: dataA,
+          partial: false,
+        });
+        expect(queryManager.getCurrentQueryResult(observableB)).toEqual({
+          data: {},
+          partial: true,
+        });
+      }),
+      observableToPromise({ observable: observableB }, () => {
+        expect(
+          stripSymbols(queryManager.getCurrentQueryResult(observableA)),
+        ).toEqual({
+          data: dataA,
+          partial: false,
+        });
+        expect(queryManager.getCurrentQueryResult(observableB)).toEqual({
+          data: dataB,
+          partial: false,
+        });
+      }),
+    ]);
+  });
+
   describe('polling queries', () => {
     it('allows you to poll queries', () => {
       const query = gql`