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

Distinguish between executing (a selection set) vs processing (a response) #1154

Open
wants to merge 4 commits into
base: benjie/incremental-common
Choose a base branch
from

Conversation

martinbonnin
Copy link
Contributor

This PR is based on #1039 ("Replace ExecuteSelectionSet with ExecuteGroupedFieldSet")

See context in #1152 (comment) and #894


This PR clarifies that execution is for selection sets and fields. Requests and operations are "processed" instead.

This is especially important for subscription that will execute their root selection set multiple times. It's also in line with variable coercion errors being "request errors" and not "execution errors"

@martinbonnin martinbonnin changed the title Clarify execution Distinguish executing (a selection set) vs processing (a response) Mar 27, 2025
@martinbonnin martinbonnin changed the title Distinguish executing (a selection set) vs processing (a response) Distinguish between executing (a selection set) vs processing (a response) Mar 27, 2025
Copy link
Contributor

@mjmahone mjmahone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this makes the difference between "processing" stage vs "execution" stage much more clear, and I like this pull-apart.

@@ -8,43 +8,49 @@ A GraphQL service generates a response from a request via execution.
- {document}: A {Document} which must contain GraphQL {OperationDefinition} and
may contain {FragmentDefinition}.
- {operationName} (optional): The name of the Operation in the Document to
execute.
process.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imho we should leave this as execute

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.

4 participants