Repository navigation
validator: fix list and leaf type conflict checks - #512
Merged
StevenACoffman merged 2 commits intoOct 21, 2026
Merged
StevenACoffman merged 2 commits into
StevenACoffman merged 2 commits into
Conversation
doTypesConflict recursed into list item types before comparing nullability, and the non-null flag of a list lives on the same node as its Elem. So two fields with the same response name returning [Int]! and [Int] (or [[Int]!] and [[Int]]) in mutually exclusive fragments were allowed to merge, even though one can be null and the other cannot. SameResponseShape in the spec checks non-null before list types, and graphql-js reports this as `Fields "list" conflict because they return conflicting types "[Int]!" and "[Int]".` Compare the non-null flags first, at every level of the type. The imported graphql-js cases only compare nullability on named types, so add spec cases for a list, a nested list and a matching list.
StevenACoffman
force-pushed
the
fix/overlapping-list-nullability
branch
from
October 21, 2026 12:17
7b332c2 to
1a35feb
Compare
StevenACoffman
changed the title
doTypesConflict only reported a named-type conflict when both types were scalars or enums, so a scalar field merged with an object field (Int with Item) in mutually exclusive fragments. A leaf has no subfields to compare later, so graphql-js reports a conflict when either type is a leaf. Do the same with IsLeafType(). The imported graphql-js cases never overlap a leaf with a composite type, so add spec cases for both orders, an enum, a list, and two object types that must still merge. While here, fold the list-shape checks into the non-null check and add a doc comment adapted from graphql-js.
StevenACoffman
force-pushed
the
fix/overlapping-list-nullability
branch
from
October 21, 2026 12:20
1a35feb to
2c19a66
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
doTypesConflictlet two kinds of conflicting fields merge in OverlappingFieldsCanBeMerged:ast.Typenode as itsElem, and the item type was recursed into before that flag was compared, so[Int]!merged with[Int](and[[Int]!]with[[Int]]).x: Intmerged withx: Item.{ someBox { ... on A { list } ... on B { list } } } # [Int]! vs [Int] { someBox { ... on A { x } ... on B { x { id } } } } # Int vs Itemgraphql-js rejects both with
they return conflicting types .... Non-null and list wrappers are now compared before recursing, and a conflict is reported when either named type is a leaf (IsLeafType()), matching graphql-js'sisLeafType(type1) || isLeafType(type2).The imported graphql-js cases cover neither, so I added spec cases for both, including cases that must still merge.
I have: