Skip to content

Commit

Permalink
parse fragment signatures when building execution context and reuse g…
Browse files Browse the repository at this point in the history 
…etArgumentValues

to completely reuse getArgumentValues without greater changes requires omiited arguments to be present as undefined

needs further investigation
  • Loading branch information
@yaacovCR
yaacovCR committed Aug 20, 2024
1 parent b6a4300 commit e5744fc
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 98 deletions.
2 changes: 1 addition & 1 deletion src/execution/__tests__/executor-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1172,7 +1172,7 @@ describe('Execute: Handles basic execution tasks', () => {
const result = executeSync({ schema, document });
expect(result).to.deep.equal({
data: {
field: '{ a: true, c: false, e: 0 }',
field: '{ a: true, b: undefined, c: false, d: undefined, e: 0 }',
},
});
});
Expand Down
2 changes: 1 addition & 1 deletion src/execution/__tests__/variables-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,7 @@ describe('Execute: Handles inputs', () => {

expect(result).to.deep.equal({
data: {
fieldWithNullableStringInput: null,
fieldWithNullableStringInput: 'undefined',
},
});
});
Expand Down
41 changes: 29 additions & 12 deletions src/execution/collectFields.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@ import type {
import { OperationTypeNode } from '../language/ast.js';
import { Kind } from '../language/kinds.js';

import type { GraphQLObjectType } from '../type/definition.js';
import type {
GraphQLInputType,
GraphQLObjectType,
} from '../type/definition.js';
import { isAbstractType } from '../type/definition.js';
import {
GraphQLDeferDirective,
Expand All @@ -24,7 +27,7 @@ import type { GraphQLSchema } from '../type/schema.js';

import { typeFromAST } from '../utilities/typeFromAST.js';

import { getArgumentValuesFromSpread, getDirectiveValues } from './values.js';
import { getArgumentValues, getDirectiveValues } from './values.js';

export interface DeferUsage {
label: string | undefined;
Expand All @@ -41,9 +44,18 @@ export type FieldGroup = ReadonlyArray<FieldDetails>;

export type GroupedFieldSet = ReadonlyMap<string, FieldGroup>;

export interface GraphQLFragmentSignature {
name: string;
type: GraphQLInputType;
defaultValue: unknown;
}

interface CollectFieldsContext {
schema: GraphQLSchema;
fragments: ObjMap<FragmentDefinitionNode>;
fragments: ObjMap<{
definition: FragmentDefinitionNode;
signatures: ReadonlyArray<GraphQLFragmentSignature>;
}>;
operation: OperationDefinitionNode;
runtimeType: GraphQLObjectType;
visitedFragmentNames: Set<string>;
Expand All @@ -62,7 +74,10 @@ interface CollectFieldsContext {
*/
export function collectFields(
schema: GraphQLSchema,
fragments: ObjMap<FragmentDefinitionNode>,
fragments: ObjMap<{
definition: FragmentDefinitionNode;
signatures: ReadonlyArray<GraphQLFragmentSignature>;
}>,
variableValues: { [variable: string]: unknown },
runtimeType: GraphQLObjectType,
operation: OperationDefinitionNode,
Expand Down Expand Up @@ -104,7 +119,10 @@ export function collectFields(
// eslint-disable-next-line max-params
export function collectSubfields(
schema: GraphQLSchema,
fragments: ObjMap<FragmentDefinitionNode>,
fragments: ObjMap<{
definition: FragmentDefinitionNode;
signatures: ReadonlyArray<GraphQLFragmentSignature>;
}>,
variableValues: { [variable: string]: unknown },
operation: OperationDefinitionNode,
returnType: GraphQLObjectType,
Expand Down Expand Up @@ -232,7 +250,7 @@ function collectFieldsImpl(
const fragment = fragments[fragmentName];
if (
fragment == null ||
!doesFragmentConditionMatch(schema, fragment, runtimeType)
!doesFragmentConditionMatch(schema, fragment.definition, runtimeType)
) {
continue;
}
Expand All @@ -246,11 +264,10 @@ function collectFieldsImpl(
// 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(
context.localVariableValues = fragment.definition.variableDefinitions
? getArgumentValues(
selection,
schema,
fragment.variableDefinitions,
fragment.signatures,
variableValues,
context.localVariableValues,
)
Expand All @@ -260,7 +277,7 @@ function collectFieldsImpl(
visitedFragmentNames.add(fragmentName);
collectFieldsImpl(
context,
fragment.selectionSet,
fragment.definition.selectionSet,
groupedFieldSet,
newDeferUsages,
deferUsage,
Expand All @@ -269,7 +286,7 @@ function collectFieldsImpl(
newDeferUsages.push(newDeferUsage);
collectFieldsImpl(
context,
fragment.selectionSet,
fragment.definition.selectionSet,
groupedFieldSet,
newDeferUsages,
newDeferUsage,
Expand Down
40 changes: 35 additions & 5 deletions src/execution/execute.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -39,6 +40,7 @@ import type {
} from '../type/definition.js';
import {
isAbstractType,
isInputType,
isLeafType,
isListType,
isNonNullType,
Expand All @@ -48,11 +50,15 @@ import { GraphQLStreamDirective } from '../type/directives.js';
import type { GraphQLSchema } from '../type/schema.js';
import { assertValidSchema } from '../type/validate.js';

import { typeFromAST } from '../utilities/typeFromAST.js';
import { valueFromAST } from '../utilities/valueFromAST.js';

import type { DeferUsageSet, ExecutionPlan } from './buildExecutionPlan.js';
import { buildExecutionPlan } from './buildExecutionPlan.js';
import type {
DeferUsage,
FieldGroup,
GraphQLFragmentSignature,
GroupedFieldSet,
} from './collectFields.js';
import {
Expand Down Expand Up @@ -132,7 +138,10 @@ const collectSubfields = memoize3(
*/
export interface ExecutionContext {
schema: GraphQLSchema;
fragments: ObjMap<FragmentDefinitionNode>;
fragments: ObjMap<{
definition: FragmentDefinitionNode;
signatures: ReadonlyArray<GraphQLFragmentSignature>;
}>;
rootValue: unknown;
contextValue: unknown;
operation: OperationDefinitionNode;
Expand Down Expand Up @@ -445,7 +454,10 @@ export function buildExecutionContext(
assertValidSchema(schema);

let operation: OperationDefinitionNode | undefined;
const fragments: ObjMap<FragmentDefinitionNode> = Object.create(null);
const fragments: ObjMap<{
definition: FragmentDefinitionNode;
signatures: ReadonlyArray<GraphQLFragmentSignature>;
}> = Object.create(null);
for (const definition of document.definitions) {
switch (definition.kind) {
case Kind.OPERATION_DEFINITION:
Expand All @@ -462,9 +474,24 @@ export function buildExecutionContext(
operation = definition;
}
break;
case Kind.FRAGMENT_DEFINITION:
fragments[definition.name.value] = definition;
case Kind.FRAGMENT_DEFINITION: {
const signatures: Array<GraphQLFragmentSignature> = [];
if (definition.variableDefinitions) {
for (const varDef of definition.variableDefinitions) {
const varType = typeFromAST(schema, varDef.type);
if (isInputType(varType)) {
const signature: GraphQLFragmentSignature = {
name: varDef.variable.name.value,
type: varType,
defaultValue: valueFromAST(varDef.defaultValue, varType),
};
signatures.push(signature);
}
}
}
fragments[definition.name.value] = { definition, signatures };
break;
}
default:
// ignore non-executable definitions
}
Expand Down Expand Up @@ -807,7 +834,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,
Expand Down
82 changes: 6 additions & 76 deletions src/execution/values.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ import type { GraphQLSchema } from '../type/schema.js';
import { coerceInputValue } from '../utilities/coerceInputValue.js';
import { typeFromAST } from '../utilities/typeFromAST.js';
import { valueFromAST } from '../utilities/valueFromAST.js';
import { valueFromASTUntyped } from '../utilities/valueFromASTUntyped.js';

import type { GraphQLFragmentSignature } from './collectFields.js';

type CoercedVariableValues =
| { errors: ReadonlyArray<GraphQLError>; coerced?: never }
Expand Down Expand Up @@ -159,8 +160,8 @@ function coerceVariableValues(
* Object prototype.
*/
export function getArgumentValues(
node: FieldNode | DirectiveNode,
argDefs: ReadonlyArray<GraphQLArgument>,
node: FieldNode | DirectiveNode | FragmentSpreadNode,
argDefs: ReadonlyArray<GraphQLArgument | GraphQLFragmentSignature>,
variableValues: Maybe<ObjMap<unknown>>,
fragmentArgValues?: Maybe<ObjMap<unknown>>,
): { [argument: string]: unknown } {
Expand All @@ -183,6 +184,8 @@ export function getArgumentValues(
'was not provided.',
{ nodes: node },
);
} else {
coercedValues[name] = undefined;
}
continue;
}
Expand Down Expand Up @@ -248,79 +251,6 @@ export function getArgumentValues(
return coercedValues;
}

export function getArgumentValuesFromSpread(
/** NOTE: For error annotations only */
node: FragmentSpreadNode,
schema: GraphQLSchema,
fragmentVarDefs: ReadonlyArray<VariableDefinitionNode>,
variableValues: Maybe<ObjMap<unknown>>,
fragmentArgValues?: Maybe<ObjMap<unknown>>,
): { [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
Expand Down
12 changes: 9 additions & 3 deletions src/validation/rules/SingleFieldSubscriptionsRule.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ import type {
import { Kind } from '../../language/kinds.js';
import type { ASTVisitor } from '../../language/visitor.js';

import type { FieldGroup } from '../../execution/collectFields.js';
import type {
FieldGroup,
GraphQLFragmentSignature,
} from '../../execution/collectFields.js';
import { collectFields } from '../../execution/collectFields.js';

import type { ValidationContext } from '../ValidationContext.js';
Expand Down Expand Up @@ -41,10 +44,13 @@ export function SingleFieldSubscriptionsRule(
[variable: string]: any;
} = Object.create(null);
const document = context.getDocument();
const fragments: ObjMap<FragmentDefinitionNode> = Object.create(null);
const fragments: ObjMap<{
definition: FragmentDefinitionNode;
signatures: ReadonlyArray<GraphQLFragmentSignature>;
}> = 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, signatures: [] };
}
}
const { groupedFieldSet } = collectFields(
Expand Down

0 comments on commit e5744fc

Please sign in to comment.