Shape of models with fragments unexpectedly different when migrating to V1

[pm-dev]

Summary

Let's say I have the following .graphql:

{
   me {
      ...meFragmentOne
      ...meFragmentTwo
   }
}

The shape of the generated swift will look like:

class Query {
   var me: Me? { __data["me"] }

   struct Me: SelectionSet {
      var asMeFragmentOne: AsMeFragmentOne? { _asInlineFragment() }
      var asMeFragmentTwo: AsMeFragmentTwo? { _asInlineFragment() }
    }

    struct AsMeFragmentOne: InlineFragment {
        struct Fragments: FragmentContainer {
            var meFragmentOne: MeFragmentOne { _toFragment() }
          }
    }

    struct AsMeFragmentTwo: InlineFragment {
        struct Fragments: FragmentContainer {
            var meFragmentTwo: MeFragmentTwo { _toFragment() }
          }
    }
}

To access these fragments it looks like:

data.me?.asMeFragmentOne?.fragments.meFragmentOne
data.me?.asMeFragmentTwo?.fragments.meFragmentTwo

Prior to V1 accessing these fragments would look like:

data.me?.fragments.meFragmentOne
data.me?.fragments.meFragmentTwo

It seems there is a fragments property on the Me SelectionSet but it resolves to NoFragments which makes me think this is a bug? If not, this makes migration extremely inconvenient because each usage of a fragment in code will need to be updated

I generated this using the .none option for FieldMerging

Version

apollographql/apollo-ios-dev#571

Steps to reproduce the behavior

Logs

No response

Anything else?

No response

[pm-dev]

This is due to apollographql/apollo-ios-dev#571. Because this is not an issue on a released version, I'm going to close this issue. Discussion to follow in the PR.

[github-actions]

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo iOS usage and allow us to serve you better.

[AnthonyMDev]

What are the types used here? If the me field's return value and meFragmentOne parent type are of the same type, I don't expect this and would consider it a bug.

But this would be the expected generated code for something that looks like this:

type Me { ... }
type MeFragmentOne { ... }
type MeFragmentTwo { ... }

{
   me { // type is Me
      ...meFragmentOne
      ...meFragmentTwo
   }
}

fragment meFragmentOne on MeFragmentOne { ... }
fragment meFragmentTwo on MeFragmentTwo { ... }

[AnthonyMDev]

If they are of the same type, you may be able to get this working by using a field merging policy of [.ancestors, .siblings].

[pm-dev]

@AnthonyMDev assume the Me type is an interface and that the fragments are on different types. In this case [.ancestors, .siblings] wont help.

Again, in this case I'd expect to be able to access each fragment like: data.me?.fragments.meFragmentOne

[AnthonyMDev]

I see, thank you for the clarification. This is a design decision that was made for 1.0 and is a change from the behavior of the legacy models. This is not a bug. You will need to change your call sites.

The 1.0 version has a lot of breaking changes from the 0.x, and it's not an easy migration. Unfortunately, we needed to make some future-forward decisions to create a more stable API to support additional features, and this just means a lot of legacy code is incompatible. Once you've migrated to 1.0, future versions should be easy to migrate to though. We are working on 2.0 right now, and anticipate the migration path to be much less work there.

[github-actions]

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo iOS usage and allow us to serve you better.

[pm-dev]

@AnthonyMDev thx. Is there an issue/PR that explains why this design decision was made? I'm sure it was well founded, I'm just curious to learn the reasoning.

[AnthonyMDev]

I can give you the reasoning!

Given this schema and operation:

interface Me { ... }
type A { ... }
type B { ... }

{
   me { // type is Me
      ...meFragmentOne
      ...meFragmentTwo
   }
}

fragment meFragmentOne on A { ... }
fragment meFragmentTwo on B { ... }

The way this is actually executed by a GraphQL server is to check if the underlying type of Me is A. Only if it is, then execute the selections in the meFragmentOne. Same for meFragmentTwo with B. Essentially, the operation above is a reduced version of this operation:

{
   me { // type is Me
      ... on A {
         ...meFragmentOne
      }
      ... on B {
         ...meFragmentTwo
      }
   }
}

And this is actually how the GraphQL spec expects that a server handles execution of this operation.

We decided to have the response models more closely resemble the expanded version of this operation. And the reason actually has to do with clarity of the code when dealing with optionality.

In this situation, if you were to handle it the way we did in the legacy codegen, data.me.meFragmentOne, would actually return an Optional<MeFragmentOne>. But the fragment itself can't actually be optional. The reason why this is optional is because the fragment will only be present if the underlying run time type of me is A. That isn't clear in the call site above.

Instead, the call site now requires you to call this as data.me.asA?.meFragmentOne. It's now clear that the reason for optionality is because of the typecast to A, which may or not succeed. This becomes a lot more confusing when you have multiple different fragments.

Even in the operation above, the legacy version would have callosities like data.me.meFragmentOne and data.me.meFragmentTwo. These are both optional, but with different type conditions (A or B). There is no way to understand why you would be getting one of them but not the other from looking at the call site. It would even be difficult to understand what was going on there when looking at the generated models. You would really need to refer all the way back to the GraphQL files themselves, looking at both the operation and fragments, and piecing together manually what is going on here.

This design change dramatically improves local reasoning. That is why we went in this direction.

---

I'd also note that in the situation where we can guarantee that the fragment will be resolved, we do simplify the generated models (though some of that simplification does require field merging to be enabled, which I know you are not able to use).

Given this alternative schema:

interface Me implements A & B { ... }
interface A { ... }
interface B { ... }

The code generation engine does recognize that ... on A will always resolve successfully and it does reduce the generated models to reflect that. So you wouldn't even have a data.me.asA field to access. The fragment would be accessible using data.me.meFragmentOne which would not be Optional at all in this situation.

[pm-dev]

Thanks for the detailed explaination :)