From d9bc448fd988fae06fc8c619c36d8ed377ef31ee Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Sun, 25 Aug 2024 15:17:20 +0300 Subject: [PATCH] suggestions for execution (#11) * introduce internal getVariableSignature utility now extracted also to graphql-js PR, see https://github.com/graphql/graphql-js/pull/4175 * execution suggestions --- src/execution/collectFields.ts | 121 +++++++------- src/execution/execute.ts | 42 +++-- src/execution/values.ts | 152 +++++------------- src/utilities/getVariableSignature.ts | 44 +++++ src/utilities/valueFromAST.ts | 39 ++++- .../rules/SingleFieldSubscriptionsRule.ts | 15 +- 6 files changed, 203 insertions(+), 210 deletions(-) create mode 100644 src/utilities/getVariableSignature.ts diff --git a/src/execution/collectFields.ts b/src/execution/collectFields.ts index fe43499698..68261c7194 100644 --- a/src/execution/collectFields.ts +++ b/src/execution/collectFields.ts @@ -22,33 +22,44 @@ import { } from '../type/directives.js'; import type { GraphQLSchema } from '../type/schema.js'; +import type { GraphQLVariableSignature } from '../utilities/getVariableSignature.js'; import { typeFromAST } from '../utilities/typeFromAST.js'; -import { getArgumentValuesFromSpread, getDirectiveValues } from './values.js'; +import { experimentalGetArgumentValues, getDirectiveValues } from './values.js'; export interface DeferUsage { label: string | undefined; parentDeferUsage: DeferUsage | undefined; } +export interface FragmentVariables { + signatures: ObjMap; + values: ObjMap; +} + export interface FieldDetails { node: FieldNode; - deferUsage: DeferUsage | undefined; - fragmentVariableValues?: ObjMap | undefined; + deferUsage?: DeferUsage | undefined; + fragmentVariables?: FragmentVariables | undefined; } export type FieldGroup = ReadonlyArray; export type GroupedFieldSet = ReadonlyMap; +export interface FragmentDetails { + definition: FragmentDefinitionNode; + variableSignatures?: ObjMap | undefined; +} + interface CollectFieldsContext { schema: GraphQLSchema; - fragments: ObjMap; + fragments: ObjMap; + variableValues: { [variable: string]: unknown }; + fragmentVariableValues?: FragmentVariables; operation: OperationDefinitionNode; runtimeType: GraphQLObjectType; visitedFragmentNames: Set; - localVariableValues: { [variable: string]: unknown } | undefined; - variableValues: { [variable: string]: unknown }; } /** @@ -62,7 +73,7 @@ interface CollectFieldsContext { */ export function collectFields( schema: GraphQLSchema, - fragments: ObjMap, + fragments: ObjMap, variableValues: { [variable: string]: unknown }, runtimeType: GraphQLObjectType, operation: OperationDefinitionNode, @@ -75,10 +86,9 @@ export function collectFields( const context: CollectFieldsContext = { schema, fragments, - runtimeType, variableValues, + runtimeType, operation, - localVariableValues: undefined, visitedFragmentNames: new Set(), }; @@ -104,7 +114,7 @@ export function collectFields( // eslint-disable-next-line max-params export function collectSubfields( schema: GraphQLSchema, - fragments: ObjMap, + fragments: ObjMap, variableValues: { [variable: string]: unknown }, operation: OperationDefinitionNode, returnType: GraphQLObjectType, @@ -116,9 +126,8 @@ export function collectSubfields( const context: CollectFieldsContext = { schema, fragments, - runtimeType: returnType, - localVariableValues: undefined, variableValues, + runtimeType: returnType, operation, visitedFragmentNames: new Set(), }; @@ -144,19 +153,20 @@ export function collectSubfields( }; } +// eslint-disable-next-line max-params function collectFieldsImpl( context: CollectFieldsContext, selectionSet: SelectionSetNode, groupedFieldSet: AccumulatorMap, newDeferUsages: Array, deferUsage?: DeferUsage, + fragmentVariables?: FragmentVariables, ): void { const { schema, fragments, - runtimeType, variableValues, - localVariableValues, + runtimeType, operation, visitedFragmentNames, } = context; @@ -164,29 +174,19 @@ function collectFieldsImpl( for (const selection of selectionSet.selections) { switch (selection.kind) { case Kind.FIELD: { - if ( - !shouldIncludeNode( - variableValues, - selection, - context.localVariableValues, - ) - ) { + if (!shouldIncludeNode(selection, variableValues, fragmentVariables)) { continue; } groupedFieldSet.add(getFieldEntryKey(selection), { node: selection, deferUsage, - fragmentVariableValues: localVariableValues ?? undefined, + fragmentVariables, }); break; } case Kind.INLINE_FRAGMENT: { if ( - !shouldIncludeNode( - variableValues, - selection, - context.localVariableValues, - ) || + !shouldIncludeNode(selection, variableValues, fragmentVariables) || !doesFragmentConditionMatch(schema, selection, runtimeType) ) { continue; @@ -195,9 +195,9 @@ function collectFieldsImpl( const newDeferUsage = getDeferUsage( operation, variableValues, + fragmentVariables, selection, deferUsage, - context.localVariableValues, ); if (!newDeferUsage) { @@ -207,6 +207,7 @@ function collectFieldsImpl( groupedFieldSet, newDeferUsages, deferUsage, + fragmentVariables, ); } else { newDeferUsages.push(newDeferUsage); @@ -216,78 +217,72 @@ function collectFieldsImpl( groupedFieldSet, newDeferUsages, newDeferUsage, + fragmentVariables, ); } break; } case Kind.FRAGMENT_SPREAD: { - const fragmentName = selection.name.value; + const fragName = selection.name.value; const newDeferUsage = getDeferUsage( operation, variableValues, + fragmentVariables, selection, deferUsage, - context.localVariableValues, ); if ( !newDeferUsage && - (visitedFragmentNames.has(fragmentName) || - !shouldIncludeNode( - variableValues, - selection, - context.localVariableValues, - )) + (visitedFragmentNames.has(fragName) || + !shouldIncludeNode(selection, variableValues, fragmentVariables)) ) { continue; } - const fragment = fragments[fragmentName]; + const fragment = fragments[fragName]; if ( fragment == null || - !doesFragmentConditionMatch(schema, fragment, runtimeType) + !doesFragmentConditionMatch(schema, fragment.definition, runtimeType) ) { continue; } - // We need to introduce a concept of shadowing: - // - // - when a fragment defines a variable that is in the parent scope but not given - // in the fragment-spread we need to look at this variable as undefined and check - // whether the definition has a defaultValue, if not remove it from the variableValues. - // - when a fragment does not define a variable we need to copy it over from the parent - // scope as that variable can still get used in spreads later on in the selectionSet. - // - when a value is passed in through the fragment-spread we need to copy over the key-value - // into our variable-values. - context.localVariableValues = fragment.variableDefinitions - ? getArgumentValuesFromSpread( + const fragmentVariableSignatures = fragment.variableSignatures; + let newFragmentVariables: FragmentVariables | undefined; + if (fragmentVariableSignatures) { + newFragmentVariables = { + signatures: fragmentVariableSignatures, + values: experimentalGetArgumentValues( selection, - schema, - fragment.variableDefinitions, + Object.values(fragmentVariableSignatures), variableValues, - context.localVariableValues, - ) - : undefined; + fragmentVariables, + ), + }; + } if (!newDeferUsage) { - visitedFragmentNames.add(fragmentName); + visitedFragmentNames.add(fragName); collectFieldsImpl( context, - fragment.selectionSet, + fragment.definition.selectionSet, groupedFieldSet, newDeferUsages, deferUsage, + newFragmentVariables, ); } else { newDeferUsages.push(newDeferUsage); collectFieldsImpl( context, - fragment.selectionSet, + fragment.definition.selectionSet, groupedFieldSet, newDeferUsages, newDeferUsage, + newFragmentVariables, ); } break; @@ -304,15 +299,15 @@ function collectFieldsImpl( function getDeferUsage( operation: OperationDefinitionNode, variableValues: { [variable: string]: unknown }, + fragmentVariables: FragmentVariables | undefined, node: FragmentSpreadNode | InlineFragmentNode, parentDeferUsage: DeferUsage | undefined, - fragmentVariableValues: { [variable: string]: unknown } | undefined, ): DeferUsage | undefined { const defer = getDirectiveValues( GraphQLDeferDirective, node, variableValues, - fragmentVariableValues, + fragmentVariables, ); if (!defer) { @@ -339,15 +334,15 @@ function getDeferUsage( * directives, where `@skip` has higher precedence than `@include`. */ function shouldIncludeNode( - variableValues: { [variable: string]: unknown }, node: FragmentSpreadNode | FieldNode | InlineFragmentNode, - fragmentVariableValues: { [variable: string]: unknown } | undefined, + variableValues: { [variable: string]: unknown }, + fragmentVariables: FragmentVariables | undefined, ): boolean { const skip = getDirectiveValues( GraphQLSkipDirective, node, variableValues, - fragmentVariableValues, + fragmentVariables, ); if (skip?.if === true) { return false; @@ -357,7 +352,7 @@ function shouldIncludeNode( GraphQLIncludeDirective, node, variableValues, - fragmentVariableValues, + fragmentVariables, ); if (include?.if === false) { return false; diff --git a/src/execution/execute.ts b/src/execution/execute.ts index c4d2cfb8cc..f394ac2bae 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -5,6 +5,7 @@ import { isAsyncIterable } from '../jsutils/isAsyncIterable.js'; import { isIterableObject } from '../jsutils/isIterableObject.js'; import { isObjectLike } from '../jsutils/isObjectLike.js'; import { isPromise } from '../jsutils/isPromise.js'; +import { mapValue } from '../jsutils/mapValue.js'; import type { Maybe } from '../jsutils/Maybe.js'; import { memoize3 } from '../jsutils/memoize3.js'; import type { ObjMap } from '../jsutils/ObjMap.js'; @@ -20,7 +21,6 @@ import { locatedError } from '../error/locatedError.js'; import type { DocumentNode, FieldNode, - FragmentDefinitionNode, OperationDefinitionNode, } from '../language/ast.js'; import { OperationTypeNode } from '../language/ast.js'; @@ -48,11 +48,14 @@ import { GraphQLStreamDirective } from '../type/directives.js'; import type { GraphQLSchema } from '../type/schema.js'; import { assertValidSchema } from '../type/validate.js'; +import { getVariableSignature } from '../utilities/getVariableSignature.js'; + import type { DeferUsageSet, ExecutionPlan } from './buildExecutionPlan.js'; import { buildExecutionPlan } from './buildExecutionPlan.js'; import type { DeferUsage, FieldGroup, + FragmentDetails, GroupedFieldSet, } from './collectFields.js'; import { @@ -74,6 +77,7 @@ import type { } from './types.js'; import { DeferredFragmentRecord } from './types.js'; import { + experimentalGetArgumentValues, getArgumentValues, getDirectiveValues, getVariableValues, @@ -132,7 +136,7 @@ const collectSubfields = memoize3( */ export interface ExecutionContext { schema: GraphQLSchema; - fragments: ObjMap; + fragments: ObjMap; rootValue: unknown; contextValue: unknown; operation: OperationDefinitionNode; @@ -445,7 +449,7 @@ export function buildExecutionContext( assertValidSchema(schema); let operation: OperationDefinitionNode | undefined; - const fragments: ObjMap = Object.create(null); + const fragments: ObjMap = Object.create(null); for (const definition of document.definitions) { switch (definition.kind) { case Kind.OPERATION_DEFINITION: @@ -462,9 +466,18 @@ export function buildExecutionContext( operation = definition; } break; - case Kind.FRAGMENT_DEFINITION: - fragments[definition.name.value] = definition; + case Kind.FRAGMENT_DEFINITION: { + let variableSignatures; + if (definition.variableDefinitions) { + variableSignatures = Object.create(null); + for (const varDef of definition.variableDefinitions) { + const signature = getVariableSignature(schema, varDef); + variableSignatures[signature.name] = signature; + } + } + fragments[definition.name.value] = { definition, variableSignatures }; break; + } default: // ignore non-executable definitions } @@ -720,11 +733,11 @@ function executeField( // Build a JS object of arguments from the field.arguments AST, using the // variables scope to fulfill any variable references. // TODO: find a way to memoize, in case this field is within a List type. - const args = getArgumentValues( + const args = experimentalGetArgumentValues( fieldGroup[0].node, fieldDef.args, exeContext.variableValues, - fieldGroup[0].fragmentVariableValues, + fieldGroup[0].fragmentVariables, ); // The resolve function's optional third argument is a context value that @@ -807,7 +820,10 @@ export function buildResolveInfo( parentType, path, schema: exeContext.schema, - fragments: exeContext.fragments, + fragments: mapValue( + exeContext.fragments, + (fragment) => fragment.definition, + ), rootValue: exeContext.rootValue, operation: exeContext.operation, variableValues: exeContext.variableValues, @@ -1030,7 +1046,7 @@ function getStreamUsage( GraphQLStreamDirective, fieldGroup[0].node, exeContext.variableValues, - fieldGroup[0].fragmentVariableValues, + fieldGroup[0].fragmentVariables, ); if (!stream) { @@ -1059,6 +1075,7 @@ function getStreamUsage( const streamedFieldGroup: FieldGroup = fieldGroup.map((fieldDetails) => ({ node: fieldDetails.node, deferUsage: undefined, + fragmentVariables: fieldDetails.fragmentVariables, })); const streamUsage = { @@ -2053,12 +2070,7 @@ function executeSubscription( // Build a JS object of arguments from the field.arguments AST, using the // variables scope to fulfill any variable references. - const args = getArgumentValues( - fieldNodes[0], - fieldDef.args, - variableValues, - undefined, - ); + const args = getArgumentValues(fieldDef, fieldNodes[0], variableValues); // The resolve function's optional third argument is a context value that // is provided to every resolve function within an execution. It is commonly diff --git a/src/execution/values.ts b/src/execution/values.ts index fabb083583..3b1cf4b32c 100644 --- a/src/execution/values.ts +++ b/src/execution/values.ts @@ -14,15 +14,17 @@ import type { import { Kind } from '../language/kinds.js'; import { print } from '../language/printer.js'; -import type { GraphQLArgument } from '../type/definition.js'; -import { isInputType, isNonNullType } from '../type/definition.js'; +import type { GraphQLArgument, GraphQLField } from '../type/definition.js'; +import { isNonNullType } from '../type/definition.js'; import type { GraphQLDirective } from '../type/directives.js'; import type { GraphQLSchema } from '../type/schema.js'; import { coerceInputValue } from '../utilities/coerceInputValue.js'; -import { typeFromAST } from '../utilities/typeFromAST.js'; +import type { GraphQLVariableSignature } from '../utilities/getVariableSignature.js'; +import { getVariableSignature } from '../utilities/getVariableSignature.js'; import { valueFromAST } from '../utilities/valueFromAST.js'; -import { valueFromASTUntyped } from '../utilities/valueFromASTUntyped.js'; + +import type { FragmentVariables } from './collectFields.js'; type CoercedVariableValues = | { errors: ReadonlyArray; coerced?: never } @@ -70,14 +72,6 @@ export function getVariableValues( return { errors }; } -/** - * Prepares an object map of argument values given a list of argument - * definitions and list of argument AST nodes. - * - * Note: The returned value is a plain Object with a prototype, since it is - * exposed to user code. Care should be taken to not pull values from the - * Object prototype. - */ function coerceVariableValues( schema: GraphQLSchema, varDefNodes: ReadonlyArray, @@ -86,24 +80,16 @@ function coerceVariableValues( ): { [variable: string]: unknown } { const coercedValues: { [variable: string]: unknown } = {}; for (const varDefNode of varDefNodes) { - const varName = varDefNode.variable.name.value; - const varType = typeFromAST(schema, varDefNode.type); - if (!isInputType(varType)) { - // Must use input types for variables. This should be caught during - // validation, however is checked again here for safety. - const varTypeStr = print(varDefNode.type); - onError( - new GraphQLError( - `Variable "$${varName}" expected value of type "${varTypeStr}" which cannot be used as an input type.`, - { nodes: varDefNode.type }, - ), - ); + const varSignature = getVariableSignature(schema, varDefNode); + if (varSignature instanceof GraphQLError) { + onError(varSignature); continue; } + const { name: varName, type: varType } = varSignature; if (!Object.hasOwn(inputs, varName)) { if (varDefNode.defaultValue) { - coercedValues[varName] = valueFromAST(varDefNode.defaultValue, varType); + coercedValues[varName] = varSignature.defaultValue; } else if (isNonNullType(varType)) { const varTypeStr = inspect(varType); onError( @@ -159,15 +145,25 @@ function coerceVariableValues( * Object prototype. */ export function getArgumentValues( + def: GraphQLField | GraphQLDirective, node: FieldNode | DirectiveNode, - argDefs: ReadonlyArray, + variableValues?: Maybe>, +): { [argument: string]: unknown } { + return experimentalGetArgumentValues(node, def.args, variableValues); +} + +export function experimentalGetArgumentValues( + node: FieldNode | DirectiveNode | FragmentSpreadNode, + argDefs: ReadonlyArray, variableValues: Maybe>, - fragmentArgValues?: Maybe>, + fragmentVariables?: Maybe, ): { [argument: string]: unknown } { const coercedValues: { [argument: string]: unknown } = {}; - const argNodeMap = new Map( - node.arguments?.map((arg) => [arg.name.value, arg]), - ); + + // FIXME: https://github.com/graphql/graphql-js/issues/2203 + /* c8 ignore next */ + const argumentNodes = node.arguments ?? []; + const argNodeMap = new Map(argumentNodes.map((arg) => [arg.name.value, arg])); for (const argDef of argDefs) { const name = argDef.name; @@ -192,11 +188,8 @@ export function getArgumentValues( let hasValue = valueNode.kind !== Kind.NULL; if (valueNode.kind === Kind.VARIABLE) { const variableName = valueNode.name.value; - if ( - fragmentArgValues != null && - Object.hasOwn(fragmentArgValues, variableName) - ) { - hasValue = fragmentArgValues[variableName] != null; + if (fragmentVariables?.signatures[variableName]) { + hasValue = fragmentVariables.values[variableName] != null; if (!hasValue && argDef.defaultValue !== undefined) { coercedValues[name] = argDef.defaultValue; continue; @@ -228,12 +221,12 @@ export function getArgumentValues( ); } - // TODO: Make this follow the spec more closely - const coercedValue = valueFromAST(valueNode, argType, { - ...variableValues, - ...fragmentArgValues, - }); - + const coercedValue = valueFromAST( + valueNode, + argType, + variableValues, + fragmentVariables?.values, + ); if (coercedValue === undefined) { // Note: ValuesOfCorrectTypeRule validation should catch this before // execution. This is a runtime check to ensure execution does not @@ -248,79 +241,6 @@ export function getArgumentValues( return coercedValues; } -export function getArgumentValuesFromSpread( - /** NOTE: For error annotations only */ - node: FragmentSpreadNode, - schema: GraphQLSchema, - fragmentVarDefs: ReadonlyArray, - variableValues: Maybe>, - fragmentArgValues?: Maybe>, -): { [argument: string]: unknown } { - const coercedValues: { [argument: string]: unknown } = {}; - const argNodeMap = new Map( - node.arguments?.map((arg) => [arg.name.value, arg]), - ); - - for (const varDef of fragmentVarDefs) { - const name = varDef.variable.name.value; - const argType = typeFromAST(schema, varDef.type); - const argumentNode = argNodeMap.get(name); - - if (argumentNode == null) { - if (varDef.defaultValue !== undefined) { - coercedValues[name] = valueFromASTUntyped(varDef.defaultValue); - } else if (isNonNullType(argType)) { - throw new GraphQLError( - `Argument "${name}" of required type "${inspect(argType)}" ` + - 'was not provided.', - { nodes: node }, - ); - } else { - coercedValues[name] = undefined; - } - continue; - } - - const valueNode = argumentNode.value; - - let hasValue = valueNode.kind !== Kind.NULL; - if (valueNode.kind === Kind.VARIABLE) { - const variableName = valueNode.name.value; - if ( - fragmentArgValues != null && - Object.hasOwn(fragmentArgValues, variableName) - ) { - hasValue = fragmentArgValues[variableName] != null; - } else if ( - variableValues != null && - Object.hasOwn(variableValues, variableName) - ) { - hasValue = variableValues[variableName] != null; - } - } - - if (!hasValue && isNonNullType(argType)) { - throw new GraphQLError( - `Argument "${name}" of non-null type "${inspect(argType)}" ` + - 'must not be null.', - { nodes: valueNode }, - ); - } - - // TODO: Make this follow the spec more closely - let coercedValue; - if (argType && isInputType(argType)) { - coercedValue = valueFromAST(valueNode, argType, { - ...variableValues, - ...fragmentArgValues, - }); - } - - coercedValues[name] = coercedValue; - } - return coercedValues; -} - /** * Prepares an object map of argument values given a directive definition * and a AST node which may contain directives. Optionally also accepts a map @@ -336,14 +256,14 @@ export function getDirectiveValues( directiveDef: GraphQLDirective, node: { readonly directives?: ReadonlyArray | undefined }, variableValues?: Maybe>, - fragmentVariables?: Maybe>, + fragmentVariables?: Maybe, ): undefined | { [argument: string]: unknown } { const directiveNode = node.directives?.find( (directive) => directive.name.value === directiveDef.name, ); if (directiveNode) { - return getArgumentValues( + return experimentalGetArgumentValues( directiveNode, directiveDef.args, variableValues, diff --git a/src/utilities/getVariableSignature.ts b/src/utilities/getVariableSignature.ts new file mode 100644 index 0000000000..4ff7e977e4 --- /dev/null +++ b/src/utilities/getVariableSignature.ts @@ -0,0 +1,44 @@ +import { GraphQLError } from '../error/GraphQLError.js'; + +import { print } from '../language/printer.js'; + +import type { GraphQLInputType, GraphQLSchema } from '../type/index.js'; +import { isInputType } from '../type/index.js'; + +import type { VariableDefinitionNode } from '../index.js'; + +import { typeFromAST } from './typeFromAST.js'; +import { valueFromAST } from './valueFromAST.js'; + +/** + * A GraphQLVariableSignature is required to coerce a variable value. + * */ +export interface GraphQLVariableSignature { + name: string; + type: GraphQLInputType; + defaultValue: unknown; +} + +export function getVariableSignature( + schema: GraphQLSchema, + varDefNode: VariableDefinitionNode, +): GraphQLVariableSignature | GraphQLError { + const varName = varDefNode.variable.name.value; + const varType = typeFromAST(schema, varDefNode.type); + + if (!isInputType(varType)) { + // Must use input types for variables. This should be caught during + // validation, however is checked again here for safety. + const varTypeStr = print(varDefNode.type); + return new GraphQLError( + `Variable "$${varName}" expected value of type "${varTypeStr}" which cannot be used as an input type.`, + { nodes: varDefNode.type }, + ); + } + + return { + name: varName, + type: varType, + defaultValue: valueFromAST(varDefNode.defaultValue, varType), + }; +} diff --git a/src/utilities/valueFromAST.ts b/src/utilities/valueFromAST.ts index 5e0d3c517e..3aec3f272f 100644 --- a/src/utilities/valueFromAST.ts +++ b/src/utilities/valueFromAST.ts @@ -38,6 +38,7 @@ export function valueFromAST( valueNode: Maybe, type: GraphQLInputType, variables?: Maybe>, + fragmentVariables?: Maybe>, ): unknown { if (!valueNode) { // When there is no node, then there is also no value. @@ -47,11 +48,12 @@ export function valueFromAST( if (valueNode.kind === Kind.VARIABLE) { const variableName = valueNode.name.value; - if (variables == null || variables[variableName] === undefined) { + const variableValue = + fragmentVariables?.[variableName] ?? variables?.[variableName]; + if (variableValue === undefined) { // No valid return value. return; } - const variableValue = variables[variableName]; if (variableValue === null && isNonNullType(type)) { return; // Invalid: intentionally return no value. } @@ -65,7 +67,7 @@ export function valueFromAST( if (valueNode.kind === Kind.NULL) { return; // Invalid: intentionally return no value. } - return valueFromAST(valueNode, type.ofType, variables); + return valueFromAST(valueNode, type.ofType, variables, fragmentVariables); } if (valueNode.kind === Kind.NULL) { @@ -78,7 +80,7 @@ export function valueFromAST( if (valueNode.kind === Kind.LIST) { const coercedValues = []; for (const itemNode of valueNode.values) { - if (isMissingVariable(itemNode, variables)) { + if (isMissingVariable(itemNode, variables, fragmentVariables)) { // If an array contains a missing variable, it is either coerced to // null or if the item type is non-null, it considered invalid. if (isNonNullType(itemType)) { @@ -86,7 +88,12 @@ export function valueFromAST( } coercedValues.push(null); } else { - const itemValue = valueFromAST(itemNode, itemType, variables); + const itemValue = valueFromAST( + itemNode, + itemType, + variables, + fragmentVariables, + ); if (itemValue === undefined) { return; // Invalid: intentionally return no value. } @@ -95,7 +102,12 @@ export function valueFromAST( } return coercedValues; } - const coercedValue = valueFromAST(valueNode, itemType, variables); + const coercedValue = valueFromAST( + valueNode, + itemType, + variables, + fragmentVariables, + ); if (coercedValue === undefined) { return; // Invalid: intentionally return no value. } @@ -112,7 +124,10 @@ export function valueFromAST( ); for (const field of Object.values(type.getFields())) { const fieldNode = fieldNodes.get(field.name); - if (fieldNode == null || isMissingVariable(fieldNode.value, variables)) { + if ( + fieldNode == null || + isMissingVariable(fieldNode.value, variables, fragmentVariables) + ) { if (field.defaultValue !== undefined) { coercedObj[field.name] = field.defaultValue; } else if (isNonNullType(field.type)) { @@ -120,7 +135,12 @@ export function valueFromAST( } continue; } - const fieldValue = valueFromAST(fieldNode.value, field.type, variables); + const fieldValue = valueFromAST( + fieldNode.value, + field.type, + variables, + fragmentVariables, + ); if (fieldValue === undefined) { return; // Invalid: intentionally return no value. } @@ -166,9 +186,12 @@ export function valueFromAST( function isMissingVariable( valueNode: ValueNode, variables: Maybe>, + fragmentVariables: Maybe>, ): boolean { return ( valueNode.kind === Kind.VARIABLE && + (fragmentVariables == null || + fragmentVariables[valueNode.name.value] === undefined) && (variables == null || variables[valueNode.name.value] === undefined) ); } diff --git a/src/validation/rules/SingleFieldSubscriptionsRule.ts b/src/validation/rules/SingleFieldSubscriptionsRule.ts index 700bc0bda7..dfefc6cdd8 100644 --- a/src/validation/rules/SingleFieldSubscriptionsRule.ts +++ b/src/validation/rules/SingleFieldSubscriptionsRule.ts @@ -2,15 +2,14 @@ import type { ObjMap } from '../../jsutils/ObjMap.js'; import { GraphQLError } from '../../error/GraphQLError.js'; -import type { - FieldNode, - FragmentDefinitionNode, - OperationDefinitionNode, -} from '../../language/ast.js'; +import type { FieldNode, OperationDefinitionNode } from '../../language/ast.js'; import { Kind } from '../../language/kinds.js'; import type { ASTVisitor } from '../../language/visitor.js'; -import type { FieldGroup } from '../../execution/collectFields.js'; +import type { + FieldGroup, + FragmentDetails, +} from '../../execution/collectFields.js'; import { collectFields } from '../../execution/collectFields.js'; import type { ValidationContext } from '../ValidationContext.js'; @@ -41,10 +40,10 @@ export function SingleFieldSubscriptionsRule( [variable: string]: any; } = Object.create(null); const document = context.getDocument(); - const fragments: ObjMap = Object.create(null); + const fragments: ObjMap = Object.create(null); for (const definition of document.definitions) { if (definition.kind === Kind.FRAGMENT_DEFINITION) { - fragments[definition.name.value] = definition; + fragments[definition.name.value] = { definition }; } } const { groupedFieldSet } = collectFields(