diff --git a/src/execution/collectFields.ts b/src/execution/collectFields.ts index bdca1ba9e0..bc00b413d8 100644 --- a/src/execution/collectFields.ts +++ b/src/execution/collectFields.ts @@ -2,6 +2,7 @@ import { AccumulatorMap } from '../jsutils/AccumulatorMap.js'; import type { ObjMap } from '../jsutils/ObjMap.js'; import type { + DirectiveNode, FieldNode, FragmentDefinitionNode, FragmentSpreadNode, @@ -23,7 +24,11 @@ import { typeFromAST } from '../utilities/typeFromAST.js'; import type { GraphQLVariableSignature } from './getVariableSignature.js'; import type { VariableValues } from './values.js'; -import { getDirectiveValues, getFragmentVariableValues } from './values.js'; +import { + experimentalGetArgumentValues, + getDirectiveValues, + getFragmentVariableValues, +} from './values.js'; export interface DeferUsage { label: string | undefined; @@ -52,6 +57,8 @@ interface CollectFieldsContext { runtimeType: GraphQLObjectType; visitedFragmentNames: Set; hideSuggestions: boolean; + forbiddenDirectiveInstances: Array; + forbidSkipAndInclude: boolean; } /** @@ -71,9 +78,11 @@ export function collectFields( runtimeType: GraphQLObjectType, selectionSet: SelectionSetNode, hideSuggestions: boolean, + forbidSkipAndInclude = false, ): { groupedFieldSet: GroupedFieldSet; newDeferUsages: ReadonlyArray; + forbiddenDirectiveInstances: ReadonlyArray; } { const groupedFieldSet = new AccumulatorMap(); const newDeferUsages: Array = []; @@ -84,10 +93,16 @@ export function collectFields( runtimeType, visitedFragmentNames: new Set(), hideSuggestions, + forbiddenDirectiveInstances: [], + forbidSkipAndInclude, }; collectFieldsImpl(context, selectionSet, groupedFieldSet, newDeferUsages); - return { groupedFieldSet, newDeferUsages }; + return { + groupedFieldSet, + newDeferUsages, + forbiddenDirectiveInstances: context.forbiddenDirectiveInstances, + }; } /** @@ -119,6 +134,8 @@ export function collectSubfields( runtimeType: returnType, visitedFragmentNames: new Set(), hideSuggestions, + forbiddenDirectiveInstances: [], + forbidSkipAndInclude: false, }; const subGroupedFieldSet = new AccumulatorMap(); const newDeferUsages: Array = []; @@ -166,7 +183,12 @@ function collectFieldsImpl( switch (selection.kind) { case Kind.FIELD: { if ( - !shouldIncludeNode(selection, variableValues, fragmentVariableValues) + !shouldIncludeNode( + context, + selection, + variableValues, + fragmentVariableValues, + ) ) { continue; } @@ -180,6 +202,7 @@ function collectFieldsImpl( case Kind.INLINE_FRAGMENT: { if ( !shouldIncludeNode( + context, selection, variableValues, fragmentVariableValues, @@ -224,7 +247,12 @@ function collectFieldsImpl( if ( visitedFragmentNames.has(fragName) || - !shouldIncludeNode(selection, variableValues, fragmentVariableValues) + !shouldIncludeNode( + context, + selection, + variableValues, + fragmentVariableValues, + ) ) { continue; } @@ -320,26 +348,47 @@ function getDeferUsage( * directives, where `@skip` has higher precedence than `@include`. */ function shouldIncludeNode( + context: CollectFieldsContext, node: FragmentSpreadNode | FieldNode | InlineFragmentNode, variableValues: VariableValues, fragmentVariableValues: VariableValues | undefined, ): boolean { - const skip = getDirectiveValues( - GraphQLSkipDirective, - node, - variableValues, - fragmentVariableValues, + const skipDirectiveNode = node.directives?.find( + (directive) => directive.name.value === GraphQLSkipDirective.name, ); + if (skipDirectiveNode && context.forbidSkipAndInclude) { + context.forbiddenDirectiveInstances.push(skipDirectiveNode); + return false; + } + const skip = skipDirectiveNode + ? experimentalGetArgumentValues( + skipDirectiveNode, + GraphQLSkipDirective.args, + variableValues, + fragmentVariableValues, + context.hideSuggestions, + ) + : undefined; if (skip?.if === true) { return false; } - const include = getDirectiveValues( - GraphQLIncludeDirective, - node, - variableValues, - fragmentVariableValues, + const includeDirectiveNode = node.directives?.find( + (directive) => directive.name.value === GraphQLIncludeDirective.name, ); + if (includeDirectiveNode && context.forbidSkipAndInclude) { + context.forbiddenDirectiveInstances.push(includeDirectiveNode); + return false; + } + const include = includeDirectiveNode + ? experimentalGetArgumentValues( + includeDirectiveNode, + GraphQLIncludeDirective.args, + variableValues, + fragmentVariableValues, + context.hideSuggestions, + ) + : undefined; if (include?.if === false) { return false; } diff --git a/src/validation/__tests__/SingleFieldSubscriptionsRule-test.ts b/src/validation/__tests__/SingleFieldSubscriptionsRule-test.ts index 7e0a227d07..7e301b3720 100644 --- a/src/validation/__tests__/SingleFieldSubscriptionsRule-test.ts +++ b/src/validation/__tests__/SingleFieldSubscriptionsRule-test.ts @@ -286,6 +286,48 @@ describe('Validate: Subscriptions with single field', () => { ]); }); + it('fails with @skip or @include directive', () => { + expectErrors(` + subscription RequiredRuntimeValidation($bool: Boolean!) { + newMessage @include(if: $bool) { + body + sender + } + disallowedSecondRootField @skip(if: $bool) + } + `).toDeepEqual([ + { + message: + 'Subscription "RequiredRuntimeValidation" must not use `@skip` or `@include` directives in the top level selection.', + locations: [ + { line: 3, column: 20 }, + { line: 7, column: 35 }, + ], + }, + ]); + }); + + it('fails with @skip or @include directive in anonymous subscription', () => { + expectErrors(` + subscription ($bool: Boolean!) { + newMessage @include(if: $bool) { + body + sender + } + disallowedSecondRootField @skip(if: $bool) + } + `).toDeepEqual([ + { + message: + 'Anonymous Subscription must not use `@skip` or `@include` directives in the top level selection.', + locations: [ + { line: 3, column: 20 }, + { line: 7, column: 35 }, + ], + }, + ]); + }); + it('skips if not subscription type', () => { const emptySchema = buildSchema(` type Query { diff --git a/src/validation/rules/SingleFieldSubscriptionsRule.ts b/src/validation/rules/SingleFieldSubscriptionsRule.ts index 02948fc895..5d8bdb615d 100644 --- a/src/validation/rules/SingleFieldSubscriptionsRule.ts +++ b/src/validation/rules/SingleFieldSubscriptionsRule.ts @@ -23,7 +23,8 @@ function toNodes(fieldDetailsList: FieldDetailsList): ReadonlyArray { * Subscriptions must only include a non-introspection field. * * A GraphQL subscription is valid only if it contains a single root field and - * that root field is not an introspection field. + * that root field is not an introspection field. `@skip` and `@include` + * directives are forbidden. * * See https://spec.graphql.org/draft/#sec-Single-root-field */ @@ -45,14 +46,27 @@ export function SingleFieldSubscriptionsRule( fragments[definition.name.value] = { definition }; } } - const { groupedFieldSet } = collectFields( - schema, - fragments, - variableValues, - subscriptionType, - node.selectionSet, - context.hideSuggestions, - ); + const { groupedFieldSet, forbiddenDirectiveInstances } = + collectFields( + schema, + fragments, + variableValues, + subscriptionType, + node.selectionSet, + context.hideSuggestions, + true, + ); + if (forbiddenDirectiveInstances.length > 0) { + context.reportError( + new GraphQLError( + operationName != null + ? `Subscription "${operationName}" must not use \`@skip\` or \`@include\` directives in the top level selection.` + : 'Anonymous Subscription must not use `@skip` or `@include` directives in the top level selection.', + { nodes: forbiddenDirectiveInstances }, + ), + ); + return; + } if (groupedFieldSet.size > 1) { const fieldDetailsLists = [...groupedFieldSet.values()]; const extraFieldDetailsLists = fieldDetailsLists.slice(1);