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

RFC: __typename should be valid at subscription root #806

Closed
wants to merge 1 commit into from

Conversation

benjie
Copy link
Member

@benjie benjie commented Dec 5, 2020

This is an alternative solution to #776 wherein __typename is explicitly allowed inspired by @IvanGoncharov's comment on that PR.

Description of issue

__typename does not return an event stream, so it does not make sense to allow for it to be the source stream in a GraphQL subscription operation. As currently specified, the following query passes validation, but it should always produce an error since the ResolveFieldEventStream algorithm cannot resolve a subscription resolver for __typename:

subscription {
  __typename
}

Separately; it's valid to add __typename to any selection set in any GraphQL operation except the root selection set (including fragments) on a Subscription operation. This exclusion complicates life for various GraphQL tooling; it's desirable that this (currently invalid) GraphQL operation be valid:

subscription sub {
  newMessage {
    body
    sender
  }
  __typename
}

The current GraphQL algorithm for subscriptions operates in two steps; first it resolves the "source stream" from the root field that will generate the subscription events, and then when an event is received it executes the entire operation (NOTE: not just the selection set of the source stream's field, but the entire selection set of the operation) using this event as the initialValue. As such, __typename could be valid in the root selection set so long as there is exactly one field capable of providing the source stream.

Solution outline

  • Change validation for subscription operations so that instead of saying the root selection set must include exactly one field, it's now exactly one non-introspection field.
  • Change CreateSourceEventStream such that it uses this non-introspection field as the event source (i.e. so that it ignores introspection fields).

@benjie
Copy link
Member Author

benjie commented Jan 6, 2022

#776 was merged and is the direction we decided to go 👍

@benjie benjie closed this Jan 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💭 Strawman (RFC 0) RFC Stage 0 (See CONTRIBUTING.md)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants