dotansimha / graphql-code-generator

A tool for generating code based on a GraphQL schema and GraphQL operations (query/mutation/subscription), with flexible support for custom plugins.
https://the-guild.dev/graphql/codegen/
MIT License
10.88k stars 1.34k forks source link

enumValues with exported type break type safety #5122

Open Shahor opened 4 years ago

Shahor commented 4 years ago

Describe the bug

When specifying enumValues and pointing to an exported type directly, the exported type could not match the enum at all.

This means the schema isn't enforced on the types imported and it breaks type safety.

To Reproduce Steps to reproduce the behavior:

  1. My GraphQL schema:
enum Foobar {
  FOO
  BAR
}

type Something {
  someArg: String!
  someFoobar: Foobar!
}

input SomeInput {
  foobar: Foobar!
}

type Query {
  show(input: SomeInput!): String
}
  1. My codegen.yml config file:
schema: ./**/*.graphql
generates:
  ./schema-types.ts:
    plugins:
      - typescript
    config:
      namingConvention:
        enumValues: keep
      typesPrefix: "Graphql"
      enumPrefix: false
      nonOptionalTypename: false
      avoidOptionals: false
      skipTypename: true
      scalars:
        Date: "string"
        Cursor: "string"
        JSON: "{ [key: string]: any }"
      enumValues:
        Foobar: ./index#Foobar
  1. Sandbox link

-> https://codesandbox.io/s/exciting-haibt-xpoln?file=/codegen.yml

Expected behavior

As you can see in the generated types, Foobar is properly used from the exported existing type, itself imported from index#Foobar.

Because we delegated the enum to a re-export of a pre-defined enum, the schema is no longer the source of truth here, which means that I can have an enum that partially (or not at all) implements an enum defined in the schema.

Could the generator make an extra check on this to see if all schema-defined keys are present in the code-defined enum?

That would give a better level of type safety.

Environment:

See Sandbox: https://codesandbox.io/s/exciting-haibt-xpoln?file=/index.ts

QuestionAndAnswer commented 2 years ago

@dotansimha, @charlypoly Seems like this observation might be useful. I also kinda facing type safety issues with generated enums signature, and what I found, and can't answer on, is why this (from generated .ts file)

export type EnumResolverSignature<T, AllowedValues = any> = { [key in keyof T]?: AllowedValues };

has keys of a resolver defined as optional? I manually removed optional modifier, and TS started to react on the case similar to @Shahor case, as well as it started to react to cases like enums names missmatch.

Idk what else might happen if we would just remove optional modifier, but it seems like it's a root cause. Why it was added in the first place?? I took a look at original PR when enumsValues where implemented and found nothing which could've helped me to answer that.

I'd create a PR if we'd come to agreement that this is a relatively safe change.