Adding __typename on a interface field generates generates string type instead of union type with constant strings

[n1ru4l]

Schema Types:

type DiceRollCloseParenNode implements DiceRollDetail {
  content: String!
}

type DiceRollConstantNode implements DiceRollDetail {
  content: String!
}

interface DiceRollDetail {
  content: String!
}

type DiceRollDiceRollNode implements DiceRollDetail {
  content: String!
  min: Float!
  max: Float!
  rollResults: [DiceRollResult!]!
}

type DiceRollOpenParenNode implements DiceRollDetail {
  content: String!
}

type DiceRoll {
  result: Float!
  detail: [DiceRollDetail!]!
}

export const FormattedDiceRoll = createFragmentContainer(DiceRollRenderer, {
  diceRoll: graphql`
    fragment formattedDiceRoll_diceRoll on DiceRoll {
      result
      detail {
        __typename
        content
        ... on DiceRollDiceRollNode {
          content
          rollResults {
            dice
            result
            category
          }
        }
      }
    }
  `,
});

Generated Type:

export type formattedDiceRoll_diceRoll = {
    readonly result: number;
    readonly detail: ReadonlyArray<{
        readonly __typename: string;
        readonly content: string;
        readonly rollResults?: ReadonlyArray<{
            readonly dice: string;
            readonly result: number;
            readonly category: DiceRollCategory;
        }>;
    }>;
    readonly " $refType": "formattedDiceRoll_diceRoll";
};

Expected Generated Type:

export type formattedDiceRoll_diceRoll = {
    readonly result: number;
    readonly detail: ReadonlyArray<
      {
        readonly __typename: "DiceRollCloseParenNode" | "DiceRollOpenParenNode" | "DiceRollConstantNode";
        readonly content: string;
      } |
      {
        readonly __typename: "DiceRollDiceRollNode"
        readonly content: string;
        readonly rollResults: ReadonlyArray<{
            readonly dice: string;
            readonly result: number;
            readonly category: DiceRollCategory;
        }>;
      }
    }>;
    readonly " $refType": "formattedDiceRoll_diceRoll";
};

HOWEVER:

The following will generates the correct types:

Fragment Container:

export const FormattedDiceRoll = createFragmentContainer(DiceRollRenderer, {
  diceRoll: graphql`
    fragment formattedDiceRoll_diceRoll on DiceRoll {
      result
      detail {
        ... on DiceRollOperatorNode {
          __typename
          content
        }
        ... on DiceRollConstantNode {
          __typename
          content
        }
        ... on DiceRollOpenParenNode {
          __typename
          content
        }
        ... on DiceRollCloseParenNode {
          __typename
          content
        }
        ... on DiceRollDiceRollNode {
          __typename
          content
          rollResults {
            dice
            result
            category
          }
        }
      }
    }
  `,
});

Generated Code:

export type formattedDiceRoll_diceRoll = {
    readonly result: number;
    readonly detail: ReadonlyArray<{
        readonly __typename: "DiceRollOperatorNode";
        readonly content: string;
    } | {
        readonly __typename: "DiceRollConstantNode";
        readonly content: string;
    } | {
        readonly __typename: "DiceRollOpenParenNode";
        readonly content: string;
    } | {
        readonly __typename: "DiceRollCloseParenNode";
        readonly content: string;
    } | {
        readonly __typename: "DiceRollDiceRollNode";
        readonly content: string;
        readonly rollResults: ReadonlyArray<{
            readonly dice: string;
            readonly result: number;
            readonly category: DiceRollCategory;
        }>;
    } | {
        /*This will never be '%other', but we need some
        value in case none of the concrete values match.*/
        readonly __typename: "%other";
    }>;
    readonly " $refType": "formattedDiceRoll_diceRoll";
};

But this is how I expect union types to work. IMHO interface fields should not require that much boiler plate code.

GraphQL Codegen supports generating the "correct" interface selection set typings.

[renanmav]

Maybe I’m missing something, but shouldn’t you declare the union type within your schema?

Something like:

union DiceRollDetail =
    DiceRollDiceRollNode
  | DiceRollConstantNode
  | DiceRollCloseParenNode

type DiceRoll {
  result: Float!
  detail: [DiceRollDetail!]!
}

[n1ru4l]

@renanmav Why use a union type instead of an interface type? It makes it utterly complicated when selecting only the shared fields (such as content in this example).

Interface

fragment formattedDiceRoll_diceRoll on DiceRoll {
  result
  detail {
    __typename
    content
  }
}

vs

Union

fragment formattedDiceRoll_diceRoll on DiceRoll {
  result
  detail {
    ... on DiceRollOperatorNode {
      __typename
      content
    }
    ... on DiceRollConstantNode {
      __typename
      content
    }
    ... on DiceRollOpenParenNode {
      __typename
      content
    }
    ... on DiceRollCloseParenNode {
      __typename
      content
    }
    ... on DiceRollDiceRollNode {
      __typename
      content
    }
  }
}

[renanmav]

I’m not very experienced with union types, but the last snippet seems more explicit, and one of relay’s purposes is explicit data requirements.

[kastermester]

How does the generated code and your expected output align up with how Relay for flow works? I know we've had some issues regarding this in the past, but I think we should generate the code the same way the official implementation works (I'm not saying that is what is happening now, but there has been bugs both in the official implementation and in ours).

Can you do comparisons with the flow based implementation and see if there's any differences there? If there is then it is definitely a bug that we should look into fixing.

[maraisr]

@n1ru4l I love this idea! Would make conditional's type-safe as well.

If we get a consensus we should look at implementing it.

@kassens @josephsavona @jstejada can you give us some guidance if this is even a good idea, as we'd love this to make into relay-compiler (rust) as well.

[josephsavona]

Until recently this wasn't feasible, since Relay didn't know at runtime which types implement which interfaces. We changed that with the new precise type refinement feature, so that we dynamically query this information as necessary. We expect to enable that feature by default in an upcoming release at which point we can start generating more precise types for interfaces (ie as suggested here, instead of making all fields on the interface result value optional, generate a union with non-optional fields). However, we're also looking at alternatives to __typename for doing refinement in product code. Ie instead of checking typename, we could assign the result of a type refinement (... on SomeType { selection }) to a named object so that you could do if (object.SomeType) { object.SomeType.selection }.

So: we're not quite ready to do this yet, but agree w the direction.

[embpdaniel]

@josephsavona Can you give us any update on this and the open PR #192 that seems to resolve this?

[josephsavona]

This is in our backlog. It's a major change to the way we produce types and we can't simply flip a switch here, we need to figure out an upgrade path.

[embpdaniel]

Thanks for the info @josephsavona keep us posted 😀

[jasonkuhrt]

@josephsavona Our team our Prisma would be interested in contributing a solution here. Let me know what potential there is for that, we use result types in our API and so rely on this feature heavily (we're migrating from genql).