Minor editorial tweaks following the merge of #1039#1175
Conversation
For `type User { name: String }`, `User.name` is a field of an object
type (`User`). Clarify that we mean the return type of the field, not
the type to which it belongs.
benjie
added
🚀 Next Stage?
✅ Deploy Preview for graphql-spec-draft ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| Valid operations must supply a _selection set_ for every field whose return type | ||
| is an object type, so this operation is not valid: |
There was a problem hiding this comment.
User.name is a "field of an object type", but should not have a selection set.
| During execution, the simultaneous execution of fields with the same response | ||
| name is accomplished by {CollectSubfields()} before execution. | ||
| name is accomplished by performing {CollectSubfields()} before their execution. |
There was a problem hiding this comment.
During execution, ... before execution. did not read well.
| root type must be known, as well as whether the fields must be executed in a | ||
| series, or normally by executing all fields in parallel (see |
There was a problem hiding this comment.
"each field must be executed serially" sounds like the serial-ness is a property of the fields own execution (e.g. of its resolver and sub selection set), rather than it's contextual execution relative to its sibling fields. Minor tweak for clarity.
| ### Field Collection | ||
|
|
||
| Before execution, each _selection set_ is converted to a _collected fields map_ | ||
| by calling {CollectFields()} by collecting all fields with the same response |
There was a problem hiding this comment.
Merely deleted by calling {CollectFields()} since the algorithms (including {CollectSubfields()} too) are mentioned in next paragraph, and by ... by ... read unpleasantly.
| with the next {selection} in {selectionSet}. | ||
| - Let {fragmentSelectionSet} be the top-level selection set of {fragment}. | ||
| - Let {fragmentCollectedFieldMap} be the result of calling | ||
| - Let {fragmentCollectedFieldsMap} be the result of calling |
There was a problem hiding this comment.
+s throughout to match existing collectedFieldsMap term
There was a problem hiding this comment.
ah thanks - that was my intent
leebyron
left a comment
leebyron
left a comment
There was a problem hiding this comment.
beautiful - thanks for the careful read and spot check fixes
2 participants
Follow up to:
ExecuteSelectionSetwithExecuteCollectedFields#1039@leebyron noted:
I like the new edits, they're clear and achieve the goals of the PR! Here's some minor tweaks to improve the end result.
Recommendation: review on a commit-by-commit basis, each commit is self-contained.