Skip to content

Commit b748679

Browse files
committed
Better Predicates
Introduces new predicate and assertion functions for each kind of type mirroring the existing ones for the higher order types. It also includes predicates for directives and schema. This should remove the need to use `instanceof` in any usage of GraphQL.js This consolidates the checking of instances which generalizes the cross-realm/multiple-module check so that the clearer error message is provided in nearly all scenarios, reducing confusion. This also replaces nearly all `instanceof` with new predicate functions internally. Fixes #1134
1 parent 5b62e07 commit b748679

31 files changed

+650
-445
lines changed

src/execution/__tests__/executor-test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ describe('Execute: Handles basic execution tasks', () => {
3939
execute({
4040
document: parse('{ field }'),
4141
}),
42-
).to.throw('Must provide schema');
42+
).to.throw('Expected undefined to be a GraphQL schema.');
4343
});
4444

4545
it('accepts an object with named properties as arguments', async () => {

src/execution/execute.js

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,15 @@ import {
2222
getDirectiveValues,
2323
} from './values';
2424
import {
25-
GraphQLObjectType,
25+
isObjectType,
2626
isAbstractType,
2727
isLeafType,
28+
isListType,
29+
isNonNullType,
2830
} from '../type/definition';
29-
import { GraphQLList, GraphQLNonNull } from '../type/wrappers';
31+
import type { GraphQLList } from '../type/wrappers';
3032
import type {
33+
GraphQLObjectType,
3134
GraphQLOutputType,
3235
GraphQLLeafType,
3336
GraphQLAbstractType,
@@ -807,7 +810,7 @@ function completeValueCatchingError(
807810
): mixed {
808811
// If the field type is non-nullable, then it is resolved without any
809812
// protection from errors, however it still properly locates the error.
810-
if (returnType instanceof GraphQLNonNull) {
813+
if (isNonNullType(returnType)) {
811814
return completeValueWithLocatedError(
812815
exeContext,
813816
returnType,
@@ -934,7 +937,7 @@ function completeValue(
934937

935938
// If field type is NonNull, complete for inner type, and throw field error
936939
// if result is null.
937-
if (returnType instanceof GraphQLNonNull) {
940+
if (isNonNullType(returnType)) {
938941
const completed = completeValue(
939942
exeContext,
940943
returnType.ofType,
@@ -959,7 +962,7 @@ function completeValue(
959962
}
960963

961964
// If field type is List, complete each item in the list with the inner type
962-
if (returnType instanceof GraphQLList) {
965+
if (isListType(returnType)) {
963966
return completeListValue(
964967
exeContext,
965968
returnType,
@@ -990,7 +993,7 @@ function completeValue(
990993
}
991994

992995
// If field type is Object, execute and complete all sub-selections.
993-
if (returnType instanceof GraphQLObjectType) {
996+
if (isObjectType(returnType)) {
994997
return completeObjectValue(
995998
exeContext,
996999
returnType,
@@ -1015,7 +1018,7 @@ function completeValue(
10151018
*/
10161019
function completeListValue(
10171020
exeContext: ExecutionContext,
1018-
returnType: GraphQLList<*>,
1021+
returnType: GraphQLList<GraphQLOutputType>,
10191022
fieldNodes: $ReadOnlyArray<FieldNode>,
10201023
info: GraphQLResolveInfo,
10211024
path: ResponsePath,
@@ -1138,7 +1141,7 @@ function ensureValidRuntimeType(
11381141
? exeContext.schema.getType(runtimeTypeOrName)
11391142
: runtimeTypeOrName;
11401143

1141-
if (!(runtimeType instanceof GraphQLObjectType)) {
1144+
if (!isObjectType(runtimeType)) {
11421145
throw new GraphQLError(
11431146
`Abstract type ${returnType.name} must resolve to an Object type at ` +
11441147
`runtime for field ${info.parentType.name}.${info.fieldName} with ` +

src/execution/values.js

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,7 @@ import { valueFromAST } from '../utilities/valueFromAST';
1717
import { isValidLiteralValue } from '../utilities/isValidLiteralValue';
1818
import * as Kind from '../language/kinds';
1919
import { print } from '../language/printer';
20-
import { isInputType } from '../type/definition';
21-
import { GraphQLNonNull } from '../type/wrappers';
22-
20+
import { isInputType, isNonNullType } from '../type/definition';
2321
import type { ObjMap } from '../jsutils/ObjMap';
2422
import type { GraphQLField } from '../type/definition';
2523
import type { GraphQLDirective } from '../type/directives';
@@ -69,7 +67,7 @@ export function getVariableValues(
6967
} else {
7068
const value = inputs[varName];
7169
if (isInvalid(value)) {
72-
if (varType instanceof GraphQLNonNull) {
70+
if (isNonNullType(varType)) {
7371
errors.push(
7472
new GraphQLError(
7573
`Variable "$${varName}" of required type ` +
@@ -134,7 +132,7 @@ export function getArgumentValues(
134132
if (!argumentNode) {
135133
if (!isInvalid(defaultValue)) {
136134
coercedValues[name] = defaultValue;
137-
} else if (argType instanceof GraphQLNonNull) {
135+
} else if (isNonNullType(argType)) {
138136
throw new GraphQLError(
139137
`Argument "${name}" of required type ` +
140138
`"${String(argType)}" was not provided.`,
@@ -154,7 +152,7 @@ export function getArgumentValues(
154152
coercedValues[name] = variableValues[variableName];
155153
} else if (!isInvalid(defaultValue)) {
156154
coercedValues[name] = defaultValue;
157-
} else if (argType instanceof GraphQLNonNull) {
155+
} else if (isNonNullType(argType)) {
158156
throw new GraphQLError(
159157
`Argument "${name}" of required type "${String(argType)}" was ` +
160158
`provided the variable "$${variableName}" which was not provided ` +

src/index.js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,23 +79,45 @@ export {
7979
__EnumValue,
8080
__TypeKind,
8181
// Predicates
82+
isSchema,
83+
isDirective,
8284
isType,
85+
isScalarType,
86+
isObjectType,
87+
isInterfaceType,
88+
isUnionType,
89+
isEnumType,
90+
isInputObjectType,
91+
isListType,
92+
isNonNullType,
8393
isInputType,
8494
isOutputType,
8595
isLeafType,
8696
isCompositeType,
8797
isAbstractType,
98+
isWrappingType,
99+
isNullableType,
88100
isNamedType,
89101
isSpecifiedScalarType,
90102
isIntrospectionType,
91103
isSpecifiedDirective,
92104
// Assertions
93105
assertType,
106+
assertScalarType,
107+
assertObjectType,
108+
assertInterfaceType,
109+
assertUnionType,
110+
assertEnumType,
111+
assertInputObjectType,
112+
assertListType,
113+
assertNonNullType,
94114
assertInputType,
95115
assertOutputType,
96116
assertLeafType,
97117
assertCompositeType,
98118
assertAbstractType,
119+
assertWrappingType,
120+
assertNullableType,
99121
assertNamedType,
100122
// Un-modifiers
101123
getNullableType,
@@ -112,6 +134,7 @@ export type {
112134
GraphQLLeafType,
113135
GraphQLCompositeType,
114136
GraphQLAbstractType,
137+
GraphQLWrappingType,
115138
GraphQLNullableType,
116139
GraphQLNamedType,
117140
Thunk,

src/jsutils/instanceOf.js

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/**
2+
* Copyright (c) 2015-present, Facebook, Inc.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @flow
8+
*/
9+
10+
/**
11+
* A replacement for instanceof which includes an error warning when multi-realm
12+
* constructors are detected.
13+
*/
14+
declare function instanceOf(
15+
value: mixed,
16+
constructor: mixed,
17+
): boolean %checks(value instanceof constructor);
18+
19+
// eslint-disable-next-line no-redeclare
20+
export default function instanceOf(value, constructor) {
21+
if (value instanceof constructor) {
22+
return true;
23+
}
24+
if (value) {
25+
const valueConstructor = value.constructor;
26+
if (valueConstructor && valueConstructor.name === constructor.name) {
27+
throw new Error(
28+
`Cannot use ${constructor.name} "${value}" from another module or realm.
29+
30+
Ensure that there is only one instance of "graphql" in the node_modules
31+
directory. If different versions of "graphql" are the dependencies of other
32+
relied on modules, use "resolutions" to ensure only one version is installed.
33+
34+
https://yarnpkg.com/en/docs/selective-version-resolutions
35+
36+
Duplicate "graphql" modules cannot be used at the same time since different
37+
versions may have different capabilities and behavior. The data from one
38+
version used in the function from another could produce confusing and
39+
spurious results.`,
40+
);
41+
}
42+
}
43+
return false;
44+
}

src/subscription/__tests__/subscribe-test.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -313,12 +313,12 @@ describe('Subscription Initialization Phase', () => {
313313

314314
await expectPromiseToThrow(
315315
() => subscribe(null, document),
316-
'Must provide schema.',
316+
'Expected null to be a GraphQL schema.',
317317
);
318318

319319
await expectPromiseToThrow(
320320
() => subscribe({ document }),
321-
'Must provide schema.',
321+
'Expected undefined to be a GraphQL schema.',
322322
);
323323
});
324324

src/type/__tests__/definition-test.js

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import {
2323
import { describe, it } from 'mocha';
2424
import { expect } from 'chai';
2525

26-
import { isInputType, isOutputType } from '../definition';
26+
import { isObjectType, isInputType, isOutputType } from '../definition';
2727

2828
const BlogImage = new GraphQLObjectType({
2929
name: 'Image',
@@ -118,19 +118,19 @@ describe('Type System: Example', () => {
118118
const articleFieldType = articleField ? articleField.type : null;
119119

120120
const titleField =
121-
articleFieldType instanceof GraphQLObjectType &&
121+
isObjectType(articleFieldType) &&
122122
articleFieldType.getFields()[('title': string)];
123123
expect(titleField && titleField.name).to.equal('title');
124124
expect(titleField && titleField.type).to.equal(GraphQLString);
125125
expect(titleField && titleField.type.name).to.equal('String');
126126

127127
const authorField =
128-
articleFieldType instanceof GraphQLObjectType &&
128+
isObjectType(articleFieldType) &&
129129
articleFieldType.getFields()[('author': string)];
130130

131131
const authorFieldType = authorField ? authorField.type : null;
132132
const recentArticleField =
133-
authorFieldType instanceof GraphQLObjectType &&
133+
isObjectType(authorFieldType) &&
134134
authorFieldType.getFields()[('recentArticle': string)];
135135

136136
expect(recentArticleField && recentArticleField.type).to.equal(BlogArticle);
@@ -374,12 +374,6 @@ describe('Type System: Example', () => {
374374
});
375375
});
376376

377-
it('prohibits nesting NonNull inside NonNull', () => {
378-
expect(() => GraphQLNonNull(GraphQLNonNull(GraphQLInt))).to.throw(
379-
'Can only create NonNull of a Nullable GraphQLType but got: Int!.',
380-
);
381-
});
382-
383377
it('prohibits putting non-Object types in unions', () => {
384378
const badUnionTypes = [
385379
GraphQLInt,
@@ -476,7 +470,7 @@ describe('Type System: Example', () => {
476470
});
477471
});
478472

479-
describe('Type System: List must accept GraphQL types', () => {
473+
describe('Type System: List must accept only types', () => {
480474
const types = [
481475
GraphQLString,
482476
ScalarType,
@@ -506,7 +500,7 @@ describe('Type System: List must accept GraphQL types', () => {
506500
});
507501
});
508502

509-
describe('Type System: NonNull must accept GraphQL types', () => {
503+
describe('Type System: NonNull must only accept non-nullable types', () => {
510504
const nullableTypes = [
511505
GraphQLString,
512506
ScalarType,
@@ -536,7 +530,7 @@ describe('Type System: NonNull must accept GraphQL types', () => {
536530
notNullableTypes.forEach(type => {
537531
it(`rejects a non-type as nullable type of non-null: ${type}`, () => {
538532
expect(() => GraphQLNonNull(type)).to.throw(
539-
`Can only create NonNull of a Nullable GraphQLType but got: ${type}.`,
533+
`Expected ${type} to be a GraphQL nullable type.`,
540534
);
541535
});
542536
});

0 commit comments

Comments
 (0)