Removing __typename from monomorphic fragments

[AnkitPatanaik]

Use case
I have an existing project that already uses GraphQL in production and am currently in the process of adding Apollo 3.0.0 to our project. Part of the complexity is that for any given query, we rely on many dozens of fragments which currently don’t explicitly query for __typename.

Describe the solution you'd like

• For us to most easily integrate Apollo throughout our project it would be necessary to not add __typename to monomorphic types. I believe @martinbonnin mentioned possible solutions here in !3672. Specifically:

\ mid-term: (apollo-side) optimize and do not add __typename for monomorphic fields

• Do you have a timeline for when you would be able to address this or have any recommended work-arounds to ease integration?

Let me know if there's anything I can add or provide that would be helpful!

• I created this new Issue to make it easier for discussion/visibility to avoid piling onto !3672 which is closed
• This is also related to !1458

[martinbonnin]

Thanks for giving this problem the issue it deserves :)!

I'll take a look this week. Early draft there: #3777

[AnkitPatanaik]

Hey @martinbonnin! Thanks for sharing your draft 😄 I was wondering if you had any updates on a timeline for it's merge

[martinbonnin]

Hi @AnkitPatanaik. Very sorry for the delay on this. I've been having a few personal issues + this issue is not an easy one so it takes time to dive into it but it's still ongoing.

[martinbonnin]

Small update: we had to ship 3.1.0 without this because this issue is still ongoing and it's a substantial change that I don't want to rush in. I dumped my current thoughts in Typename.md. Sorry for the delay but this is still ongoing.

[martinbonnin]

Apologies again for the huge delay there that is due to a bunch of unplanned life events...

The good news is that this PR finally landed on main. It introduces a addTypename option to customise how __typename is handled:

apollo {
  /**
   * Add '__typename' for abstract fields, i.e. fields that are of union or interface type
   *
   * `"ifAbstract"` is a good and simple value that will work most of the times.
   *
   * Note: It also adds '__typename' on fragment definitions that satisfy the same property because fragments
   * could be read from the cache and we don't have a containing field in that case.
   **/
  addTypename.set("ifAbstract")

  /**
   * Add '__typename' for polymorphic fields, i.e. fields that contains a subfragment
   * (inline or named) whose type condition isn't a super type of the field type.
   * If a field is monomorphic, no '__typename' will be added.
   *
   * `"ifPolymorphic"` adds the bare minimum number of `__typename`. If you're using the cache it can lead
   * to some extra cache misses because of a missing `__typename` so test before you opt-in
   * 
   * Note: It also adds '__typename' on fragment definitions that satisfy the same property because fragments
   * could be read from the cache and we don't have a containing field in that case.
   **/
  addTypename.set("ifPolymorphic")


  /**
   * Add '__typename' for every selection set that contains fragments (inline or named) (Default)
   *
   * `"ifFragments"` is adding a lot more '__typename' than the above "ifPolymorphic" and "isAbstract" and will be removed in
   * a future version. If you require '__typename' explicitely, you should add it to your queries.
   **/
  addTypename.set("ifFragments")
}

If you're not using the normalized cache, "isPolymorphic" is certainly a good value. If you are, "ifAbstract" is safer (and also considerably simpler). You can read more about it in the design doc.

There's still a default "ifFragments" value that is kept for backward compatibility. The current plan is to change the default to "ifAbstract somewhere down the road, depending on feedbacks.

Let us know what you think!

[AnkitPatanaik]

Thanks for fixing this @martinbonnin. This is great!

[AnkitPatanaik]

I'll go ahead and close this too!