Skip to content
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

Fix union render with no UnionMemberTypes #1085

Merged

Conversation

kdawgwilk
Copy link
Contributor

@kdawgwilk kdawgwilk commented Jul 7, 2021

Adds a unit test for a case that wasnt being tested for union with no types and fixes the issue. Spec says that the UnionMemberTypes are optional https://spec.graphql.org/draft/#UnionTypeDefinition

Resolves #1084

@kdawgwilk kdawgwilk changed the title Fix union render with no types Fix union render with no UnionMemberTypes Jul 7, 2021
@binaryseed
Copy link
Contributor

Hmm, further down in the spec, I see this:

A Union type must include one or more unique member types.

https://spec.graphql.org/draft/#sec-Unions.Type-Validation

@kdawgwilk
Copy link
Contributor Author

Interesting, I may be able to just not produce this type if there are no UnionMemberTypes but not sure what the right call here is

@kdawgwilk
Copy link
Contributor Author

If we want to go that direction we should prob add validation for that so that those unions won't be allowed in absinthe if they have no NamedTypes

@binaryseed
Copy link
Contributor

I asked around about this, and the SDL syntax can define a "partial" type which can then be extended later... But eventually it needs to have NamedTypes to be valid...

@benwilson512
Copy link
Contributor

Yeah I think that's the right call @binaryseed. I think this is good to go then?

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

Successfully merging this pull request may close these issues.

SDL output for union with no types is invalid
4 participants