Skip to content

Commit

Permalink
Preserve sources of variable values
Browse files Browse the repository at this point in the history
Depends on #3074

By way of introducing type `VariableValues`, allows `getVariableValues` to return both the coerced values as well as the original sources, which are then made available in `ExecutionContext`.

While variable sources are not used directly here, they're used directly in #3065. This PR is pulled out as a pre-req to aid review
  • Loading branch information
leebyron committed May 15, 2021
1 parent 705f154 commit b967e71
Show file tree
Hide file tree
Showing 11 changed files with 170 additions and 83 deletions.
4 changes: 3 additions & 1 deletion src/execution/execute.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import {
GraphQLObjectType,
} from '../type/definition';

import { VariableValues } from './values';

/**
* Terminology
*
Expand Down Expand Up @@ -55,7 +57,7 @@ export interface ExecutionContext {
contextValue: unknown;
fragments: ObjMap<FragmentDefinitionNode>;
operation: OperationDefinitionNode;
variableValues: { [key: string]: unknown };
variableValues: VariableValues;
fieldResolver: GraphQLFieldResolver<any, any>;
typeResolver: GraphQLTypeResolver<any, any>;
errors: Array<GraphQLError>;
Expand Down
13 changes: 7 additions & 6 deletions src/execution/execute.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ import {
import { typeFromAST } from '../utilities/typeFromAST';
import { getOperationRootType } from '../utilities/getOperationRootType';

import type { VariableValues } from './values';
import {
getVariableValues,
getArgumentValues,
Expand Down Expand Up @@ -98,7 +99,7 @@ export type ExecutionContext = {|
rootValue: mixed,
contextValue: mixed,
operation: OperationDefinitionNode,
variableValues: { [variable: string]: mixed, ... },
variableValues: VariableValues,
fieldResolver: GraphQLFieldResolver<any, any>,
typeResolver: GraphQLTypeResolver<any, any>,
errors: Array<GraphQLError>,
Expand Down Expand Up @@ -295,15 +296,15 @@ export function buildExecutionContext(
// istanbul ignore next (See: 'https://github.com/graphql/graphql-js/issues/2203')
const variableDefinitions = operation.variableDefinitions ?? [];

const coercedVariableValues = getVariableValues(
const variableValuesOrErrors = getVariableValues(
schema,
variableDefinitions,
rawVariableValues ?? {},
{ maxErrors: 50 },
);

if (coercedVariableValues.errors) {
return coercedVariableValues.errors;
if (variableValuesOrErrors.errors) {
return variableValuesOrErrors.errors;
}

// $FlowFixMe[incompatible-return]
Expand All @@ -313,7 +314,7 @@ export function buildExecutionContext(
rootValue,
contextValue,
operation,
variableValues: coercedVariableValues.coerced,
variableValues: variableValuesOrErrors.variableValues,
fieldResolver: fieldResolver ?? defaultFieldResolver,
typeResolver: typeResolver ?? defaultTypeResolver,
errors: [],
Expand Down Expand Up @@ -679,7 +680,7 @@ export function buildResolveInfo(
fragments: exeContext.fragments,
rootValue: exeContext.rootValue,
operation: exeContext.operation,
variableValues: exeContext.variableValues,
variableValues: exeContext.variableValues.coerced,
};
}

Expand Down
25 changes: 17 additions & 8 deletions src/execution/values.d.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Maybe } from '../jsutils/Maybe';
import { ObjMap } from '../jsutils/ObjMap';
import { ReadOnlyObjMap } from '../jsutils/ObjMap';

import { GraphQLError } from '../error/GraphQLError';
import {
Expand All @@ -10,11 +10,20 @@ import {

import { GraphQLDirective } from '../type/directives';
import { GraphQLSchema } from '../type/schema';
import { GraphQLField } from '../type/definition';
import { GraphQLField, GraphQLInputType } from '../type/definition';

type CoercedVariableValues =
| { errors: ReadonlyArray<GraphQLError>; coerced?: never }
| { errors?: never; coerced: { [key: string]: unknown } };
export type VariableValues = {
readonly sources: ReadOnlyObjMap<{
readonly variable: VariableDefinitionNode;
readonly type: GraphQLInputType;
readonly value: unknown;
}>;
readonly coerced: ReadOnlyObjMap<unknown>;
};

type VariableValuesOrErrors =
| { variableValues: VariableValues; errors?: never }
| { errors: ReadonlyArray<GraphQLError>; variableValues?: never };

/**
* Prepares an object map of variableValues of the correct type based on the
Expand All @@ -30,7 +39,7 @@ export function getVariableValues(
varDefNodes: ReadonlyArray<VariableDefinitionNode>,
inputs: { [key: string]: unknown },
options?: { maxErrors?: number },
): CoercedVariableValues;
): VariableValuesOrErrors;

/**
* Prepares an object map of argument values given a list of argument
Expand All @@ -43,7 +52,7 @@ export function getVariableValues(
export function getArgumentValues(
def: GraphQLField<unknown, unknown> | GraphQLDirective,
node: FieldNode | DirectiveNode,
variableValues?: Maybe<ObjMap<unknown>>,
variableValues?: Maybe<VariableValues>,
): { [key: string]: unknown };

/**
Expand All @@ -62,5 +71,5 @@ export function getDirectiveValues(
node: {
readonly directives?: ReadonlyArray<DirectiveNode>;
},
variableValues?: Maybe<ObjMap<unknown>>,
variableValues?: Maybe<VariableValues>,
): undefined | { [key: string]: unknown };
62 changes: 38 additions & 24 deletions src/execution/values.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { ObjMap } from '../jsutils/ObjMap';
import { keyMap } from '../jsutils/keyMap';
import type { ReadOnlyObjMap, ReadOnlyObjMapLike } from '../jsutils/ObjMap';
import { inspect } from '../jsutils/inspect';
import { keyMap } from '../jsutils/keyMap';
import { printPathArray } from '../jsutils/printPathArray';

import { GraphQLError } from '../error/GraphQLError';
Expand All @@ -14,7 +14,7 @@ import { Kind } from '../language/kinds';
import { print } from '../language/printer';

import type { GraphQLSchema } from '../type/schema';
import type { GraphQLField } from '../type/definition';
import type { GraphQLInputType, GraphQLField } from '../type/definition';
import type { GraphQLDirective } from '../type/directives';
import { isInputType, isNonNullType } from '../type/definition';

Expand All @@ -25,9 +25,18 @@ import {
coerceDefaultValue,
} from '../utilities/coerceInputValue';

type CoercedVariableValues =
| {| errors: $ReadOnlyArray<GraphQLError> |}
| {| coerced: { [variable: string]: mixed, ... } |};
export type VariableValues = {|
+sources: ReadOnlyObjMap<{|
+variable: VariableDefinitionNode,
+type: GraphQLInputType,
+value: mixed,
|}>,
+coerced: ReadOnlyObjMap<mixed>,
|};

type VariableValuesOrErrors =
| {| variableValues: VariableValues |}
| {| errors: $ReadOnlyArray<GraphQLError> |};

/**
* Prepares an object map of variableValues of the correct type based on the
Expand All @@ -43,13 +52,13 @@ type CoercedVariableValues =
export function getVariableValues(
schema: GraphQLSchema,
varDefNodes: $ReadOnlyArray<VariableDefinitionNode>,
inputs: { +[variable: string]: mixed, ... },
inputs: ReadOnlyObjMapLike<mixed>,
options?: {| maxErrors?: number |},
): CoercedVariableValues {
): VariableValuesOrErrors {
const errors = [];
const maxErrors = options?.maxErrors;
try {
const coerced = coerceVariableValues(
const variableValues = coerceVariableValues(
schema,
varDefNodes,
inputs,
Expand All @@ -64,7 +73,7 @@ export function getVariableValues(
);

if (errors.length === 0) {
return { coerced };
return { variableValues };
}
} catch (error) {
errors.push(error);
Expand All @@ -76,10 +85,11 @@ export function getVariableValues(
function coerceVariableValues(
schema: GraphQLSchema,
varDefNodes: $ReadOnlyArray<VariableDefinitionNode>,
inputs: { +[variable: string]: mixed, ... },
inputs: ReadOnlyObjMapLike<mixed>,
onError: (error: GraphQLError) => void,
): { [variable: string]: mixed, ... } {
const coercedValues = {};
): VariableValues {
const sources = Object.create(null);
const coerced = Object.create(null);
for (const varDefNode of varDefNodes) {
const varName = varDefNode.variable.name.value;
const varType = typeFromAST(schema, varDefNode.type);
Expand All @@ -97,11 +107,14 @@ function coerceVariableValues(
}

if (!hasOwnProperty(inputs, varName)) {
if (varDefNode.defaultValue) {
coercedValues[varName] = coerceInputLiteral(
varDefNode.defaultValue,
varType,
);
const defaultValue = varDefNode.defaultValue;
if (defaultValue) {
sources[varName] = {
variable: varDefNode,
type: varType,
value: undefined,
};
coerced[varName] = coerceInputLiteral(defaultValue, varType);
} else if (isNonNullType(varType)) {
const varTypeStr = inspect(varType);
onError(
Expand All @@ -126,7 +139,8 @@ function coerceVariableValues(
continue;
}

coercedValues[varName] = coerceInputValue(
sources[varName] = { variable: varDefNode, type: varType, value };
coerced[varName] = coerceInputValue(
value,
varType,
(path, invalidValue, error) => {
Expand All @@ -149,7 +163,7 @@ function coerceVariableValues(
);
}

return coercedValues;
return { sources, coerced };
}

/**
Expand All @@ -165,7 +179,7 @@ function coerceVariableValues(
export function getArgumentValues(
def: GraphQLField<mixed, mixed> | GraphQLDirective,
node: FieldNode | DirectiveNode,
variableValues?: ?ObjMap<mixed>,
variableValues?: ?VariableValues,
): { [argument: string]: mixed, ... } {
const coercedValues = {};

Expand Down Expand Up @@ -201,7 +215,7 @@ export function getArgumentValues(
const variableName = valueNode.name.value;
if (
variableValues == null ||
!hasOwnProperty(variableValues, variableName)
variableValues.coerced[variableName] === undefined
) {
if (argDef.defaultValue) {
coercedValues[name] = coerceDefaultValue(
Expand All @@ -217,7 +231,7 @@ export function getArgumentValues(
}
continue;
}
isNull = variableValues[variableName] == null;
isNull = variableValues.coerced[variableName] == null;
}

if (isNull && isNonNullType(argType)) {
Expand Down Expand Up @@ -257,7 +271,7 @@ export function getArgumentValues(
export function getDirectiveValues(
directiveDef: GraphQLDirective,
node: { +directives?: $ReadOnlyArray<DirectiveNode>, ... },
variableValues?: ?ObjMap<mixed>,
variableValues?: ?VariableValues,
): void | { [argument: string]: mixed, ... } {
// istanbul ignore next (See: 'https://github.com/graphql/graphql-js/issues/2203')
const directiveNode = node.directives?.find(
Expand Down
8 changes: 4 additions & 4 deletions src/type/definition.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { Maybe } from '../jsutils/Maybe';

import { PromiseOrValue } from '../jsutils/PromiseOrValue';
import { Path } from '../jsutils/Path';
import { ObjMap } from '../jsutils/ObjMap';
import { ObjMap, ReadOnlyObjMap } from '../jsutils/ObjMap';

import {
ScalarTypeDefinitionNode,
Expand Down Expand Up @@ -345,7 +345,7 @@ export type GraphQLScalarValueParser<TInternal> = (
) => Maybe<TInternal>;
export type GraphQLScalarLiteralParser<TInternal> = (
valueNode: ValueNode,
variables: Maybe<ObjMap<unknown>>,
variables: Maybe<ReadOnlyObjMap<unknown>>,
) => Maybe<TInternal>;

export interface GraphQLScalarTypeConfig<TInternal, TExternal> {
Expand Down Expand Up @@ -487,7 +487,7 @@ export interface GraphQLResolveInfo {
readonly fragments: ObjMap<FragmentDefinitionNode>;
readonly rootValue: unknown;
readonly operation: OperationDefinitionNode;
readonly variableValues: { [variableName: string]: unknown };
readonly variableValues: ReadOnlyObjMap<unknown>;
}

/**
Expand Down Expand Up @@ -789,7 +789,7 @@ export class GraphQLEnumType {
parseValue(value: unknown): Maybe<any>;
parseLiteral(
valueNode: ValueNode,
_variables: Maybe<ObjMap<unknown>>,
_variables: Maybe<ReadOnlyObjMap<unknown>>,
): Maybe<any>;

toConfig(): GraphQLEnumTypeConfig & {
Expand Down
9 changes: 6 additions & 3 deletions src/type/definition.js
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,7 @@ export type GraphQLScalarValueParser<TInternal> = (

export type GraphQLScalarLiteralParser<TInternal> = (
valueNode: ValueNode,
variables: ?ObjMap<mixed>,
variables: ?ReadOnlyObjMap<mixed>,
) => ?TInternal;

export type GraphQLScalarTypeConfig<TInternal, TExternal> = {|
Expand Down Expand Up @@ -909,7 +909,7 @@ export type GraphQLResolveInfo = {|
+fragments: ObjMap<FragmentDefinitionNode>,
+rootValue: mixed,
+operation: OperationDefinitionNode,
+variableValues: { [variable: string]: mixed, ... },
+variableValues: ReadOnlyObjMap<mixed>,
|};

export type GraphQLFieldConfig<
Expand Down Expand Up @@ -1351,7 +1351,10 @@ export class GraphQLEnumType /* <T> */ {
return enumValue.value;
}

parseLiteral(valueNode: ValueNode, _variables: ?ObjMap<mixed>): ?any /* T */ {
parseLiteral(
valueNode: ValueNode,
_variables: ?ReadOnlyObjMap<mixed>,
): ?any /* T */ {
// Note: variables will be resolved to a value before calling this function.
if (valueNode.kind !== Kind.ENUM) {
const valueStr = print(valueNode);
Expand Down
Loading

0 comments on commit b967e71

Please sign in to comment.