Apollo-client: IntrospectionFragmentMatcher works without providing types

[benjarwar]

Intended outcome:

I can make queries using fragments and inline fragments (sometimes combined) using an IntrospectionFragmentMatcher instance. Further, I can implement a solution that ensures the IntrospectionFragmentMatcher is always fed the latest schema data – if required – in a native mobile application environment.

Actual outcome:

My code is working, but the instantiated IntrospectionFragmentMatcher doesn't require any types data to successfully resolve my queries. I'm using cache-and-network and the queries appear to be cached properly (and updated on subsequent client instantiations). So I'm wondering if the instructions provided for fragments on unions/interfaces are accurate. See stripped down example code below.

I read in a previous issue that the IntrospectionFragmentMatcher needs to know the types explicitly, but I'm not clear on the reasons why, nor why it seems to work for me without providing said types.

Further, the documented solution (running a script and saving a JSON file with the data) won't be a good solution for us. For one thing, we're using Contentful, and there may be frequent content model updates occurring outside of our codebase and build pipeline. So we'd need to generate this JSON in our CI pipeline to ensure that it's current. But more importantly, we're building a native mobile app (with ReactNative), so even if we were to update schema JSON as part of our build process, we can't guarantee our users will update their application by the time schema changes are propagated to production, which could easily introduce conflicts and failed requests.

