Apollo-client: Fragment matching broken when adding a new type that implements an existing interface

[rylanc]

Context:
Imagine a scenario where the client and server code are deployed separately.

Here's the server schema:

  type Query {
    allTheStuff: [Node]
  }

  interface Node {
    id: ID!
  }

  type Rocket implements Node {
    id: ID!
    name: String
  }

  type User implements Node {
    id: ID!
    email: String!
  }

We have a client query that looks like this:

  query {
    allTheStuff {
      ...TestFragment
    }
  }

  fragment TestFragment on Node {
    id
  }

The client has been bundled with the latest fragment types, which include Rocket and User as the possible types for the Node interface.

Now, we add a third implementing type:

  type Launch implements Node {
    id: ID!
    site: String
  }

Since our client and server are deployed separately, the client's fragment types are stale.

Intended outcome:
Existing and deployed client code has access to all the fields defined in TestFragment for all objects returned by the Query.allTheStuff field.
More generally: when fragment types are stale, we'd expect to see more cache-misses but not missing data.

Actual outcome:
Existing clients do not have access to the id fields for any objects of the Launch type.
Note that this problem does not exist when using the no-cache policy, which makes the experience of using Apollo Client Cache inferior to that of not using the cache.
More generally: When fragment types are stale, using the cache provides a broken experience with missing data.

How to reproduce the issue:
I created a fork of the fullstack tutorial that illustrates this issue. It uses apollo-client v3 (I tested and confirmed the issue in v2, as well) and configures the possibleTypes for Node to only include Rocket and User. The "launches" page query does not receive any data for the Launch object returned from Query.allTheStuff.

Versions

  System:
    OS: macOS Mojave 10.14.6
  Binaries:
    Node: 12.13.0 - ~/.nvm/versions/node/v12.13.0/bin/node
    Yarn: 1.17.3 - ~/workspace/oss/fullstack-tutorial/final/client/node_modules/.bin/yarn
    npm: 6.12.0 - ~/.nvm/versions/node/v12.13.0/bin/npm
  Browsers:
    Chrome: 79.0.3945.88
    Safari: 13.0.4
  npmPackages:
    @apollo/client: ^3.0.0-beta.19 => 3.0.0-beta.19
    @apollo/react-hooks: 3.0.0 => 3.0.0
    @apollo/react-testing: 3.0.0 => 3.0.0
    apollo: ^2.16.3 => 2.17.1
    apollo-cache-inmemory: ^1.6.2 => 1.6.2
    apollo-client: ^2.6.3 => 2.6.3
    apollo-link-http: ^1.5.15 => 1.5.15
  npmGlobalPackages:
    apollo: 2.21.0

My company is moving towards using Apollo Federation with Graph Manager as the source of truth for our schema. We've been moving our schema-dependent tools and processes over to use the schema stored in Graph Manager. We're now looking at fetching the schema for the IntrospectionFragmentMatcher. We've setup a script to download the schema from GM and generating the fragment types JSON from that. Since pieces of our schema are now being deployed separately and at different times, it will be difficult for us to keep the fragment types from being stale and potentially causing client bugs.

[benjamn]

Since this involves obtaining new information about possibleTypes from the GraphQL server at runtime, in production, and schema introspection is typically disabled in production, I think this needs dedicated server support. I'm confident we can get this working flawlessly if you're using Apollo Server, but otherwise you might need to adapt our solution to whatever server you're using.

Specifically, the server could send back any relevant extensions: { possibleTypes: { [supertype: string]: string[] }} information with every query response. That might sound like a lot of extra information, but it would be limited to just the relevant supertype-subtype relationships for the current query, given the set of all concrete __typenames in the response, and all supertypes that appear as fragment type conditions. For most queries, this would be an extremely limited set of relationships. For queries containing no fragments with type conditions, nothing would need to be sent back. This extensions.possibleTypes metadata would be safe to send to any client, even if the client isn't prepared to process it, since it can be safely ignored.

I think it still makes sense to have a way of specifying possibleTypes on the client, in case you're using a server that doesn't have this extension, but a cooperating client and server could exchange all the possibleTypes information they need in this way, in principle.

[rylanc]

Wow, that sounds pretty ideal. We are using Apollo Server (for our gateway), so that solution sounds like it would work. Should I file an issue in the apollo-server repo?

[noaelad]

Second that, that sounds fantastic

[noaelad]

Would this be the equivalent of the extension based strategy of ProgressiveFragmentMatcher?
https://github.com/lucasconstantino/apollo-progressive-fragment-matcher

[nodkz]

@benjamn it may be a stupid question – but maybe we can get rid of possibleTypes?

How I understand all these things created for readFromStore.ts & writeToStore.ts:
https://github.com/apollographql/apollo-client/blob/4cca16e448cc1d52069512cde58dfa5603338e6f/src/cache/inmemory/readFromStore.ts#L305-L307
https://github.com/apollographql/apollo-client/blob/4cca16e448cc1d52069512cde58dfa5603338e6f/src/cache/inmemory/writeToStore.ts#L258-L260

But what I really cannot understand – for what purpose exists this if (policies.fragmentMatches(fragment, typename)) check? Maybe we can read/write any fragment data to cache without any fragment check?

Anyway, the cache does not know anything about types and fragment types is some kind of duck-checking.

I will be glad if you point me for test-case or issue where I can understand the real purpose of this if-check. Tnx.

[nodkz]

No if (policies.fragmentMatches(fragment, typename)) in the code – no problem with this issue. And all stale clients (when graphql schema was extended by additional types) continue to work without breaks.

[noaelad]

@benjamn any update on this issue?