diff --git a/src/execution/collectFields.ts b/src/execution/collectFields.ts index 9146312f70..714ae2bad0 100644 --- a/src/execution/collectFields.ts +++ b/src/execution/collectFields.ts @@ -32,15 +32,10 @@ export interface DeferUsage { parentDeferUsage: DeferUsage | undefined; } -export interface FragmentVariables { - signatures: ObjMap; - values: ObjMap; -} - export interface FieldDetails { node: FieldNode; deferUsage?: DeferUsage | undefined; - fragmentVariables?: FragmentVariables | undefined; + fragmentVariableValues?: { [variable: string]: unknown } | undefined; } export type FieldGroup = ReadonlyArray; @@ -136,14 +131,14 @@ export function collectSubfields( for (const fieldDetail of fieldGroup) { const selectionSet = fieldDetail.node.selectionSet; if (selectionSet) { - const { deferUsage, fragmentVariables } = fieldDetail; + const { deferUsage, fragmentVariableValues } = fieldDetail; collectFieldsImpl( context, selectionSet, subGroupedFieldSet, newDeferUsages, deferUsage, - fragmentVariables, + fragmentVariableValues, ); } } @@ -161,7 +156,7 @@ function collectFieldsImpl( groupedFieldSet: AccumulatorMap, newDeferUsages: Array, deferUsage?: DeferUsage, - fragmentVariables?: FragmentVariables, + fragmentVariableValues?: { [variable: string]: unknown }, ): void { const { schema, @@ -172,22 +167,24 @@ function collectFieldsImpl( visitedFragmentNames, } = context; + const scopedVariableValues = fragmentVariableValues ?? variableValues; + for (const selection of selectionSet.selections) { switch (selection.kind) { case Kind.FIELD: { - if (!shouldIncludeNode(selection, variableValues, fragmentVariables)) { + if (!shouldIncludeNode(scopedVariableValues, selection)) { continue; } groupedFieldSet.add(getFieldEntryKey(selection), { node: selection, deferUsage, - fragmentVariables, + fragmentVariableValues, }); break; } case Kind.INLINE_FRAGMENT: { if ( - !shouldIncludeNode(selection, variableValues, fragmentVariables) || + !shouldIncludeNode(scopedVariableValues, selection) || !doesFragmentConditionMatch(schema, selection, runtimeType) ) { continue; @@ -195,8 +192,7 @@ function collectFieldsImpl( const newDeferUsage = getDeferUsage( operation, - variableValues, - fragmentVariables, + fragmentVariableValues ?? variableValues, selection, deferUsage, ); @@ -208,7 +204,7 @@ function collectFieldsImpl( groupedFieldSet, newDeferUsages, deferUsage, - fragmentVariables, + fragmentVariableValues, ); } else { newDeferUsages.push(newDeferUsage); @@ -218,7 +214,7 @@ function collectFieldsImpl( groupedFieldSet, newDeferUsages, newDeferUsage, - fragmentVariables, + fragmentVariableValues, ); } @@ -229,8 +225,7 @@ function collectFieldsImpl( const newDeferUsage = getDeferUsage( operation, - variableValues, - fragmentVariables, + scopedVariableValues, selection, deferUsage, ); @@ -238,7 +233,10 @@ function collectFieldsImpl( if ( !newDeferUsage && (visitedFragmentNames.has(fragName) || - !shouldIncludeNode(selection, variableValues, fragmentVariables)) + !shouldIncludeNode( + fragmentVariableValues ?? variableValues, + selection, + )) ) { continue; } @@ -252,17 +250,20 @@ function collectFieldsImpl( } const fragmentVariableSignatures = fragment.variableSignatures; - let newFragmentVariables: FragmentVariables | undefined; + let newFragmentVariableValues: + | { [variable: string]: unknown } + | undefined; if (fragmentVariableSignatures) { - newFragmentVariables = { - signatures: fragmentVariableSignatures, - values: experimentalGetArgumentValues( - selection, - Object.values(fragmentVariableSignatures), - variableValues, - fragmentVariables, - ), - }; + newFragmentVariableValues = experimentalGetArgumentValues( + selection, + Object.values(fragmentVariableSignatures), + scopedVariableValues, + ); + for (const [variableName, value] of Object.entries(variableValues)) { + if (!fragment.variableSignatures?.[variableName]) { + newFragmentVariableValues[variableName] = value; + } + } } if (!newDeferUsage) { @@ -273,7 +274,7 @@ function collectFieldsImpl( groupedFieldSet, newDeferUsages, deferUsage, - newFragmentVariables, + newFragmentVariableValues, ); } else { newDeferUsages.push(newDeferUsage); @@ -283,7 +284,7 @@ function collectFieldsImpl( groupedFieldSet, newDeferUsages, newDeferUsage, - newFragmentVariables, + newFragmentVariableValues, ); } break; @@ -300,16 +301,10 @@ function collectFieldsImpl( function getDeferUsage( operation: OperationDefinitionNode, variableValues: { [variable: string]: unknown }, - fragmentVariables: FragmentVariables | undefined, node: FragmentSpreadNode | InlineFragmentNode, parentDeferUsage: DeferUsage | undefined, ): DeferUsage | undefined { - const defer = getDirectiveValues( - GraphQLDeferDirective, - node, - variableValues, - fragmentVariables, - ); + const defer = getDirectiveValues(GraphQLDeferDirective, node, variableValues); if (!defer) { return; @@ -335,16 +330,10 @@ function getDeferUsage( * directives, where `@skip` has higher precedence than `@include`. */ function shouldIncludeNode( - node: FragmentSpreadNode | FieldNode | InlineFragmentNode, variableValues: { [variable: string]: unknown }, - fragmentVariables: FragmentVariables | undefined, + node: FragmentSpreadNode | FieldNode | InlineFragmentNode, ): boolean { - const skip = getDirectiveValues( - GraphQLSkipDirective, - node, - variableValues, - fragmentVariables, - ); + const skip = getDirectiveValues(GraphQLSkipDirective, node, variableValues); if (skip?.if === true) { return false; } @@ -353,7 +342,6 @@ function shouldIncludeNode( GraphQLIncludeDirective, node, variableValues, - fragmentVariables, ); if (include?.if === false) { return false; diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 97e88f07ad..4c3dd65cfd 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -752,8 +752,7 @@ function executeField( const args = experimentalGetArgumentValues( fieldGroup[0].node, fieldDef.args, - exeContext.variableValues, - fieldGroup[0].fragmentVariables, + fieldGroup[0].fragmentVariableValues ?? exeContext.variableValues, ); // The resolve function's optional third argument is a context value that @@ -1061,8 +1060,7 @@ function getStreamUsage( const stream = getDirectiveValues( GraphQLStreamDirective, fieldGroup[0].node, - exeContext.variableValues, - fieldGroup[0].fragmentVariables, + fieldGroup[0].fragmentVariableValues ?? exeContext.variableValues, ); if (!stream) { @@ -1091,7 +1089,7 @@ function getStreamUsage( const streamedFieldGroup: FieldGroup = fieldGroup.map((fieldDetails) => ({ node: fieldDetails.node, deferUsage: undefined, - fragmentVariables: fieldDetails.fragmentVariables, + fragmentVariableValues: fieldDetails.fragmentVariableValues, })); const streamUsage = { diff --git a/src/execution/values.ts b/src/execution/values.ts index 23863fd107..5896f85ba2 100644 --- a/src/execution/values.ts +++ b/src/execution/values.ts @@ -22,7 +22,6 @@ import type { GraphQLSchema } from '../type/schema.js'; import { coerceInputValue } from '../utilities/coerceInputValue.js'; import { valueFromAST } from '../utilities/valueFromAST.js'; -import type { FragmentVariables } from './collectFields.js'; import type { GraphQLVariableSignature } from './getVariableSignature.js'; import { getVariableSignature } from './getVariableSignature.js'; @@ -156,7 +155,6 @@ export function experimentalGetArgumentValues( node: FieldNode | DirectiveNode | FragmentSpreadNode, argDefs: ReadonlyArray, variableValues: Maybe>, - fragmentVariables?: Maybe, ): { [argument: string]: unknown } { const coercedValues: { [argument: string]: unknown } = {}; @@ -188,12 +186,9 @@ export function experimentalGetArgumentValues( if (valueNode.kind === Kind.VARIABLE) { const variableName = valueNode.name.value; - const scopedVariableValues = fragmentVariables?.signatures[variableName] - ? fragmentVariables.values - : variableValues; if ( - scopedVariableValues == null || - !Object.hasOwn(scopedVariableValues, variableName) + variableValues == null || + !Object.hasOwn(variableValues, variableName) ) { if (argDef.defaultValue !== undefined) { coercedValues[name] = argDef.defaultValue; @@ -206,7 +201,7 @@ export function experimentalGetArgumentValues( } continue; } - isNull = scopedVariableValues[variableName] == null; + isNull = variableValues[variableName] == null; } if (isNull && isNonNullType(argType)) { @@ -217,12 +212,7 @@ export function experimentalGetArgumentValues( ); } - const coercedValue = valueFromAST( - valueNode, - argType, - variableValues, - fragmentVariables?.values, - ); + const coercedValue = valueFromAST(valueNode, argType, variableValues); if (coercedValue === undefined) { // Note: ValuesOfCorrectTypeRule validation should catch this before // execution. This is a runtime check to ensure execution does not @@ -254,7 +244,6 @@ export function getDirectiveValues( directiveDef: GraphQLDirective, node: { readonly directives?: ReadonlyArray | undefined }, variableValues?: Maybe>, - fragmentVariables?: Maybe, ): undefined | { [argument: string]: unknown } { const directiveNode = node.directives?.find( (directive) => directive.name.value === directiveDef.name, @@ -265,7 +254,6 @@ export function getDirectiveValues( directiveNode, directiveDef.args, variableValues, - fragmentVariables, ); } } diff --git a/src/utilities/valueFromAST.ts b/src/utilities/valueFromAST.ts index 3aec3f272f..9dee860995 100644 --- a/src/utilities/valueFromAST.ts +++ b/src/utilities/valueFromAST.ts @@ -38,7 +38,6 @@ 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. @@ -48,8 +47,7 @@ export function valueFromAST( if (valueNode.kind === Kind.VARIABLE) { const variableName = valueNode.name.value; - const variableValue = - fragmentVariables?.[variableName] ?? variables?.[variableName]; + const variableValue = variables?.[variableName]; if (variableValue === undefined) { // No valid return value. return; @@ -67,7 +65,7 @@ export function valueFromAST( if (valueNode.kind === Kind.NULL) { return; // Invalid: intentionally return no value. } - return valueFromAST(valueNode, type.ofType, variables, fragmentVariables); + return valueFromAST(valueNode, type.ofType, variables); } if (valueNode.kind === Kind.NULL) { @@ -80,7 +78,7 @@ export function valueFromAST( if (valueNode.kind === Kind.LIST) { const coercedValues = []; for (const itemNode of valueNode.values) { - if (isMissingVariable(itemNode, variables, fragmentVariables)) { + if (isMissingVariable(itemNode, variables)) { // 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)) { @@ -88,12 +86,7 @@ export function valueFromAST( } coercedValues.push(null); } else { - const itemValue = valueFromAST( - itemNode, - itemType, - variables, - fragmentVariables, - ); + const itemValue = valueFromAST(itemNode, itemType, variables); if (itemValue === undefined) { return; // Invalid: intentionally return no value. } @@ -102,12 +95,7 @@ export function valueFromAST( } return coercedValues; } - const coercedValue = valueFromAST( - valueNode, - itemType, - variables, - fragmentVariables, - ); + const coercedValue = valueFromAST(valueNode, itemType, variables); if (coercedValue === undefined) { return; // Invalid: intentionally return no value. } @@ -124,10 +112,7 @@ export function valueFromAST( ); for (const field of Object.values(type.getFields())) { const fieldNode = fieldNodes.get(field.name); - if ( - fieldNode == null || - isMissingVariable(fieldNode.value, variables, fragmentVariables) - ) { + if (fieldNode == null || isMissingVariable(fieldNode.value, variables)) { if (field.defaultValue !== undefined) { coercedObj[field.name] = field.defaultValue; } else if (isNonNullType(field.type)) { @@ -135,12 +120,7 @@ export function valueFromAST( } continue; } - const fieldValue = valueFromAST( - fieldNode.value, - field.type, - variables, - fragmentVariables, - ); + const fieldValue = valueFromAST(fieldNode.value, field.type, variables); if (fieldValue === undefined) { return; // Invalid: intentionally return no value. } @@ -186,12 +166,9 @@ 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) ); }