We could perhaps implement a solution that fetches the schema data at runtime, every time. But this creates performance issues (we'd need to prevent our app from rendering until the current schema is fetched). And we could subsequently start creating some caching mechanisms for this initial fetch, but that creates complexity that we'd prefer to avoid until we're 100% certain the types data is necessary for the IntrospectionFragmentMatcher.

How to reproduce the issue:

My client is instantiated like so (stripped down):

const fragmentMatcher = new IntrospectionFragmentMatcher({
  introspectionQueryResultData: {
    __schema: {
      types: [], // no types provided
    },
  },
});

const cache = new InMemoryCache({ fragmentMatcher });

const httpLink = createHttpLink({
  uri: "/graphql",
});
const link = ApolloLink.from([httpLink]);

const defaultOptions = {
  watchQuery: {
    fetchPolicy: "cache-and-network",
    errorPolicy: "all",
  },
  query: {
    fetchPolicy: "cache-and-network",
    errorPolicy: "all",
  },
  mutate: {
    errorPolicy: "all",
  },
};

const client = new ApolloClient({
  link,
  cache,
  defaultOptions,
});

And my working queries looks like this:

query Sources {
  foo: {
    ... on Source {
      ...sourceFields
    }
  }
  bar: {
    ... on Source {
      ...sourceFields
    }
  }
}

fragment sourceFields on Source {
  id
  image {
    url
  }
}

And this:

query {
  feedItems {
    item {
      associatedContent {
        ... on Foo {
          slug
          firstName
          lastName
        }
        ... on Bar {
          slug
          url
          text
        }
      }
    }
  }
}

Version

• [email protected]

[greabock]

I beg of you, do not fix this behavior. Is only way to use dynamicaly schema... i cant cache my schema, beacuse of different users have different schemas at one endpoint.

[benjarwar]

@greabock to be clear, I'm not proposing changing the implementation. The issue is that it doesn't seem to matter whether or not types are passed into IntrospectionFragmentMatcher. Cached queries resolve fine (for me) without them.

[lahed]

So? @benjarwar How to ignore that error ?. For me it also works well without IntrospectionFragmentMatcher.

[benjarwar]

@lahed see the issue posted above where the Apollo team decided to implement a warning. I don't know of a way to ignore it.

CC @helfer who initially suggested adding the warning; any insight here?

[benjarwar]

@helfer been a while but still no response here. Any thoughts?

[hwillson]

To help provide a more clear separation between feature requests / discussions and bugs, and to help clean up the feature request / discussion backlog, Apollo Client feature requests / discussions are now being managed under the https://github.com/apollographql/apollo-feature-requests repository.

This feature request / discussion will be closed here, but anyone interested in migrating this issue to the new repository (to make sure it stays active), can click here to start the migration process. This manual migration process is intended to help identify which of the older feature requests are still considered to be of value to the community. Thanks!

[benjarwar]

@hwillson respectfully, this isn't a feature request as much as it is discrepancies in observed behavior versus the documentation. I'll migrate the issue, but it feels more like a potential bug to me.

[hwillson]

Fair enough @benjarwar! I just re-read and I agree. Re-opening here as a bug. Thanks!

[hwillson]

@benjarwar I've taken a closer look at this. I can't tell from your code samples - are you using any unions or interfaces? Looking at the apollo-cache-inmemory code, I believe things are working for you without providing type details because you aren't using unions/interfaces:

https://github.com/apollographql/apollo-client/blob/d8926f2214fc944885e5f42cf8a8d90248a484f7/packages/apollo-cache-inmemory/src/fragmentMatcher.ts#L137-L146

The above shows the only 2 places a match can be identified by the IntrospectionFragmentMatcher. If things are working for you without specifying the additional type details, then that means you're passing the if (obj.__typename === typeCondition) { check. If you were using unions/interfaces however, you would have to supply type information, or else the union/interface check (the if (implementingTypes && implementingTypes.indexOf(obj.__typename) > -1) { line) will always fail.

Given this, it doesn't sound like there is a bug here. Is there anything additional you think is needed in the docs to help clarify this?

[hwillson]

I'll close this for now @benjarwar - let me know if you think this should be handled differently. Thanks!

[msindwan]

@hwillson We're using interfaces, but an empty IntrospectionFragmentMatcher seems to be working for us.

const fragmentMatcher = new IntrospectionFragmentMatcher({
      introspectionQueryResultData: {
        __schema: {
          types: []
        }
      }
    });

And our query uses fragments with interfaces:

foo {
    ...on fooInstance {
       ....
    }
}

[adamsoffer]

Same here. I'm using a remote graphql endpoint (prismic) that uses interfaces and unions so I was seeing the IntrospectionFragmentMatcher error. I don't have access to the fragment types since it's generated for me remotely by prismic, so in order to suppress the warning I just pass an empty array like @msindwan. It works in that I no longer see the error, but feels like I'm doing something wrong by passing an empty array...

[rooch84]

I can also confirm that I'm in this situation. Again, the empty fragment matcher works for me with a Union.

Requiring the need to inspect the schema before setting up the cache is going to cause me problems for my particular use case - I'll probably need to create the client, extract the fragment matcher, then restart the client each time, which is less then ideal.

Given that it appears to be working OK, is there a need for the IntrospectionFragmentMatcher? Or is defining the __resolveType on the Union the thing that is nullifying the issue?

@adamsoffer - Are you defining the __resolveType on the Union also?

[joelgetaction]

@hwillson I can also confirm the experiences of @rooch84 , @adamsoffer and @msindwan - just adding an empty IntrospectionFragmentMatcher seems to work. So the warning that ApolloClient issues is not accurate and also the instructions at https://www.apollographql.com/docs/react/advanced/fragments.html are also inaccurate and confusing.

This was frustrating for me and felt odd given that Apollo and GraphQL mostly "just work" otherwise.

Any ideas how we can tighten this up so it's less confusing and "just works" by default? Having a stated dependence that the client needs to introspect the schema is just another build complication and doesn't work well in CI, testing or production deployments.

[joelgetaction]

@hwillson can we re-open this please, it doesn't feel fixed yet ... Thanks! :-)

[ledenis]

There seems to be a case where the empty types does not work: when we use named fragments on interfaces.

It would be great if it get fixed for this case also.

  fragment characterFields on Character {
    ... on Droid {
      primaryFunction
    }
    ... on Human {
      height
    }
  }

  query {
    characters {
      __typename
      ...characterFields
    }
  }

Apollo query result contains:

"characters": [
  {
    "__typename": "Droid"
  },
  {
    "__typename": "Human"
  }
]

Sample project: https://github.com/ledenis/apollo-introspection-fragment-matcher-issue

[intellix]

Created this earlier in Spectrum and a colleague alerted me to this issue: https://spectrum.chat/apollo/apollo-client/simple-heuristic-fragment-matcher-but-your-queries-contain-union-or-interface-types~8537e840-f75e-445f-bc1f-31e316984891

If turning on an empty IntrospectionFragmentMatcher works then why is ApolloClient throwing warnings and errors that it's an absolute requirement and needed.

I'd say union support is entirely broken and wrongly saying something is wrong?

Edit: ok so after playing with @ledenis example above the following fails:

fragment CharacterFields on Character {
  ... on Droid {
    primaryFunction
  }
  ... on Human {
    height
  }
}

query {
  characters {
    ...CharacterFields
  }
}

But this succeeds:

query {
  characters {
    ... on Droid {
      primaryFunction
    }
    ... on Human {
      height
    }
  }
}

So the problem is deep nested fragments? It's not particularly about interfaces because it's unions too... if that changes anything?

[jbaxleyiii]

Since this issue has been opened and closed a few times, can someone please open a new issue with a reproduction? Thank you!

[intellix]

Re-opened the issue with a reproduction ^