Apollo-client: Union type with fragments fails upon data change.

[dfee]

Intended outcome:
I can query on a Union type. Upon resetting the store, new data is available to the component.

Actual outcome:
When I reset the store (refetching the query) the store fails to update the component.

How to reproduce the issue:
Clone this repo and run: https://github.com/dfee/react-apollo-error-template/tree/union

[helfer]

@dfee Thanks for the reproduction. What am I supposed to see / do with it? I just see a "reset store" button. Not clear what should / should not happen when I press it.

[dfee]

@helfer so when you press "reset store" you'd expect the Primary objects to be updated just like the People objects. The "Primary" query is a union query between People and Books.

The "reset store" is just used as a convenience instead of a "refetch" on each component. But fundamentally, you'll see that "People" are updated after the mutation (and a refetch), while "Primaries" aren't.

[dfee]

@helfer let me know if i can do anything more here. Thanks

[dfee]

FYI – I saw documented on graphql-tools that that package requires __resolveType to be specified for interfaces and unions. My server now provides that field, but the error persists on the client.

A brief overview of what does and does not work:
Fails

// `props.data.subscribeToMore.updateQuery` executes,
// and new props are NOT passed to component.
const _fragments = {
  event: gql`
    fragment Event on Event {
      __resolveType
      __typename
      ... on MembershipEvent {
        id
      }
      ... on MessageEvent {
        id
      }
    }
  `,
};

Succeeds

// `props.data.subscribeToMore.updateQuery` executes,
// and new props ARE passed to component.
const _fragments = {
  event: gql`
    fragment Event on Event {
      __resolveType
      __typename
      # ... on MembershipEvent {
      #  id
      # }
      ... on MessageEvent {
        id
      }
    }
  `,
};

Even more strangely, when props.data.subscribeToMore.updateQuery is called multiple times on the _failing_ case, we still see that the previousResult reflects that the return value from the previous run did update the Apollo store.

This leads me to believe there is some simple "gotcha" that I'm missing.

Edit: I oughta re-iterate that the query succeeds for both cases. This is only an issue on new subscription data.

[helfer]

Thanks @dfee and sorry for the delay, I'll take another look today!

[dfee]

With @helfer's help I believe we've uncovered my problem, and I'll provide a synopsis here:

Problem
Using union (or interface) ObjectTypes, requires providing a custom fragmentMatcher property to the constructor options of apolloClient.

What's going on
I'm not entirely clear on this (JS / TS is a second language for me, so I've really just a cursory understanding) but the code for the IntrospectionFragmentMatcher is here.

Note: the IntrospectionFragmentMatcher is _not_ the default fragmentMatcher (the default is actually the HeuristicFragmentMatcher as shown in the client constructor).

You'll notice for the code of the HeuristicFragmentMatcher is unable to determine what type of fragment is being parsed – specifically in the case of UNION and INTERFACES this is problematic.

Solution
The solution is to provide a custom fragmentMatcher argument to the options for your apolloClient.

Right now, this is something you'll need to do by hand – i.e. if you revise your ObjectTypes on your server, you'll need to keep this in sync on your client (more about this in the next section).

Here's the approach I took:

1. Query the GraphQL server to get a review of the schema:

{
  __schema {
    types {
      kind
      name
      possibleTypes {
        name
      }
    }
  }
}

1. extract UNION and INTERFACE types from the list and construct a fragment matcher like so:

// file: `config/apollo/fragmentMatcher.js`
import { IntrospectionFragmentMatcher } from 'apollo-client';

const fm = new IntrospectionFragmentMatcher({
  introspectionQueryResultData: {
    __schema: {
      types: [
        {
          kind: "UNION",
          name: "Event",
          possibleTypes: [
            { name: "MembershipEvent" },
            { name: "MessageEvent" },
          ],
        }, // this is an example, put your INTERFACE and UNION kinds here!
      ],
    },
  }
});

export default fm;

1. Use this fragmentMatcher in your apolloClient constructor:

// file: `config/apollo/client.js`
import { ApolloClient } from 'react-apollo';

import fragmentMatcher from './fragmentMatcher';
import networkInterface from './networkInterface';

const client = new ApolloClient({
  fragmentMatcher,
  networkInterface, // I use a custom networkInterface. If you aren't, don't include this arg :)
});
export default client;

Next Steps
There are a few ideas that should be considered at this point:

• consider querying your GraphQL server for the schema before instantiating your apolloClient (removes the hardcoding concern).
• wait and see if IntrospectionFragmentMatcher becomes the default fragment matcher.
• update the official documentation to include the process above
• receive feedback from @helfer and the Apollo team to see what other points they'd add.

Etc
Very much appreciate the support from @helfer on Apollo's official slack community. Thanks!

[decafdennis]

Great writeup @dfee!

I want to add: there's a huge advantage to using the IntrospectionFragmentMatcher because it will actually throw an exception if you query for a union/interface type that it doesn't know about, whereas the default HeuristicFragmentMatcher fails silently and the only symptom is your component not updating. For me, switching to the IntrospectionFragmentMatcher uncovered a couple of places in my app where I was relying on union/interface types that I didn't even realize were not updating properly.

[helfer]

I'm thinking about printing a warning to the console any time someone uses fragments but doesn't use the introspection fragment matcher. Do you guys think that would be a good approach?

[dfee]

