diff --git a/benchmark/fragment-arguments-benchmark.js b/benchmark/fragment-arguments-benchmark.js new file mode 100644 index 0000000000..b718847256 --- /dev/null +++ b/benchmark/fragment-arguments-benchmark.js @@ -0,0 +1,48 @@ +import { execute } from 'graphql/execution/execute.js'; +import { parse } from 'graphql/language/parser.js'; +import { buildSchema } from 'graphql/utilities/buildASTSchema.js'; + +const schema = buildSchema('type Query { field(echo: String!): [String] }'); + +const integers = Array.from({ length: 100 }, (_, i) => i + 1); + +const document = parse( + ` + query WithManyFragmentArguments(${integers + .map((i) => `$echo${i}: String`) + .join(', ')}) { + ${integers.map((i) => `echo${i}: field(echo: $echo${i})`).join('\n')} + ${integers.map((i) => `...EchoFragment${i}(echo: $echo${i})`).join('\n')} + } + + ${integers + .map( + (i) => + `fragment EchoFragment${i}($echo: String) on Query { echoFragment${i}: field(echo: $echo) }`, + ) + .join('\n')} + `, + { experimentalFragmentArguments: true }, +); + +const variableValues = Object.create(null); +for (const i of integers) { + variableValues[`echo${i}`] = `Echo ${i}`; +} + +function field(_, args) { + return args.echo; +} + +export const benchmark = { + name: 'Execute Operation with Fragment Arguments', + count: 10, + async measure() { + await execute({ + schema, + document, + rootValue: { field }, + variableValues, + }); + }, +}; diff --git a/src/execution/collectFields.ts b/src/execution/collectFields.ts index 9146312f70..1850e819fb 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; + scopedVariableValues?: { [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, scopedVariableValues } = fieldDetail; collectFieldsImpl( context, selectionSet, subGroupedFieldSet, newDeferUsages, deferUsage, - fragmentVariables, + scopedVariableValues, ); } } @@ -161,33 +156,35 @@ function collectFieldsImpl( groupedFieldSet: AccumulatorMap, newDeferUsages: Array, deferUsage?: DeferUsage, - fragmentVariables?: FragmentVariables, + scopedVariableValues?: { [variable: string]: unknown }, ): void { const { schema, fragments, - variableValues, + variableValues: operationVariableValues, runtimeType, operation, visitedFragmentNames, } = context; + const variableValues = scopedVariableValues ?? operationVariableValues; + for (const selection of selectionSet.selections) { switch (selection.kind) { case Kind.FIELD: { - if (!shouldIncludeNode(selection, variableValues, fragmentVariables)) { + if (!shouldIncludeNode(variableValues, selection)) { continue; } groupedFieldSet.add(getFieldEntryKey(selection), { node: selection, deferUsage, - fragmentVariables, + scopedVariableValues, }); break; } case Kind.INLINE_FRAGMENT: { if ( - !shouldIncludeNode(selection, variableValues, fragmentVariables) || + !shouldIncludeNode(variableValues, selection) || !doesFragmentConditionMatch(schema, selection, runtimeType) ) { continue; @@ -196,7 +193,6 @@ function collectFieldsImpl( const newDeferUsage = getDeferUsage( operation, variableValues, - fragmentVariables, selection, deferUsage, ); @@ -208,7 +204,7 @@ function collectFieldsImpl( groupedFieldSet, newDeferUsages, deferUsage, - fragmentVariables, + scopedVariableValues, ); } else { newDeferUsages.push(newDeferUsage); @@ -218,7 +214,7 @@ function collectFieldsImpl( groupedFieldSet, newDeferUsages, newDeferUsage, - fragmentVariables, + scopedVariableValues, ); } @@ -230,7 +226,6 @@ function collectFieldsImpl( const newDeferUsage = getDeferUsage( operation, variableValues, - fragmentVariables, selection, deferUsage, ); @@ -238,7 +233,7 @@ function collectFieldsImpl( if ( !newDeferUsage && (visitedFragmentNames.has(fragName) || - !shouldIncludeNode(selection, variableValues, fragmentVariables)) + !shouldIncludeNode(variableValues, selection)) ) { continue; } @@ -251,19 +246,12 @@ function collectFieldsImpl( continue; } - const fragmentVariableSignatures = fragment.variableSignatures; - let newFragmentVariables: FragmentVariables | undefined; - if (fragmentVariableSignatures) { - newFragmentVariables = { - signatures: fragmentVariableSignatures, - values: experimentalGetArgumentValues( - selection, - Object.values(fragmentVariableSignatures), - variableValues, - fragmentVariables, - ), - }; - } + const newScopedVariableValues = getScopedVariableValues( + fragment.variableSignatures, + selection, + variableValues, + operationVariableValues, + ); if (!newDeferUsage) { visitedFragmentNames.add(fragName); @@ -273,7 +261,7 @@ function collectFieldsImpl( groupedFieldSet, newDeferUsages, deferUsage, - newFragmentVariables, + newScopedVariableValues, ); } else { newDeferUsages.push(newDeferUsage); @@ -283,7 +271,7 @@ function collectFieldsImpl( groupedFieldSet, newDeferUsages, newDeferUsage, - newFragmentVariables, + newScopedVariableValues, ); } break; @@ -300,16 +288,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 +317,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 +329,6 @@ function shouldIncludeNode( GraphQLIncludeDirective, node, variableValues, - fragmentVariables, ); if (include?.if === false) { return false; @@ -383,6 +358,31 @@ function doesFragmentConditionMatch( return false; } +/** + * Constructs a variableValues map for the given fragment. + */ +function getScopedVariableValues( + fragmentVariableSignatures: ObjMap | undefined, + fragmentSpreadNode: FragmentSpreadNode, + variableValues: { [variable: string]: unknown }, + operationVariableValues: { [variable: string]: unknown }, +): { [variable: string]: unknown } | undefined { + if (!fragmentVariableSignatures) { + return; + } + const scopedVariableValues = experimentalGetArgumentValues( + fragmentSpreadNode, + Object.values(fragmentVariableSignatures), + variableValues, + ); + for (const [variableName, value] of Object.entries(operationVariableValues)) { + if (fragmentVariableSignatures[variableName] === undefined) { + scopedVariableValues[variableName] = value; + } + } + return scopedVariableValues; +} + /** * Implements the logic to compute the key of a given field's entry */ diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 97e88f07ad..7a36792eec 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].scopedVariableValues ?? 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].scopedVariableValues ?? exeContext.variableValues, ); if (!stream) { @@ -1091,7 +1089,7 @@ function getStreamUsage( const streamedFieldGroup: FieldGroup = fieldGroup.map((fieldDetails) => ({ node: fieldDetails.node, deferUsage: undefined, - fragmentVariables: fieldDetails.fragmentVariables, + scopedVariableValues: fieldDetails.scopedVariableValues, })); 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) ); }