Field extensions: spec edits - #1196
Conversation
✅ Deploy Preview for graphql-spec-draft ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
martinbonnin
left a comment
martinbonnin
left a comment
There was a problem hiding this comment.
Thanks for looking into this! Few comments below
martinbonnin
changed the title
martinbonnin
changed the title
…text on extending interfaces
mjmahone
left a comment
mjmahone
left a comment
There was a problem hiding this comment.
So the biggest thing I'd say is that we should be explicit that we're making extend mean merge: at each level it's OK for something to be missing in the extend, in which case that means we use the original value if any, but if it exists on both it must be exactly matching.
| ``` | ||
|
|
||
| ```graphql example | ||
| extend type Query { |
There was a problem hiding this comment.
in this world, extend is a keyword for merge?
| two fields may share the same name. | ||
| 2. If a field with the same name exists on the previous Object type: | ||
| 1. The field type must match the previous definition exactly. | ||
| 2. The field description must match the previous definition exactly. |
There was a problem hiding this comment.
or be empty. It would be really annoying to not be able to extend a field because I chose not to add the description.
There was a problem hiding this comment.
Agree this should be the intended outcome.
| 3. Any fields of an Interface type extension must not be already defined on the | ||
| previous Interface type. |
There was a problem hiding this comment.
This line now contradicts the block added just above it. Rule 2 ("For each field of an Interface type extension: … 2. If a field with the same name exists on the previous Interface type: …") permits redeclaring an existing field with a matching definition, but this rule still forbids any field "already defined on the previous Interface type."
The Object Extensions section removed its equivalent line as part of this change; the interface section looks like it should too, otherwise field extensions are simultaneously allowed (rule 2) and disallowed (this rule) on interfaces.
Drafted by Claude (Anthropic AI assistant).
There was a problem hiding this comment.
Thanks all for the thorough review.
As it stands, it looks to me like this proposal needs to be adressed more globally as we refine type extension vs type merging and make consistent and predictable rules.
With @egoodwinx approval, I'll close this one so we can focus on other areas of the spec.
I'll keep GAP-4 around as a reference for what's implemented in Apollo Kotlin in case other implementers want to try it out.
Edit: got confirmation out of the band from @egoodwinx that it's ok to close this one.
| 3. Any fields of an Interface type extension must not be already defined on the | ||
| previous Interface type. |
6 participants
Uh oh!
There was an error while loading. Please reload this page.