Assert matching response shapes in fields_will_merge
Oops, it seems like one part of FieldsWillMerge was missing. Adding it now might break some queries that used to work, so I may have to ensure there's a way to opt out of this change.
From the spec:
SameResponseShape(fieldA, fieldB)
- Let typeA be the return type of fieldA.
- Let typeB be the return type of fieldB.
- If typeA or typeB is Non-Null:
- If typeA or typeB is nullable, return false.
- Let typeA be the nullable type of typeA.
- Let typeB be the nullable type of typeB.
- If typeA or typeB is List:
- If typeA or typeB is not List, return false.
- Let typeA be the item type of typeA.
- Let typeB be the item type of typeB.
- Repeat from step 3.
- If typeA or typeB is Scalar or Enum:
- If typeA and typeB are the same type return true, otherwise return false.
https://spec.graphql.org/draft/#SameResponseShape()
In GraphQL-JS:
https://github.com/graphql/graphql-js/blob/f201681bf806a2c46c4ee8b2533287421327a302/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts#L691-L718
https://github.com/graphql/graphql-js/blob/a24a9f35b876bdd0d5050eca34d3020bd0db9a29/src/validation/tests/OverlappingFieldsCanBeMergedRule-test.ts#L738-L767
TODO:
- [x] Does this make other tests fail?
- One now-removed test was added here: https://github.com/rmosolgo/graphql-ruby/pull/2004 I think it was a test for code that doesn't exist anymore but ... will it cause a bug to make that fail?
- [ ] Is this too big of a change to make at this point?
- [ ] What about people who depend on this check not being here?
- [ ] Performance implications?
Adding it now might break some queries that used to work, so I may have to ensure there's a way to opt out of this change.
Unfortunately, yes... this is a breaking change that will start rejecting queries that used to be accepted. Probably the smoothest path to reaching spec would be to make this an opt-in setting on GraphQL::Schema that is off by default for now and activates in the next major release.