Apollo-client: updateQuery can cause error when using dataIdFromObject

[NeoPhi]

I've noticed an issue that if you have two queries with non overlapping fields on a descendant field that is mapped via dataIdFromObject, if an updateQuery changes the linked to object, the system does not correctly detect that it now has missing data and needs to refetch the query. This manifests itself as a "Perhaps you want to use the returnPartialData option?" error.

This standalone test demonstrates the behavior:

import * as chai from 'chai';
const { assert } = chai;
import * as sinon from 'sinon';
import gql from 'graphql-tag';
import mockQueryManager from './mocks/mockQueryManager';
import mockWatchQuery from './mocks/mockWatchQuery';
import { ObservableQuery } from '../src/core/ObservableQuery';
import { QueryManager } from '../src/core/QueryManager';
import mockNetworkInterface, { MockedResponse } from './mocks/mockNetworkInterface';
import { createApolloStore, ApolloReducerConfig } from '../src/store';
import subscribeAndCount from './util/subscribeAndCount';

const defaultReduxRootSelector = (state: any) => state.apollo;

const friendAgeQuery = gql`
  query($id: ID!) {
    person(id: $id) {
      id
      friend {
        id
        name
        age
      }
    }
  }
`;

const friendAgeData1 = {
  person: {
    id: 1,
    friend: {
      id: 2,
      name: 'Bob',
      age: 12
    }
  }
};

const friendAgeData2 = {
  person: {
    id: 1,
    friend: {
      id: 3,
      name: 'Chris',
      age: 13
    }
  }
};

const friendQuery = gql`
  query($id: ID!) {
    person(id: $id) {
      id
      friend {
        id
        name
      }
    }
  }
`;

const friendData = {
  person: {
    id: 1,
    friend: {
      id: 2,
      name: 'Bob',
    }
  }
};

describe('ObservableQuery', () => {
  describe('updateQuery', () => {
    it.only('handles shared object changes', (done) => {
      const mockedResponses = [{
        request: { query: friendAgeQuery, variables: { id: 1 } },
        result: { data: friendAgeData1 },
      }, {
        request: { query: friendQuery, variables: { id: 1 } },
        result: { data: friendData },
      }, {
        request: { query: friendAgeQuery, variables: { id: 1 } },
        result: { data: friendAgeData2 },        
      }];
      const reducerConfig: ApolloReducerConfig = {
        mutationBehaviorReducers: {},
        dataIdFromObject: (result: any) => {
          if (result.id) {
            return result.id;
          }
          return null;
        },
      };
      const store = createApolloStore({ config: reducerConfig });
      const queryManager = new QueryManager({
        networkInterface: mockNetworkInterface(...mockedResponses),
        store,
        reduxRootSelector: defaultReduxRootSelector,
        addTypename: false,
        reducerConfig,
      });
      const friendAge = queryManager.watchQuery({
        query: mockedResponses[0].request.query,
        variables: mockedResponses[0].request.variables,
      });
      const friend = queryManager.watchQuery({
        query: mockedResponses[1].request.query,
        variables: mockedResponses[1].request.variables,
      });
      subscribeAndCount(done, friendAge, (count, result) => {
        if (count === 1) {
          subscribeAndCount(done, friend, (count, result) => {
            if (count === 1) {
              friend.updateQuery((previous) => {
                return {
                  person: {
                    id: 1,
                    friend: {
                      id: 3, 
                      name: 'Chris'
                    }
                  }
                };
              });              
            }
          });          
        }
        if (count === 2) {
          assert.isTrue(result.loading);
        }
        if (count === 3) {
          assert.deepEqual(result.data, friendAgeData2);
          done();
        }
      });
    });
  });
});

[helfer]

@NeoPhi Thanks for the test! I think this is something we were aware of, but never addressed. I don't think there's a perfect solution, but we could make things somewhat better by merging objects when we write them into the store. That might render the object inconsistent, but at least it would prevent the error that you are seeing. In the future we might want to have the ability to detect inconsistent cache objects and trigger a refetch.

[helfer]

@NeoPhi I think this is fixed now (with stale data). Can you confirm?

[NeoPhi]

A simple test indicates that the new stale flag is handling this case correctly. As #821 was a more general version of this issue I think we are covered.