@helfer I think that would be a tremendous approach. In fact, what @decafdennis is saying is the final point I should've made! This would be incredibly helpful as a web search for the error will inevitably turn the user to using the IntrospectionFragmentMatcher (as opposed to the current situation: a silent fail).

[decafdennis]

@helfer I think that would be great! Although that's assuming there's a good reason to keep the HeuristicFragmentMatcher the default fragment matcher, which I'm not clear on, because otherwise why not just switch to an empty IntrospectionFragmentMatcher by default?

[helfer]

The main problem is that the introspection fragment matcher needs to know about all the union and interface type, otherwise those fragments will never match, and no error will be thrown.

The heuristic fragment matcher is the best way to get started because it doesn't require a build step, and it doesn't require people to manually enter information about their types, as long as their app doesn't use fragments on unions or interfaces (which I think is an ok assumption to make). As soon as fragments on union or interface types enter the picture, the heuristic fragment matcher can print a good warning, but the introspection fragment matcher could not, which is why I made the above proposal.

Does that make sense?

[decafdennis]

@helfer Assuming you're not using fragments on unions on interfaces, in what situation would an introspection fragment matcher (without type info) fail, where the heuristic fragment matcher would succeed?

[helfer]

It wouldn't, but there's no way to tell if a fragment doesn't match because it shouldn't, or because type info is missing, so there's no way to print a warning when type info is missing. with the heuristic matcher we would just print a warning any time a fragment doesn't match.

[decafdennis]

Gotcha, thanks for explaining.

To reiterate, I think the warning in the heuristic matcher is a great idea. 👍

[ndarilek]

So this should work, then? Any idea why https://github.com/ndarilek/graphql-fragments-error doesn't? lib/apollo.js in that reproduction seems to set up fragmentMatcher as shown in this issue.

Including this reproduction here because my related issue was closed, and I'd really like to find out why this particular use fails. Details of how to trigger the failure can be found in #1608.

Thanks.

[helfer]

Hi @ndarilek. I'm closing this issue since the original problem has been resolved. Please feel free to open a new issue! 🙂

[ndarilek]

Wait, what? No it hasn't. I even left my question with this reproduction
on the second issue, but haven't heard back yet.

No worries if you're busy and still looking into it, but it definitely
is not resolved for me, as stated both on this issue and on the other. I
can wait, I just wanted to be clear on where this stood.

Thanks.

[ndarilek]

Can I please ask that either this issue be reopened, or that #1608 be repurposed to address my regression, which still fails even on the latest Apollo? I'm concerned that, with both issues closed, this will be lost. I'm also concerned that, were I to open a new issue, it'd be identical to #1608 which was closed in favor of this one, which was also closed. If I open a new issue, I'll basically cut-and-paste #1608, so it just seems easier to reopen it.

Thanks.

[cesarsolorzano]

@ndarilek I'll take a look at the reproduction, and I'll reopen your issue if necessary! 🙂

[ndarilek]

Thanks so much! At this point it's probably a silly mistake on my part,
and I apologize if so. I've been banging my head on this one for weeks,
and Stack Overflow/independent research is no help.

[helfer]

@ndarilek Sorry about that. We'd be happy to open #1608 again, but I posted a suggestion there and got no reply for 2 weeks, so I saw no reason to keep the issue open. Please post an updated reproduction to use the latest versions of react-apollo and apollo-client, and then we'll reopen the issue! 🙂

[ndarilek]

Oh, I think I see what happened. Thunderbird or Github recently changed
reply-by-email so I need to use reply-to-list for commenting by email. I
commented that I'd implemented your suggestion into the reproduction and
it didn't work, but that comment didn't go through because I didn't
realize this change had happened until recently.

In any case, I implemented your suggestion and it didn't seem to change
anything. :) Reproduction was updated this morning to apollo-client
1.2.0, no change.

Sorry for the confusion/indignant response. :)

[Aides359]

A warning would be really helpful.

The way it currently works the server request is correctly executed, the correct response is obtained (as visible in the network tab of chrome debugging tools) but the result from apollo-client has data: undefined.

After 7 hours of debugging I was able to pinpoint the cause, tried to reproduce it in a test for apollo-client, only to get

You are using the simple (heuristic) fragment matcher, but your queries contain union or interface types.
Apollo Client will not be able to able to accurately map fragments.
To make this error go away, use the IntrospectionFragmentMatcher as described in the docs: http://dev.apollo
data.com/react/initialization.html#fragment-matcher

which I would have loved to have 7 hours earlier.

When I then wanted to create an issue describing the lack of warning I stumbled across this issue, realizing it was a known problem I didn't find due to the lack of information in the observed problem.

[avindra]

@Aides359 I disagree. The warning is there (in dev mode only). This is something that should throw an error. Created #1719 to request this change.

[gbenga504]

I am actually using the latest version of Apollo client and react Apollo... And also running in Dev mode, didn't see any warning. I would have loved it users could be noted to use the introspectionFragmentMatcher .. At least it would be a perfect guide.

[khankuan]

@helfer would there be a benefit of making HeuristicFragmentMatcher slightly smarter? I feel that warning devs over the recommended solution is great. At the same time, it would be less a friction to get started with apollo if we can make some assumptions and avoid having the schema included on the application. Cheers!

[helfer]

@khankuan I don't know how to make it any smarter than it already is, but if you have an idea, I'd love to hear it. Right now it won't throw any errors if you use a fragment that matches the type condition it's checked against. So unless you are using union or interface types, or you have a bad type condition, the heuristic fragment matcher will work without complaining.