Skip to content

Commit 36b3407

Browse files
authored
Return normal objects for args and vars. (#1056)
This is a follow up to #1048 which addresses an issue where flow typed resolver functions would need to be aware of the fact that arguments were prototype-less. That's something we maintain for internal usage, but is more burdensome for external use. Instead, this is just more cautious when using these values internally, using hasOwnProperty, but now returns prototypefull objects to user code. Also follows up with a few additional conversions to ObjMap, such that the only remaining uses of `{[string]: mixed}` are for variables and arguments.
1 parent efcfae6 commit 36b3407

File tree

7 files changed

+47
-32
lines changed

7 files changed

+47
-32
lines changed

src/execution/execute.js

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ export type ExecutionContext = {
8989
rootValue: mixed;
9090
contextValue: mixed;
9191
operation: OperationDefinitionNode;
92-
variableValues: ObjMap<mixed>,
92+
variableValues: {[variable: string]: mixed},
9393
fieldResolver: GraphQLFieldResolver<any, any>;
9494
errors: Array<GraphQLError>;
9595
};
@@ -102,15 +102,15 @@ export type ExecutionContext = {
102102
*/
103103
export type ExecutionResult = {
104104
errors?: Array<GraphQLError>;
105-
data?: ?{[key: string]: mixed};
105+
data?: ObjMap<mixed>;
106106
};
107107

108108
export type ExecutionArgs = {|
109109
schema: GraphQLSchema,
110110
document: DocumentNode,
111111
rootValue?: mixed,
112112
contextValue?: mixed,
113-
variableValues?: ?{[key: string]: mixed},
113+
variableValues?: ?{[variable: string]: mixed},
114114
operationName?: ?string,
115115
fieldResolver?: ?GraphQLFieldResolver<any, any>
116116
|};
@@ -135,7 +135,7 @@ declare function execute(
135135
document: DocumentNode,
136136
rootValue?: mixed,
137137
contextValue?: mixed,
138-
variableValues?: ?{[key: string]: mixed},
138+
variableValues?: ?{[variable: string]: mixed},
139139
operationName?: ?string,
140140
fieldResolver?: ?GraphQLFieldResolver<any, any>
141141
): Promise<ExecutionResult>;
@@ -249,7 +249,7 @@ export function addPath(prev: ResponsePath, key: string | number) {
249249
export function assertValidExecutionArguments(
250250
schema: GraphQLSchema,
251251
document: DocumentNode,
252-
rawVariableValues: ?{[key: string]: mixed}
252+
rawVariableValues: ?ObjMap<mixed>
253253
): void {
254254
invariant(schema, 'Must provide schema');
255255
invariant(document, 'Must provide document');
@@ -615,8 +615,8 @@ function doesFragmentConditionMatch(
615615
}
616616

617617
/**
618-
* This function transforms a JS object `{[key: string]: Promise<T>}` into
619-
* a `Promise<{[key: string]: T}>`
618+
* This function transforms a JS object `ObjMap<Promise<T>>` into
619+
* a `Promise<ObjMap<T>>`
620620
*
621621
* This is akin to bluebird's `Promise.props`, but implemented only using
622622
* `Promise.all` so it will work with any implementation of ES6 promises.

src/execution/values.js

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import {
2929
GraphQLList,
3030
GraphQLNonNull,
3131
} from '../type/definition';
32+
import type { ObjMap } from '../jsutils/ObjMap';
3233
import type {
3334
GraphQLInputType,
3435
GraphQLField
@@ -47,13 +48,17 @@ import type {
4748
* Prepares an object map of variableValues of the correct type based on the
4849
* provided variable definitions and arbitrary input. If the input cannot be
4950
* parsed to match the variable definitions, a GraphQLError will be thrown.
51+
*
52+
* Note: The returned value is a plain Object with a prototype, since it is
53+
* exposed to user code. Care should be taken to not pull values from the
54+
* Object prototype.
5055
*/
5156
export function getVariableValues(
5257
schema: GraphQLSchema,
5358
varDefNodes: Array<VariableDefinitionNode>,
54-
inputs: { [key: string]: mixed, __proto__: null }
55-
): { [key: string]: mixed, __proto__: null } {
56-
const coercedValues = Object.create(null);
59+
inputs: ObjMap<mixed>
60+
): { [variable: string]: mixed } {
61+
const coercedValues = {};
5762
for (let i = 0; i < varDefNodes.length; i++) {
5863
const varDefNode = varDefNodes[i];
5964
const varName = varDefNode.variable.name.value;
@@ -101,18 +106,22 @@ export function getVariableValues(
101106
/**
102107
* Prepares an object map of argument values given a list of argument
103108
* definitions and list of argument AST nodes.
109+
*
110+
* Note: The returned value is a plain Object with a prototype, since it is
111+
* exposed to user code. Care should be taken to not pull values from the
112+
* Object prototype.
104113
*/
105114
export function getArgumentValues(
106115
def: GraphQLField<*, *> | GraphQLDirective,
107116
node: FieldNode | DirectiveNode,
108-
variableValues?: ?{ [key: string]: mixed, __proto__: null }
109-
): { [key: string]: mixed, __proto__: null } {
117+
variableValues?: ?ObjMap<mixed>
118+
): { [argument: string]: mixed } {
119+
const coercedValues = {};
110120
const argDefs = def.args;
111121
const argNodes = node.arguments;
112122
if (!argDefs || !argNodes) {
113-
return {};
123+
return coercedValues;
114124
}
115-
const coercedValues = Object.create(null);
116125
const argNodeMap = keyMap(argNodes, arg => arg.name.value);
117126
for (let i = 0; i < argDefs.length; i++) {
118127
const argDef = argDefs[i];
@@ -132,7 +141,11 @@ export function getArgumentValues(
132141
}
133142
} else if (argumentNode.value.kind === Kind.VARIABLE) {
134143
const variableName = (argumentNode.value: VariableNode).name.value;
135-
if (variableValues && !isInvalid(variableValues[variableName])) {
144+
if (
145+
variableValues &&
146+
Object.prototype.hasOwnProperty.call(variableValues, variableName) &&
147+
!isInvalid(variableValues[variableName])
148+
) {
136149
// Note: this does not check that this variable value is correct.
137150
// This assumes that this query has been validated and the variable
138151
// usage here is of the correct type.
@@ -170,12 +183,16 @@ export function getArgumentValues(
170183
* of variable values.
171184
*
172185
* If the directive does not exist on the node, returns undefined.
186+
*
187+
* Note: The returned value is a plain Object with a prototype, since it is
188+
* exposed to user code. Care should be taken to not pull values from the
189+
* Object prototype.
173190
*/
174191
export function getDirectiveValues(
175192
directiveDef: GraphQLDirective,
176193
node: { directives?: ?Array<DirectiveNode> },
177-
variableValues?: ?{ [key: string]: mixed, __proto__: null }
178-
): void | { [key: string]: mixed, __proto__: null } {
194+
variableValues?: ?ObjMap<mixed>
195+
): void | { [argument: string]: mixed } {
179196
const directiveNode = node.directives && find(
180197
node.directives,
181198
directive => directive.name.value === directiveDef.name

src/graphql.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import { parse } from './language/parser';
1111
import { validate } from './validation/validate';
1212
import { execute } from './execution/execute';
13+
import type { ObjMap } from './jsutils/ObjMap';
1314
import type { Source } from './language/source';
1415
import type { GraphQLFieldResolver } from './type/definition';
1516
import type { GraphQLSchema } from './type/schema';
@@ -50,7 +51,7 @@ declare function graphql({|
5051
source: string | Source,
5152
rootValue?: mixed,
5253
contextValue?: mixed,
53-
variableValues?: ?{[key: string]: mixed},
54+
variableValues?: ?ObjMap<mixed>,
5455
operationName?: ?string,
5556
fieldResolver?: ?GraphQLFieldResolver<any, any>
5657
|}, ..._: []): Promise<ExecutionResult>;
@@ -60,7 +61,7 @@ declare function graphql(
6061
source: Source | string,
6162
rootValue?: mixed,
6263
contextValue?: mixed,
63-
variableValues?: ?{[key: string]: mixed},
64+
variableValues?: ?ObjMap<mixed>,
6465
operationName?: ?string,
6566
fieldResolver?: ?GraphQLFieldResolver<any, any>
6667
): Promise<ExecutionResult>;

src/subscription/subscribe.js

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import { GraphQLSchema } from '../type/schema';
2626
import invariant from '../jsutils/invariant';
2727
import mapAsyncIterator from './mapAsyncIterator';
2828

29+
import type { ObjMap } from '../jsutils/ObjMap';
2930
import type { ExecutionResult } from '../execution/execute';
3031
import type { DocumentNode } from '../language/ast';
3132
import type { GraphQLFieldResolver } from '../type/definition';
@@ -55,7 +56,7 @@ declare function subscribe({|
5556
document: DocumentNode,
5657
rootValue?: mixed,
5758
contextValue?: mixed,
58-
variableValues?: ?{[key: string]: mixed},
59+
variableValues?: ?ObjMap<mixed>,
5960
operationName?: ?string,
6061
fieldResolver?: ?GraphQLFieldResolver<any, any>,
6162
subscribeFieldResolver?: ?GraphQLFieldResolver<any, any>
@@ -66,7 +67,7 @@ declare function subscribe(
6667
document: DocumentNode,
6768
rootValue?: mixed,
6869
contextValue?: mixed,
69-
variableValues?: ?{[key: string]: mixed},
70+
variableValues?: ?ObjMap<mixed>,
7071
operationName?: ?string,
7172
fieldResolver?: ?GraphQLFieldResolver<any, any>,
7273
subscribeFieldResolver?: ?GraphQLFieldResolver<any, any>
@@ -191,7 +192,7 @@ export function createSourceEventStream(
191192
document: DocumentNode,
192193
rootValue?: mixed,
193194
contextValue?: mixed,
194-
variableValues?: ?{[key: string]: mixed},
195+
variableValues?: ObjMap<mixed>,
195196
operationName?: ?string,
196197
fieldResolver?: ?GraphQLFieldResolver<any, any>
197198
): Promise<AsyncIterable<mixed>> {

src/type/definition.js

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -625,7 +625,7 @@ export type GraphQLIsTypeOfFn<TSource, TContext> = (
625625

626626
export type GraphQLFieldResolver<TSource, TContext> = (
627627
source: TSource,
628-
args: ObjMap<any>,
628+
args: {[argument: string]: any},
629629
context: TContext,
630630
info: GraphQLResolveInfo
631631
) => mixed;
@@ -640,7 +640,7 @@ export type GraphQLResolveInfo = {
640640
fragments: ObjMap<FragmentDefinitionNode>;
641641
rootValue: mixed;
642642
operation: OperationDefinitionNode;
643-
variableValues: ObjMap<mixed>;
643+
variableValues: {[variable: string]: mixed};
644644
};
645645

646646
export type ResponsePath = { prev: ResponsePath, key: string | number } | void;
@@ -798,7 +798,6 @@ export class GraphQLUnionType {
798798

799799
_typeConfig: GraphQLUnionTypeConfig<*, *>;
800800
_types: Array<GraphQLObjectType>;
801-
_possibleTypeNames: {[typeName: string]: boolean};
802801

803802
constructor(config: GraphQLUnionTypeConfig<*, *>): void {
804803
assertValidName(config.name);

src/type/schema.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ export class GraphQLSchema {
6363
_directives: Array<GraphQLDirective>;
6464
_typeMap: TypeMap;
6565
_implementations: ObjMap<Array<GraphQLObjectType>>;
66-
_possibleTypeMap: ?ObjMap<{[possibleName: string]: boolean}>;
66+
_possibleTypeMap: ?ObjMap<ObjMap<boolean>>;
6767

6868
constructor(config: GraphQLSchemaConfig): void {
6969
invariant(

src/utilities/separateOperations.js

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77
* @flow
88
*/
99

10-
import type {ObjMap} from '../jsutils/ObjMap';
1110
import { visit } from '../language/visitor';
11+
import type {ObjMap} from '../jsutils/ObjMap';
1212
import type {
1313
DocumentNode,
1414
OperationDefinitionNode,
@@ -76,10 +76,7 @@ export function separateOperations(
7676
return separatedDocumentASTs;
7777
}
7878

79-
type DepGraph = {
80-
[from: string]: {[to: string]: boolean, __proto__: null},
81-
__proto__: null,
82-
};
79+
type DepGraph = ObjMap<ObjMap<boolean>>;
8380

8481
// Provides the empty string for anonymous operations.
8582
function opName(operation: OperationDefinitionNode): string {
@@ -89,7 +86,7 @@ function opName(operation: OperationDefinitionNode): string {
8986
// From a dependency graph, collects a list of transitive dependencies by
9087
// recursing through a dependency graph.
9188
function collectTransitiveDependencies(
92-
collected: {[key: string]: boolean, __proto__: null},
89+
collected: ObjMap<boolean>,
9390
depGraph: DepGraph,
9491
fromName: string
9592
): void {

0 commit comments

Comments
 (0)