Skip to content

Union Type fields of same name with differing types are incorrectly considered invalid #2781

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jefflewis opened this issue Oct 18, 2019 · 10 comments
Labels
core Related to codegen core/cli

Comments

@jefflewis
Copy link
Contributor

jefflewis commented Oct 18, 2019

Describe the bug
The codegen should allow for the differing type of name between the to union types.

To Reproduce
See example at codesandbox.

https://codesandbox.io/s/graphql-codegen-issue-template-hxp3g

  1. My GraphQL schema:
type Query {
  ListItems: [ItemUnion]
}

union ItemUnion = Product | ProductVariant

type Product {
  id: ID!
  name: String!
}

type ProductVariant {
  id: ID!
  name: String
}
  1. My GraphQL operations:
query ListItems {
    ListItems {
        ... on Product {
            id
            name
        }

        ... on ProductVariant {
            id
            name
        }
    }
}
  1. My codegen.yml config file:
schema: schema.graphql
documents: document.graphql
generates:
  types.ts:
    plugins:
      - typescript
      - typescript-operations

Expected behavior
Successful generation of types.

Environment:

  • codegen v. 1.8.1
@dotansimha
Copy link
Owner

@jefflewis I'm not sure what causing it, because it seems like the error is coming from graphql.js validation. I need to check what's really causing it.

@dotansimha dotansimha added bug core Related to codegen core/cli labels Oct 29, 2019
@dotansimha
Copy link
Owner

Ok so the actual error is:

GraphQLDocumentError: Fields "name" conflict because they return conflicting types String! and String. Use different aliase
s on the fields to fetch both if this was intentional.

Which makes sense, because name is defined in a different way in those types, and it means that the return value couldn't really tell if it's nullable or not.

In Product.name defined as String! and ProductVariant.name defined as String.

GraphQL validation fails and it makes sense, I think the suggestion of GraphQL is correct, you should add type aliases:

query ListItems {
    ListItems {
        ... on Product {
            id
            productName: name
        }

        ... on ProductVariant {
            id
            name
        }
    }
}

Or, change your schema to match that.

Also, if ProductVariant should reflect a possible implementation of Product - I suggest to use interfaces and not unions, this way the implementing type will get enforced to implement the parent interface correctly.

@jefflewis
Copy link
Contributor Author

@dotansimha Are you suggesting that having differing field definitions for types in union is invalid GraphQL? If so, can you point me to that part of the spec. If not, then I'm lost as to how this is invalid?

I can select these from my API with no issues. and I can use the resulting JSON to create types manually.

Also, what do you mean be implementing them as interfaces instead of unions? The GQL schema type is that of union since the API may respond with one of a number of types for a given field (given an item, it can be a Product or ProductVariant).

@bartlangelaan
Copy link

I also encountered this issue, which is why I digged into it.

The formal specification that describes this, is: https://graphql.github.io/graphql-spec/draft/#sec-Field-Selection-Merging. The last paragraph states that even if two fields can never be encountered at the same time they can still not appear on the same object.

That specific part was introduced by this merge request: graphql/graphql-spec#162

This issue has some recent comments about the validation rule: graphql/graphql-js#53

@bombillazo
Copy link

Dang, just stumbled upon this issue while trying to use fragments on a union typed field with types that share a field name but each is its own type:

fragment myFrangment on Entity {
  ... on Player{
    type
    ...playerFragment
  }
  ... on Coach{
    type
    ...coachFragment
  }
}
GraphQLDocumentError: Fields "type" conflict because they return conflicting types "PlayerType" and "CoachType".
Use different aliases on the fields to fetch both if this was intentional.

Any way to ignore this error?

@aturkewi
Copy link

I am also stumbling onto this. I don't mind using the alias, but the problem I then have is that they generated Typescript type does not have the alias and now I need to do custom typing to fix it.

e.g. from the example above:

query ListItems {
    ListItems {
        ... on Product {
            id
            productName: name
        }

        ... on ProductVariant {
            id
            name
        }
    }
}

the Product type in the generated typescript still has name (because the type is generated from the SDL) so now I need to manually re-set that part of the data as a custom type:

type CustomProductType = {
  productName: String
} & GeneratedProductType

@Siilwyn
Copy link

Siilwyn commented Feb 7, 2024

This is really common when fetching GraphQL types from a CMS.
For example, two blocks of content where the title field is required in one and optional in the other:

{
  page {
    bodyBlocks {
      ... on ImageBlockRecord {
        title
      }
      ... on TextBlockRecord {
        title
      }
    }
  }
}

Is there any workaround that's not alias? Because changing the field name is quite messy, duplicating the 'context/block' name in the field, e.g: textBlockTitle.

@thomasverleye
Copy link

thomasverleye commented Mar 29, 2024

Found out that you can skip this validation rule by adding this to your codegen.ts config file:

  config: {
    skipDocumentsValidation: {
      ignoreRules: ['OverlappingFieldsCanBeMergedRule'],
    },
  },

This is a codegen config setting of which you can select specific rules to ignore :-)

@abernier
Copy link

abernier commented Apr 2, 2024

I tried enabling this OverlappingFieldsCanBeMergedRule as you mentioned @thomasverleye but still have the

Fields "title" conflict because they return conflicting types "String!" and "String". Use different aliases on the fields to fetch both if this was intentional.

have you managed to take this option into account? I saw you provided a https://codesandbox.io/p/devbox/vibrant-lake-y7dmnw but even in this one, I can see the error despite of the config...

Thank you

@thomasverleye
Copy link

@abernier Yes it does not work in that example but later on I discovered that you can apply that config in the config
attribute which is used for the plugins used (pretty confusing, I know).

Anyway, try adding this skipDocumentsValidation the root of the config file:

  config: {
    skipDocumentsValidation: {
      ignoreRules: ['OverlappingFieldsCanBeMergedRule'],
    },
  },

Do note the config, instead of adding skipDocumentsValidation directly to the root of the config file:

    skipDocumentsValidation: {
      ignoreRules: ['OverlappingFieldsCanBeMergedRule'],
    },

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Related to codegen core/cli
Projects
None yet
Development

No branches or pull requests

8 participants