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.85k stars 1.33k forks source link

TypeScript - Bad syntax generated due to private interface #5074

Closed ArrayKnight closed 3 years ago

ArrayKnight commented 4 years ago

Describe the bug When a public query would return a private interface (due to a mistake in the public schema), the Codegen CLI "successfully" completes, but creates a type definition file with invalid (syntactically incorrect) types/interfaces (Maybe<Array<Maybe<>>>).

To Reproduce Steps to reproduce the behavior:

  1. My GraphQL schema:
type Foo = { ... } # Private type which should be public but isn't (setting in CMSs public GraphQL schema)

type Query = {
    foos(id: String): [Foo]
}
  1. My GraphQL operations:
query Foos($id: String) {
    foos(id: $id) { ... }
}
  1. My codegen.yml config file:
schema: ${GRAPHQL_SCHEMA_URL}
documents:
    - 'src/**/*.{ts,tsx}'
    - '!src/graphql/*'
generates:
    src/graphql/index.tsx:
        plugins:
            - typescript
            - typescript-operations
            - fragment-matcher
        config:
            avoidOptionals: true
            nonOptionalTypename: true
  1. My src/graphql/index.tsx:
export type FoosQuery = (
    { __typename: 'Query' } &
    { foos: Maybe<Array<Maybe<>>> } // <-- Missing interface due to private type, would be Pick<Foo, '...'> if it were public
)

Expected behavior Codegen CLI should throw an error detailing that there is a missing/private type that it can't write and then not output a file

Environment:

ArrayKnight commented 4 years ago

Another possibility would be to write unknown in place of the missing type. Though a warning in CLI would also be nice

dotansimha commented 3 years ago

Thank you for reporting this @ArrayKnight . I'm not sure I fully understand this concept of private/public here. If an interface isn't being published as part of the schema, it should fail in codegen due to a missing type during the validation phase. Can you please share a live reproduction of this issue?

ArrayKnight commented 3 years ago

I don't have any immediate time to setup an environment, but I can describe it in a more specific example:

We're using Craft CMS as a headless CMS, so we bought a Pro license, enabled the GraphQL interface and got to modeling our data.

  1. Create a top level Entry type (Page)
  2. Go to the GraphQL Public Schema
  3. Notice that Assets is unchecked (private, not part of the public schema)
  4. Check Entries > Section - Page, hit Save
  5. Add a field (images) to Page of the type Assets

Now, when you do a schema codegen you'll end up with something like:

type Query {
    entry(
        # ...removed for clarity
    ): EntryInterface

    entries(
        # ...
    ): [EntryInterface]
}

type AssetInterface { # Oddly present despite not being part of the public schema
    #...
}

type page_page_Entry implements ElementInterface & EntryInterface {
    images(
        # ...
    ): [AssetInterface]
}

So, then you would write a query:

query Page($uid: [String]){
    entry(uid: $uid) {
        ... on page_page_Entry {
            images {
                url
            }
        }
    }
}

fragment Section on section_BlockType {
    tiles {
        ... on tiles_BlockType {
            tileImage { # also an assets field
                uid
                url
            }
        }
    }
}

And what's interesting is that you will find AssetInterface as a top level type and you'll even find it referenced within top level types that have asset fields, but when you have a fragment that references that interface you get invalid syntax:

export type AssetInterface = {
    // ...
}

export type Page_Page_Entry = ElementInterface & EntryInterface & {
    // ...

    images: Maybe<Array<Maybe<AssetInterface>>>;
}

export SectionFragment = (
    { __typename: 'section_BlockType ' } & 
    { tiles: Maybe<Array<Maybe<(
        { __typename: 'tiles_BlockType' } & 
        { tileImage: Maybe<Array<Maybe<>>> } // <-- Invalid syntax
    )}
)
ArrayKnight commented 3 years ago

To remedy this, I had to go to the GraphQL Public Schema, check Assets, and hit save.

dotansimha commented 3 years ago

Closing, @ArrayKnight . This was related to missing config right?

ArrayKnight commented 3 years ago

@dotansimha I disagree that this should be closed.

I can see the argument that this is a setting in Craft CMS that causes this issue, however, I feel like the codegen tool should handle this case by either throwing an error in CLI or replacing the missing type with unknown and warning in the CLI.

dotansimha commented 3 years ago

I see, but I'm not familiar with CraftCMS, or with the exact setup that you have, and you are not able to create a complete reproduction. I don't have any tools to reproduce or investigate it. I'm willing to try to solve that, but I will need your cooperation... Please share a reproduction and then I'll re-open this.