Skip to main content

Prevent @skip and @include on root subscription selection set

At a glance​

Timeline​


Following on from #776 and based on the discussion in the GraphQL.js pull request I'm still uncomfortable with the single root field validation rule for subscription operations.

Currently the "single root field" algorithm sets variableValues to be the empty set and then calls CollectFields. To see the issue with this, consider the following subscription operation:

type Subscription {
mySubscriptionField1: Int
mySubscriptionField2: Int
mySubscriptionField3: Int
}

subscription TwoFieldsByDefault($bool: Boolean = true) {
mySubscriptionField1 @skip(if: $bool)
mySubscriptionField2 @include(if: $bool)
mySubscriptionField3 @include(if: $bool)
}

A call to CollectFields passing an empty map for {variableValues} will result in all selections with @skip directives being considered, and all selections with the @include directive being skipped. So according to the current validation rule, this operation is valid - it only has one field mySubscriptionField1 (which is not an introspection field) in the root selection set.

However, if you pass no variables at runtime, the runtime CollectFields() called during CreateSourceEventStream for the subscription operation will result in groupedFieldSet containing two selections (mySubscriptionField2 and mySubscriptionField3). This will result in a request error being raised: If groupedFieldSet does not have exactly one entry, raise a request error.

This catches the invalid operation at runtime rather than validation time, giving a false sense of security about the validity of a GraphQL operation that may fail by default.

No other validation rule in the entire of Section 5 references variableValues or calls CollectFields; but we already know that the root subscription selection set is very special (it's been discussed many times at the GraphQL Spec WG).

This PR proposes that @skip and @include are forbidden on root subscription selection sets.

🚨 This is a breaking change since previously valid operations such as the following will now be marked as invalid:

subscription ThisIsFineDotJpg($bool: Boolean = true) {
mySubscriptionField1 @skip(if: $bool)
mySubscriptionField2 @include(if: $bool)
}