Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Full support for union/interface types #153

Open
nevir opened this issue Oct 10, 2017 · 3 comments
Open

Full support for union/interface types #153

nevir opened this issue Oct 10, 2017 · 3 comments

Comments

@nevir
Copy link
Contributor

nevir commented Oct 10, 2017

Update: Much of this has been implemented. See this comment for remaining work.


We don't support ... on Type { queries currently. But it shouldn't be too difficult to support, I think.

Rough outline of the work:

  1. When parsing queries, we need to walk InlineFragment AST nodes. Most likely, they are added as another field node (but explicitly flagged as being a union field)
  2. When writing fields, any union field present in the payload should be written normally (except if not present, nulls shouldn't be inserted)
  3. When reading queries there should be no work required for unions with only static fields

Potential challenges:

  • Parameterized fields contained by a union query may need special handling during (and __typename or schema introspection)
@anthonychung14
Copy link

anthonychung14 commented Apr 20, 2018

I'd be interested in helping with this, though I'm just now getting onboarded with the source code. Our team would benefit greatly from incorporating the improvements you've made with the apollo cache.

Here's how I understand the lib + parsing queries with InlineFragment

  • Currently, reading an inline fragment would result in a "failed lookup" since it doesn't know that the fragment needs to be rebuilt (later on?). I'm assuming this is why you mentioned flagging them as union fields.

  • It seems like here detects and flattens fragments into the nodeMap. This implies to me that fragments are already (in a sense) being "walked" and they only need to be flagged as unions.
    https://github.com/convoyinc/apollo-cache-hermes/blob/master/src/ParsedQueryNode.ts#L148

  • So, would we then want to adjust how _mergeNodes works to complete this flagging?

@jamesreggio
Copy link
Contributor

Hi @anthonychung14 — welcome to the project.

I've implemented 95% of the support needed for inline fragments over in #183. If you'd like to begin using it today, I'd suggest you incorporate my branch into a private fork.

(The comment thread on that PR outlines the remaining work needed to get support to 100%, and it's not trivial — it involves having the entire schema available on the client to understand which fragment was matched. This remaining work is mostly in the realm of validation, so it doesn't need to be finished to begin using the branch today.)

@jamesreggio jamesreggio changed the title Support for inline fragments in queries Full support for union/interface types Jan 9, 2019
@jamesreggio
Copy link
Contributor

We've merged support for inline fragments in #183. In the process of implementing it, it became clear that the 'hard' part of this is handling multiple fragments for different types on a given object. This is a common scenario when querying a union type (because you'll likely have one fragment for potential type in the union), but it also crops up when querying object interfaces.

The problem is that without the client being aware of the schema, it doesn't know which type fragments actually matched the object on the server; consequently, it doesn't know which keys to write into the cache, and which to ignore.

@nevir illustrated this problem in #183 (review). Generally, this problem is benign, because it only results in extra nullified keys in the cached object. However, as illustrated in #183 (comment), it can become a little more risky when an object changes types, because you can end up with residual key/value pairs from the prior type fragments.

In the end, this will require something akin to IntrospectionFragmentMatcher from apollo-cache-inmemory to solve. Would be happy to guide anybody who would like to take on this work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants