Skip to content

Commit 461392d

Browse files
leebyronleebyron
authored
Move schema validation into separate step (type constructors) (#1132)
This is the second step of moving work from type constructors to the schema validation function.
1 parent cddcad3 commit 461392d

10 files changed

Lines changed: 1752 additions & 1575 deletions

File tree

‎src/type/__tests__/definition-test.js‎

Lines changed: 735 additions & 19 deletions
Large diffs are not rendered by default.

‎src/type/__tests__/validation-test.js‎

Lines changed: 497 additions & 1116 deletions
Large diffs are not rendered by default.

‎src/type/definition.js‎

Lines changed: 30 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import invariant from '../jsutils/invariant';
1212
import isInvalid from '../jsutils/isInvalid';
1313
import type { ObjMap } from '../jsutils/ObjMap';
1414
import * as Kind from '../language/kinds';
15-
import { assertValidName } from '../utilities/assertValidName';
1615
import { valueFromASTUntyped } from '../utilities/valueFromASTUntyped';
1716
import type {
1817
ScalarTypeDefinitionNode,
@@ -454,10 +453,11 @@ export class GraphQLScalarType {
454453
_scalarConfig: GraphQLScalarTypeConfig<*, *>;
455454

456455
constructor(config: GraphQLScalarTypeConfig<*, *>): void {
457-
assertValidName(config.name);
458456
this.name = config.name;
459457
this.description = config.description;
460458
this.astNode = config.astNode;
459+
this._scalarConfig = config;
460+
invariant(typeof config.name === 'string', 'Must provide name.');
461461
invariant(
462462
typeof config.serialize === 'function',
463463
`${this.name} must provide "serialize" function. If this custom Scalar ` +
@@ -472,7 +472,6 @@ export class GraphQLScalarType {
472472
'functions.',
473473
);
474474
}
475-
this._scalarConfig = config;
476475
}
477476

478477
// Serializes an internal value to include in a response.
@@ -563,27 +562,27 @@ export class GraphQLObjectType {
563562
name: string;
564563
description: ?string;
565564
astNode: ?ObjectTypeDefinitionNode;
566-
extensionASTNodes: ?Array<ObjectTypeExtensionNode>;
565+
extensionASTNodes: ?$ReadOnlyArray<ObjectTypeExtensionNode>;
567566
isTypeOf: ?GraphQLIsTypeOfFn<*, *>;
568567

569568
_typeConfig: GraphQLObjectTypeConfig<*, *>;
570569
_fields: GraphQLFieldMap<*, *>;
571570
_interfaces: Array<GraphQLInterfaceType>;
572571

573572
constructor(config: GraphQLObjectTypeConfig<*, *>): void {
574-
assertValidName(config.name, config.isIntrospection);
575573
this.name = config.name;
576574
this.description = config.description;
577575
this.astNode = config.astNode;
578576
this.extensionASTNodes = config.extensionASTNodes;
577+
this.isTypeOf = config.isTypeOf;
578+
this._typeConfig = config;
579+
invariant(typeof config.name === 'string', 'Must provide name.');
579580
if (config.isTypeOf) {
580581
invariant(
581582
typeof config.isTypeOf === 'function',
582583
`${this.name} must provide "isTypeOf" as a function.`,
583584
);
584585
}
585-
this.isTypeOf = config.isTypeOf;
586-
this._typeConfig = config;
587586
}
588587

589588
getFields(): GraphQLFieldMap<*, *> {
@@ -616,10 +615,7 @@ function defineInterfaces(
616615
type: GraphQLObjectType,
617616
interfacesThunk: Thunk<?Array<GraphQLInterfaceType>>,
618617
): Array<GraphQLInterfaceType> {
619-
const interfaces = resolveThunk(interfacesThunk);
620-
if (!interfaces) {
621-
return [];
622-
}
618+
const interfaces = resolveThunk(interfacesThunk) || [];
623619
invariant(
624620
Array.isArray(interfaces),
625621
`${type.name} interfaces must be an Array or a function which returns ` +
@@ -632,23 +628,15 @@ function defineFieldMap<TSource, TContext>(
632628
type: GraphQLNamedType,
633629
fieldsThunk: Thunk<GraphQLFieldConfigMap<TSource, TContext>>,
634630
): GraphQLFieldMap<TSource, TContext> {
635-
const fieldMap = resolveThunk(fieldsThunk);
631+
const fieldMap = resolveThunk(fieldsThunk) || {};
636632
invariant(
637633
isPlainObj(fieldMap),
638634
`${type.name} fields must be an object with field names as keys or a ` +
639635
'function which returns such an object.',
640636
);
641637

642-
const fieldNames = Object.keys(fieldMap);
643-
invariant(
644-
fieldNames.length > 0,
645-
`${type.name} fields must be an object with field names as keys or a ` +
646-
'function which returns such an object.',
647-
);
648-
649638
const resultFieldMap = Object.create(null);
650-
fieldNames.forEach(fieldName => {
651-
assertValidName(fieldName);
639+
Object.keys(fieldMap).forEach(fieldName => {
652640
const fieldConfig = fieldMap[fieldName];
653641
invariant(
654642
isPlainObj(fieldConfig),
@@ -664,11 +652,6 @@ function defineFieldMap<TSource, TContext>(
664652
isDeprecated: Boolean(fieldConfig.deprecationReason),
665653
name: fieldName,
666654
};
667-
invariant(
668-
isOutputType(field.type),
669-
`${type.name}.${fieldName} field type must be Output Type but ` +
670-
`got: ${String(field.type)}.`,
671-
);
672655
invariant(
673656
isValidResolver(field.resolve),
674657
`${type.name}.${fieldName} field resolver must be a function if ` +
@@ -684,13 +667,7 @@ function defineFieldMap<TSource, TContext>(
684667
'names as keys.',
685668
);
686669
field.args = Object.keys(argsConfig).map(argName => {
687-
assertValidName(argName);
688670
const arg = argsConfig[argName];
689-
invariant(
690-
isInputType(arg.type),
691-
`${type.name}.${fieldName}(${argName}:) argument type must be ` +
692-
`Input Type but got: ${String(arg.type)}.`,
693-
);
694671
return {
695672
name: argName,
696673
description: arg.description === undefined ? null : arg.description,
@@ -720,9 +697,8 @@ export type GraphQLObjectTypeConfig<TSource, TContext> = {
720697
fields: Thunk<GraphQLFieldConfigMap<TSource, TContext>>,
721698
isTypeOf?: ?GraphQLIsTypeOfFn<TSource, TContext>,
722699
description?: ?string,
723-
isIntrospection?: boolean,
724700
astNode?: ?ObjectTypeDefinitionNode,
725-
extensionASTNodes?: ?Array<ObjectTypeExtensionNode>,
701+
extensionASTNodes?: ?$ReadOnlyArray<ObjectTypeExtensionNode>,
726702
};
727703

728704
export type GraphQLTypeResolver<TSource, TContext> = (
@@ -831,26 +807,26 @@ export class GraphQLInterfaceType {
831807
name: string;
832808
description: ?string;
833809
astNode: ?InterfaceTypeDefinitionNode;
834-
extensionASTNodes: ?Array<InterfaceTypeExtensionNode>;
810+
extensionASTNodes: ?$ReadOnlyArray<InterfaceTypeExtensionNode>;
835811
resolveType: ?GraphQLTypeResolver<*, *>;
836812

837813
_typeConfig: GraphQLInterfaceTypeConfig<*, *>;
838814
_fields: GraphQLFieldMap<*, *>;
839815

840816
constructor(config: GraphQLInterfaceTypeConfig<*, *>): void {
841-
assertValidName(config.name);
842817
this.name = config.name;
843818
this.description = config.description;
844819
this.astNode = config.astNode;
845820
this.extensionASTNodes = config.extensionASTNodes;
821+
this.resolveType = config.resolveType;
822+
this._typeConfig = config;
823+
invariant(typeof config.name === 'string', 'Must provide name.');
846824
if (config.resolveType) {
847825
invariant(
848826
typeof config.resolveType === 'function',
849827
`${this.name} must provide "resolveType" as a function.`,
850828
);
851829
}
852-
this.resolveType = config.resolveType;
853-
this._typeConfig = config;
854830
}
855831

856832
getFields(): GraphQLFieldMap<*, *> {
@@ -883,7 +859,7 @@ export type GraphQLInterfaceTypeConfig<TSource, TContext> = {
883859
resolveType?: ?GraphQLTypeResolver<TSource, TContext>,
884860
description?: ?string,
885861
astNode?: ?InterfaceTypeDefinitionNode,
886-
extensionASTNodes?: ?Array<InterfaceTypeExtensionNode>,
862+
extensionASTNodes?: ?$ReadOnlyArray<InterfaceTypeExtensionNode>,
887863
};
888864

889865
/**
@@ -919,18 +895,18 @@ export class GraphQLUnionType {
919895
_types: Array<GraphQLObjectType>;
920896

921897
constructor(config: GraphQLUnionTypeConfig<*, *>): void {
922-
assertValidName(config.name);
923898
this.name = config.name;
924899
this.description = config.description;
925900
this.astNode = config.astNode;
901+
this.resolveType = config.resolveType;
902+
this._typeConfig = config;
903+
invariant(typeof config.name === 'string', 'Must provide name.');
926904
if (config.resolveType) {
927905
invariant(
928906
typeof config.resolveType === 'function',
929907
`${this.name} must provide "resolveType" as a function.`,
930908
);
931909
}
932-
this.resolveType = config.resolveType;
933-
this._typeConfig = config;
934910
}
935911

936912
getTypes(): Array<GraphQLObjectType> {
@@ -955,27 +931,12 @@ function defineTypes(
955931
unionType: GraphQLUnionType,
956932
typesThunk: Thunk<Array<GraphQLObjectType>>,
957933
): Array<GraphQLObjectType> {
958-
const types = resolveThunk(typesThunk);
959-
934+
const types = resolveThunk(typesThunk) || [];
960935
invariant(
961-
Array.isArray(types) && types.length > 0,
936+
Array.isArray(types),
962937
'Must provide Array of types or a function which returns ' +
963938
`such an array for Union ${unionType.name}.`,
964939
);
965-
const includedTypeNames = Object.create(null);
966-
types.forEach(objType => {
967-
invariant(
968-
isObjectType(objType),
969-
`${unionType.name} may only contain Object types, it cannot contain: ` +
970-
`${String(objType)}.`,
971-
);
972-
invariant(
973-
!includedTypeNames[objType.name],
974-
`${unionType.name} can include ${objType.name} type only once.`,
975-
);
976-
includedTypeNames[objType.name] = true;
977-
});
978-
979940
return types;
980941
}
981942

@@ -1025,15 +986,17 @@ export class GraphQLEnumType /* <T> */ {
1025986

1026987
constructor(config: GraphQLEnumTypeConfig /* <T> */): void {
1027988
this.name = config.name;
1028-
assertValidName(config.name, config.isIntrospection);
1029989
this.description = config.description;
1030990
this.astNode = config.astNode;
1031-
this._values = defineEnumValues(this, config.values);
1032991
this._enumConfig = config;
992+
invariant(typeof config.name === 'string', 'Must provide name.');
1033993
}
1034994

1035995
getValues(): Array<GraphQLEnumValue /* <T> */> {
1036-
return this._values;
996+
return (
997+
this._values ||
998+
(this._values = defineEnumValues(this, this._enumConfig.values))
999+
);
10371000
}
10381001

10391002
getValue(name: string): ?GraphQLEnumValue {
@@ -1108,18 +1071,7 @@ function defineEnumValues(
11081071
isPlainObj(valueMap),
11091072
`${type.name} values must be an object with value names as keys.`,
11101073
);
1111-
const valueNames = Object.keys(valueMap);
1112-
invariant(
1113-
valueNames.length > 0,
1114-
`${type.name} values must be an object with value names as keys.`,
1115-
);
1116-
return valueNames.map(valueName => {
1117-
assertValidName(valueName);
1118-
invariant(
1119-
['true', 'false', 'null'].indexOf(valueName) === -1,
1120-
`Name "${valueName}" can not be used as an Enum value.`,
1121-
);
1122-
1074+
return Object.keys(valueMap).map(valueName => {
11231075
const value = valueMap[valueName];
11241076
invariant(
11251077
isPlainObj(value),
@@ -1147,7 +1099,6 @@ export type GraphQLEnumTypeConfig /* <T> */ = {
11471099
values: GraphQLEnumValueConfigMap /* <T> */,
11481100
description?: ?string,
11491101
astNode?: ?EnumTypeDefinitionNode,
1150-
isIntrospection?: boolean,
11511102
};
11521103

11531104
export type GraphQLEnumValueConfigMap /* <T> */ = ObjMap<
@@ -1199,44 +1150,32 @@ export class GraphQLInputObjectType {
11991150
_fields: GraphQLInputFieldMap;
12001151

12011152
constructor(config: GraphQLInputObjectTypeConfig): void {
1202-
assertValidName(config.name);
12031153
this.name = config.name;
12041154
this.description = config.description;
12051155
this.astNode = config.astNode;
12061156
this._typeConfig = config;
1157+
invariant(typeof config.name === 'string', 'Must provide name.');
12071158
}
12081159

12091160
getFields(): GraphQLInputFieldMap {
12101161
return this._fields || (this._fields = this._defineFieldMap());
12111162
}
12121163

12131164
_defineFieldMap(): GraphQLInputFieldMap {
1214-
const fieldMap: any = resolveThunk(this._typeConfig.fields);
1165+
const fieldMap: any = resolveThunk(this._typeConfig.fields) || {};
12151166
invariant(
12161167
isPlainObj(fieldMap),
12171168
`${this.name} fields must be an object with field names as keys or a ` +
12181169
'function which returns such an object.',
12191170
);
1220-
const fieldNames = Object.keys(fieldMap);
1221-
invariant(
1222-
fieldNames.length > 0,
1223-
`${this.name} fields must be an object with field names as keys or a ` +
1224-
'function which returns such an object.',
1225-
);
12261171
const resultFieldMap = Object.create(null);
1227-
fieldNames.forEach(fieldName => {
1228-
assertValidName(fieldName);
1172+
Object.keys(fieldMap).forEach(fieldName => {
12291173
const field = {
12301174
...fieldMap[fieldName],
12311175
name: fieldName,
12321176
};
12331177
invariant(
1234-
isInputType(field.type),
1235-
`${this.name}.${fieldName} field type must be Input Type but ` +
1236-
`got: ${String(field.type)}.`,
1237-
);
1238-
invariant(
1239-
field.resolve == null,
1178+
!field.hasOwnProperty('resolve'),
12401179
`${this.name}.${fieldName} field type has a resolve property, but ` +
12411180
'Input Types cannot define resolvers.',
12421181
);

‎src/type/directives.js‎

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,14 @@
77
* @flow
88
*/
99

10-
import { isInputType } from './definition';
11-
import { GraphQLNonNull } from './wrappers';
12-
1310
import type {
1411
GraphQLFieldConfigArgumentMap,
1512
GraphQLArgument,
1613
} from './definition';
14+
import { GraphQLNonNull } from './wrappers';
1715
import { GraphQLString, GraphQLBoolean } from './scalars';
1816
import instanceOf from '../jsutils/instanceOf';
1917
import invariant from '../jsutils/invariant';
20-
import { assertValidName } from '../utilities/assertValidName';
2118
import type { DirectiveDefinitionNode } from '../language/ast';
2219
import {
2320
DirectiveLocation,
@@ -47,16 +44,15 @@ export class GraphQLDirective {
4744
astNode: ?DirectiveDefinitionNode;
4845

4946
constructor(config: GraphQLDirectiveConfig): void {
47+
this.name = config.name;
48+
this.description = config.description;
49+
this.locations = config.locations;
50+
this.astNode = config.astNode;
5051
invariant(config.name, 'Directive must be named.');
51-
assertValidName(config.name);
5252
invariant(
5353
Array.isArray(config.locations),
5454
'Must provide locations for directive.',
5555
);
56-
this.name = config.name;
57-
this.description = config.description;
58-
this.locations = config.locations;
59-
this.astNode = config.astNode;
6056

6157
const args = config.args;
6258
if (!args) {
@@ -67,13 +63,7 @@ export class GraphQLDirective {
6763
`@${config.name} args must be an object with argument names as keys.`,
6864
);
6965
this.args = Object.keys(args).map(argName => {
70-
assertValidName(argName);
7166
const arg = args[argName];
72-
invariant(
73-
isInputType(arg.type),
74-
`@${config.name}(${argName}:) argument type must be ` +
75-
`Input Type but got: ${String(arg.type)}.`,
76-
);
7767
return {
7868
name: argName,
7969
description: arg.description === undefined ? null : arg.description,

0 commit comments

Comments
 (0)