diff --git a/benchmark/validateSubscription-benchmark.js b/benchmark/validateSubscription-benchmark.js new file mode 100644 index 0000000000..359602ba1a --- /dev/null +++ b/benchmark/validateSubscription-benchmark.js @@ -0,0 +1,67 @@ +import { parse } from 'graphql/language/parser.js'; +import { buildSchema } from 'graphql/utilities/buildASTSchema.js'; +import { validate } from 'graphql/validation/validate.js'; + +const subscriptionSchemaSDL = ` + type Email { + from: String + subject: String + asyncSubject: String + unread: Boolean + } + + type Inbox { + total: Int + unread: Int + emails: [Email] + } + + type Query { + inbox: Inbox + } + + type EmailEvent { + email: Email + inbox: Inbox + } + + type Subscription { + importantEmail(priority: Int): EmailEvent + } +`; + +const schema = buildSchema(subscriptionSchemaSDL, { assumeValid: true }); + +const operationAST = parse(` + subscription ( + $priority: Int = 0 + $shouldDefer: Boolean = false + $shouldStream: Boolean = false + $asyncResolver: Boolean = false + ) { + importantEmail(priority: $priority) { + email { + from + subject + ... @include(if: $asyncResolver) { + asyncSubject + } + } + ... @defer(if: $shouldDefer) { + inbox { + emails @include(if: $shouldStream) @stream(if: $shouldStream) + unread + total + } + } + } + } +`); + +export const benchmark = { + name: 'Validate Subscription Operation', + count: 50, + measure() { + validate(schema, operationAST); + }, +}; diff --git a/src/execution/collectFields.ts b/src/execution/collectFields.ts index bc00b413d8..b2176dbeab 100644 --- a/src/execution/collectFields.ts +++ b/src/execution/collectFields.ts @@ -2,7 +2,6 @@ import { AccumulatorMap } from '../jsutils/AccumulatorMap.js'; import type { ObjMap } from '../jsutils/ObjMap.js'; import type { - DirectiveNode, FieldNode, FragmentDefinitionNode, FragmentSpreadNode, @@ -57,8 +56,7 @@ interface CollectFieldsContext { runtimeType: GraphQLObjectType; visitedFragmentNames: Set; hideSuggestions: boolean; - forbiddenDirectiveInstances: Array; - forbidSkipAndInclude: boolean; + shouldIncludeNodeFn: typeof shouldIncludeNode; } /** @@ -78,11 +76,10 @@ export function collectFields( runtimeType: GraphQLObjectType, selectionSet: SelectionSetNode, hideSuggestions: boolean, - forbidSkipAndInclude = false, + shouldIncludeNodeFn = shouldIncludeNode, ): { groupedFieldSet: GroupedFieldSet; newDeferUsages: ReadonlyArray; - forbiddenDirectiveInstances: ReadonlyArray; } { const groupedFieldSet = new AccumulatorMap(); const newDeferUsages: Array = []; @@ -93,15 +90,13 @@ export function collectFields( runtimeType, visitedFragmentNames: new Set(), hideSuggestions, - forbiddenDirectiveInstances: [], - forbidSkipAndInclude, + shouldIncludeNodeFn, }; collectFieldsImpl(context, selectionSet, groupedFieldSet, newDeferUsages); return { groupedFieldSet, newDeferUsages, - forbiddenDirectiveInstances: context.forbiddenDirectiveInstances, }; } @@ -134,8 +129,7 @@ export function collectSubfields( runtimeType: returnType, visitedFragmentNames: new Set(), hideSuggestions, - forbiddenDirectiveInstances: [], - forbidSkipAndInclude: false, + shouldIncludeNodeFn: shouldIncludeNode, }; const subGroupedFieldSet = new AccumulatorMap(); const newDeferUsages: Array = []; @@ -177,17 +171,18 @@ function collectFieldsImpl( runtimeType, visitedFragmentNames, hideSuggestions, + shouldIncludeNodeFn, } = context; for (const selection of selectionSet.selections) { switch (selection.kind) { case Kind.FIELD: { if ( - !shouldIncludeNode( - context, + !shouldIncludeNodeFn( selection, variableValues, fragmentVariableValues, + hideSuggestions, ) ) { continue; @@ -201,11 +196,11 @@ function collectFieldsImpl( } case Kind.INLINE_FRAGMENT: { if ( - !shouldIncludeNode( - context, + !shouldIncludeNodeFn( selection, variableValues, fragmentVariableValues, + hideSuggestions, ) || !doesFragmentConditionMatch(schema, selection, runtimeType) ) { @@ -247,11 +242,11 @@ function collectFieldsImpl( if ( visitedFragmentNames.has(fragName) || - !shouldIncludeNode( - context, + !shouldIncludeNodeFn( selection, variableValues, fragmentVariableValues, + hideSuggestions, ) ) { continue; @@ -348,25 +343,21 @@ function getDeferUsage( * directives, where `@skip` has higher precedence than `@include`. */ function shouldIncludeNode( - context: CollectFieldsContext, node: FragmentSpreadNode | FieldNode | InlineFragmentNode, variableValues: VariableValues, fragmentVariableValues: VariableValues | undefined, + hideSuggestions: boolean, ): boolean { 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, + hideSuggestions, ) : undefined; if (skip?.if === true) { @@ -376,17 +367,13 @@ function shouldIncludeNode( 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, + hideSuggestions, ) : undefined; if (include?.if === false) { diff --git a/src/validation/rules/SingleFieldSubscriptionsRule.ts b/src/validation/rules/SingleFieldSubscriptionsRule.ts index 5d8bdb615d..d47d5fcdc7 100644 --- a/src/validation/rules/SingleFieldSubscriptionsRule.ts +++ b/src/validation/rules/SingleFieldSubscriptionsRule.ts @@ -2,10 +2,19 @@ import type { ObjMap } from '../../jsutils/ObjMap.js'; import { GraphQLError } from '../../error/GraphQLError.js'; -import type { FieldNode, OperationDefinitionNode } from '../../language/ast.js'; +import type { + DirectiveNode, + FieldNode, + OperationDefinitionNode, +} from '../../language/ast.js'; import { Kind } from '../../language/kinds.js'; import type { ASTVisitor } from '../../language/visitor.js'; +import { + GraphQLIncludeDirective, + GraphQLSkipDirective, +} from '../../type/directives.js'; + import type { FieldDetailsList, FragmentDetails, @@ -46,16 +55,30 @@ export function SingleFieldSubscriptionsRule( fragments[definition.name.value] = { definition }; } } - const { groupedFieldSet, forbiddenDirectiveInstances } = - collectFields( - schema, - fragments, - variableValues, - subscriptionType, - node.selectionSet, - context.hideSuggestions, - true, - ); + const forbiddenDirectiveInstances: Array = []; + const { groupedFieldSet } = collectFields( + schema, + fragments, + variableValues, + subscriptionType, + node.selectionSet, + context.hideSuggestions, + (selection) => { + for (const directive of [ + GraphQLSkipDirective, + GraphQLIncludeDirective, + ]) { + const directiveNode = selection.directives?.find( + (d) => d.name.value === directive.name, + ); + if (directiveNode !== undefined) { + forbiddenDirectiveInstances.push(directiveNode); + return false; + } + } + return true; + }, + ); if (forbiddenDirectiveInstances.length > 0) { context.reportError( new GraphQLError(