facebook / relay

Relay is a JavaScript framework for building data-driven React applications.
https://relay.dev
MIT License
18.4k stars 1.82k forks source link

[relay-compiler]: Union types #2870

Open artola opened 5 years ago

artola commented 5 years ago

There is an "strange" generation of Flow typings depending on how a query is written. I wrote several examples using the star-wars schema (see repo). Mainly: require primaryFunction in a "human hero" should not be possible, not that it returns undefined, because it does not exists in the Human type.

// required
graphql`
  query trialQuery {
    hero {
      name
      ... on Human {
        homePlanet
      }
      ... on Droid {
        primaryFunction
      }
    }
  }
`;

// obtained
export type trialQueryResponse = {|
  +hero: {|
    +name: string,
    +homePlanet?: ?string,
    +primaryFunction?: string,
  |}
|};

// expected
export type trial4QueryResponse = {|
  +hero: {|
    +__typename: "Human",
    +name: string,
    +homePlanet: ?string,
  |} | {|
    +__typename: "Droid",
    +name: string,
    +primaryFunction: string,
  |} | {|
    // This will never be '%other', but we need some
    // value in case none of the concrete values match.
    +__typename: "%other"
  |}
|};

In our TS project obtaining strict union typings is required, see issue and discussion: https://github.com/relay-tools/relay-compiler-language-typescript/issues/135

Repo: https://github.com/artola/rc-bug

Implementation reference details: https://graphql.github.io/graphql-spec/June2018/#CollectFields()

I would like to re-check if it is a bug (see repo, at least "case 2").

kassens commented 4 years ago

This is a little bit difficult to express. name will also be present if the type is neither Human nor Droid. I think we could generate the other fields under the "other" union member. Unfortunately, the code that's converting the Schema to (flow) types is pretty messy and I'm not sure if it's easy to change :(

stale[bot] commented 3 years ago

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.

artola commented 2 years ago

@kassens Even the newest relay-compiler@main it is not able to make the union types as expected. Only when all the fields are declared inside each type and containing a __typename then we see the right unions. It would be great to have them without this nasty workaround.

graphql`
  query demo8Query {
    hero {
      ... on Human {
        __typename
        name
        homePlanet
      }
      ... on Droid {
        __typename
        name
        primaryFunction
      }
    }
  }

obtained:

export type demo8Query$data = {|
  +hero: {|
    +__typename: "Human",
    +name: string,
    +homePlanet: ?string,
  |} | {|
    +__typename: "Droid",
    +name: string,
    +primaryFunction: string,
  |} | {|
    // This will never be '%other', but we need some
    // value in case none of the concrete values match.
    +__typename: "%other",
  |},
|};