Relay: [relay-compiler] Generated types for fragment on interface type with inline fragments

[NickDubelman]

I have a reproduction here, where I show that you must do something like

  fragment App2_animal on Animal {
    __typename
    ... on Cat {
      id
      name
      age
      lives
    }
    ... on Dog {
      id
      name
      age
      breedDescription
    }
  }

instead of

  fragment App_animal on Animal {
    __typename
    id
    name
    age
    ... on Cat {
      lives
    }
    ... on Dog {
      breedDescription
    }
  }

I have read as many issues as I could find in this repo and the ts repo and it seems this is intentional? Why is that the case?

[zth]

What's the error you're getting?

[NickDubelman]

I don’t believe there is an error, I’m just trying to find out why I have to list the common fields on the interface multiple times within each inline fragment, instead of just listing them once, as selections on the interface type.

[zth]

Ok, but _what_ is forcing you to do that? If there's no error, what's not working? What effect does your code give you and what is it that you would like to happen?

I feel like we're missing a few pieces of the puzzle here 🙂 what is it that's not happening that you'd like to happen?

[NickDubelman]

Relay-compiler's type generation (for both flow and TS) is what would force me to write my fragments that way. Because if I don't, I will get this type:

export type App_animal = {|
  +__typename: string,
  +id: string,
  +name: string,
  +age: number,
  +lives?: number,
  +breedDescription?: string,
  +$refType: App_animal$ref,
|};
export type App_animal$data = App_animal;
export type App_animal$key = {
  +$data?: App_animal$data,
  +$fragmentRefs: App_animal$ref,
  ...
};

instead of this type:

export type App2_animal = {|
  +__typename: "Cat",
  +id: string,
  +name: string,
  +age: number,
  +lives: number,
  +$refType: App2_animal$ref,
|} | {|
  +__typename: "Dog",
  +id: string,
  +name: string,
  +age: number,
  +breedDescription: string,
  +$refType: App2_animal$ref,
|} | {|
  // This will never be '%other', but we need some
  // value in case none of the concrete values match.
  +__typename: "%other",
  +$refType: App2_animal$ref,
|};

If you take a look at my reproduction linked above, you'll also see that the TS generated types do the equivalent thing. However, I should note that the TS types used to be capable of generating the desired way in older versions, but they seemed to have changed it to be consistent with the flow types.

I have combed through as many issues as I could find, and as far as I can tell, the Relay team considers this to be desired behavior. First, can you confirm or refute that claim? If the claim is correct, then I would like to hear the justification as to why. If the claim is incorrect, then this should be considered a bug report.

[zth]

Ahh, right, now I follow!

I've actually looked into this previously before too IIRC. From what I remember, @josephsavona has described it as this is more close to the types you'd get at runtime, although I don't remember more about the specifics. And it does look weird that you can kind of "trick" the type generation into the behavior you want.

I'm a regular user and don't represent the Relay team, so I can't refute or confirm. Maybe someone from the team can comment again on this?

[NickDubelman]

I just found this test snapshot and it leads me to believe that I can assume this is desired behavior.

However, it doesn't seem like there is a technical hurdle to allowing users to specify the common fields just once? If you have N common fields and M possible types that implement a given interface, then you have to request N * M fields in your code, instead of just N. It seems clunky?

[AegisToast]

I've been wrestling with this with my team for the last several weeks, and this is a pretty big issue. The "workaround" doesn't actually solve the problem, and it causes bugs that cause your app to crash.

Consider a sub-component that displays the animal's name and age in a fancy way. It might have this fragment:

fragment Nameplate_animal on Animal {
  name
  age
}

Great! There are 2 options for using that fragment:

Option 1 - Duplicated on every type of Animal

fragment App_animal on Animal {
  __typename
  ... on Cat {
    ...Nameplate_animal
    id
    name
    lives
  }
  ... on Dog {
    ...Nameplate_animal
    id
    name
    breedDescription
  }
}

That gives you strong, accurate generated types, but crashes the app if Animal is a "Bird", since the FragmentRef that the Nameplate component is looking for is not present on App_animal. That's not maintainable, and requires front-end changes any time new types are added to the back-end, which is not possible when you've got an app in production.

Option 2 - Listed once as part of the common fields

fragment App_animal on Animal {
  __typename
  ...Nameplate_animal
  id
  name
  ... on Cat {
    lives
  }
  ... on Dog {
    breedDescription
  }
}

The advantage here is that you don't break your production app when new types of Animals are added (which means it's actually the _only_ option), but you lose the accurate types generated, instead getting the jumbled __typename: string object.

And in case anyone's wondering, it doesn't matter if the id or name fields are listed as common fields; if _any_ fields besides __typename are present, you get the jumbled types.

I'd really prefer not to have to decide between having accurate types or writing unmaintainable code.

As a potential solution, could we make the types that are output something like this? (Ignore the field names, just trying to illustrate the way to handle __typename)

export type App_animal = {
  __typename: "Cat"
  id: string
  name: string
  age: number
  lives: number
  $refType: App_animal$ref
} | {
  __typename: "Dog"
  id: string
  name: string
  age: number
  breedDescription: string
  $refType: App2_animal$ref
} | {
  // This will never be '%other', but we need some
  // value in case none of the concrete values match.
  __typename: string
  $refType: App2_animal$ref
}

Heck, even keeping the "%other" __typename field instead of string would be fine (though, in my opinion, string is more accurate), but we need accurate types when common fields are used.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[Traviskn]

Apollo seems to support this kind of thing with their "possible types" config: https://www.apollographql.com/docs/react/data/fragments/#using-fragments-with-unions-and-interfaces

It would be really awesome if Relay could support not needing to duplicate common fields