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
graphql · Dec 12, 2017 · 1dd7a13 · 1dd7a13
1 parent 5b62e07
commit 1dd7a13
Show file tree

Hide file tree

Showing 31 changed files with 652 additions and 445 deletions.
diff --git a/src/execution/__tests__/executor-test.js b/src/execution/__tests__/executor-test.js
@@ -39,7 +39,7 @@ describe('Execute: Handles basic execution tasks', () => {
       execute({
         document: parse('{ field }'),
       }),
-    ).to.throw('Must provide schema');
+    ).to.throw('Expected undefined to be a GraphQL schema.');
   });
 
   it('accepts an object with named properties as arguments', async () => {

diff --git a/src/execution/execute.js b/src/execution/execute.js
@@ -22,12 +22,15 @@ import {
   getDirectiveValues,
 } from './values';
 import {
-  GraphQLObjectType,
+  isObjectType,
   isAbstractType,
   isLeafType,
+  isListType,
+  isNonNullType,
 } from '../type/definition';
-import { GraphQLList, GraphQLNonNull } from '../type/wrappers';
+import type { GraphQLList } from '../type/wrappers';
 import type {
+  GraphQLObjectType,
   GraphQLOutputType,
   GraphQLLeafType,
   GraphQLAbstractType,
@@ -807,7 +810,7 @@ function completeValueCatchingError(
 ): mixed {
   // If the field type is non-nullable, then it is resolved without any
   // protection from errors, however it still properly locates the error.
-  if (returnType instanceof GraphQLNonNull) {
+  if (isNonNullType(returnType)) {
     return completeValueWithLocatedError(
       exeContext,
       returnType,
@@ -934,7 +937,7 @@ function completeValue(
 
   // If field type is NonNull, complete for inner type, and throw field error
   // if result is null.
-  if (returnType instanceof GraphQLNonNull) {
+  if (isNonNullType(returnType)) {
     const completed = completeValue(
       exeContext,
       returnType.ofType,
@@ -959,7 +962,7 @@ function completeValue(
   }
 
   // If field type is List, complete each item in the list with the inner type
-  if (returnType instanceof GraphQLList) {
+  if (isListType(returnType)) {
     return completeListValue(
       exeContext,
       returnType,
@@ -990,7 +993,7 @@ function completeValue(
   }
 
   // If field type is Object, execute and complete all sub-selections.
-  if (returnType instanceof GraphQLObjectType) {
+  if (isObjectType(returnType)) {
     return completeObjectValue(
       exeContext,
       returnType,
@@ -1015,7 +1018,7 @@ function completeValue(
  */
 function completeListValue(
   exeContext: ExecutionContext,
-  returnType: GraphQLList<*>,
+  returnType: GraphQLList<GraphQLOutputType>,
   fieldNodes: $ReadOnlyArray<FieldNode>,
   info: GraphQLResolveInfo,
   path: ResponsePath,
@@ -1138,7 +1141,7 @@ function ensureValidRuntimeType(
       ? exeContext.schema.getType(runtimeTypeOrName)
       : runtimeTypeOrName;
 
-  if (!(runtimeType instanceof GraphQLObjectType)) {
+  if (!isObjectType(runtimeType)) {
     throw new GraphQLError(
       `Abstract type ${returnType.name} must resolve to an Object type at ` +
         `runtime for field ${info.parentType.name}.${info.fieldName} with ` +

diff --git a/src/execution/values.js b/src/execution/values.js
@@ -17,9 +17,7 @@ import { valueFromAST } from '../utilities/valueFromAST';
 import { isValidLiteralValue } from '../utilities/isValidLiteralValue';
 import * as Kind from '../language/kinds';
 import { print } from '../language/printer';
-import { isInputType } from '../type/definition';
-import { GraphQLNonNull } from '../type/wrappers';
-
+import { isInputType, isNonNullType } from '../type/definition';
 import type { ObjMap } from '../jsutils/ObjMap';
 import type { GraphQLField } from '../type/definition';
 import type { GraphQLDirective } from '../type/directives';
@@ -69,7 +67,7 @@ export function getVariableValues(
     } else {
       const value = inputs[varName];
       if (isInvalid(value)) {
-        if (varType instanceof GraphQLNonNull) {
+        if (isNonNullType(varType)) {
           errors.push(
             new GraphQLError(
               `Variable "$${varName}" of required type ` +
@@ -134,7 +132,7 @@ export function getArgumentValues(
     if (!argumentNode) {
       if (!isInvalid(defaultValue)) {
         coercedValues[name] = defaultValue;
-      } else if (argType instanceof GraphQLNonNull) {
+      } else if (isNonNullType(argType)) {
         throw new GraphQLError(
           `Argument "${name}" of required type ` +
             `"${String(argType)}" was not provided.`,
@@ -154,7 +152,7 @@ export function getArgumentValues(
         coercedValues[name] = variableValues[variableName];
       } else if (!isInvalid(defaultValue)) {
         coercedValues[name] = defaultValue;
-      } else if (argType instanceof GraphQLNonNull) {
+      } else if (isNonNullType(argType)) {
         throw new GraphQLError(
           `Argument "${name}" of required type "${String(argType)}" was ` +
             `provided the variable "$${variableName}" which was not provided ` +

diff --git a/src/index.js b/src/index.js
@@ -79,23 +79,45 @@ export {
   __EnumValue,
   __TypeKind,
   // Predicates
+  isSchema,
+  isDirective,
   isType,
+  isScalarType,
+  isObjectType,
+  isInterfaceType,
+  isUnionType,
+  isEnumType,
+  isInputObjectType,
+  isListType,
+  isNonNullType,
   isInputType,
   isOutputType,
   isLeafType,
   isCompositeType,
   isAbstractType,
+  isWrappingType,
+  isNullableType,
   isNamedType,
   isSpecifiedScalarType,
   isIntrospectionType,
   isSpecifiedDirective,
   // Assertions
   assertType,
+  assertScalarType,
+  assertObjectType,
+  assertInterfaceType,
+  assertUnionType,
+  assertEnumType,
+  assertInputObjectType,
+  assertListType,
+  assertNonNullType,
   assertInputType,
   assertOutputType,
   assertLeafType,
   assertCompositeType,
   assertAbstractType,
+  assertWrappingType,
+  assertNullableType,
   assertNamedType,
   // Un-modifiers
   getNullableType,
@@ -112,6 +134,7 @@ export type {
   GraphQLLeafType,
   GraphQLCompositeType,
   GraphQLAbstractType,
+  GraphQLWrappingType,
   GraphQLNullableType,
   GraphQLNamedType,
   Thunk,

diff --git a/src/jsutils/instanceOf.js b/src/jsutils/instanceOf.js
@@ -0,0 +1,44 @@
+/**
+ * Copyright (c) 2015-present, Facebook, Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow
+ */
+
+/**
+ * A replacement for instanceof which includes an error warning when multi-realm
+ * constructors are detected.
+ */
+declare function instanceOf(
+  value: mixed,
+  constructor: mixed,
+): boolean %checks(value instanceof constructor);
+
+// eslint-disable-next-line no-redeclare
+export default function instanceOf(value, constructor) {
+  if (value instanceof constructor) {
+    return true;
+  }
+  if (value) {
+    const valueConstructor = value.constructor;
+    if (valueConstructor && valueConstructor.name === constructor.name) {
+      throw new Error(
+        `Cannot use ${constructor.name} "${value}" from another module or realm.
+
+Ensure that there is only one instance of "graphql" in the node_modules
+directory. If different versions of "graphql" are the dependencies of other
+relied on modules, use "resolutions" to ensure only one version is installed.
+
+https://yarnpkg.com/en/docs/selective-version-resolutions
+
+Duplicate "graphql" modules cannot be used at the same time since different
+versions may have different capabilities and behavior. The data from one
+version used in the function from another could produce confusing and
+spurious results.`,
+      );
+    }
+  }
+  return false;
+}
diff --git a/src/subscription/__tests__/subscribe-test.js b/src/subscription/__tests__/subscribe-test.js
@@ -313,12 +313,12 @@ describe('Subscription Initialization Phase', () => {
 
     await expectPromiseToThrow(
       () => subscribe(null, document),
-      'Must provide schema.',
+      'Expected null to be a GraphQL schema.',
     );
 
     await expectPromiseToThrow(
       () => subscribe({ document }),
-      'Must provide schema.',
+      'Expected undefined to be a GraphQL schema.',
     );
   });
 

diff --git a/src/type/__tests__/definition-test.js b/src/type/__tests__/definition-test.js
@@ -23,7 +23,7 @@ import {
 import { describe, it } from 'mocha';
 import { expect } from 'chai';
 
-import { isInputType, isOutputType } from '../definition';
+import { isObjectType, isInputType, isOutputType } from '../definition';
 
 const BlogImage = new GraphQLObjectType({
   name: 'Image',
@@ -118,19 +118,19 @@ describe('Type System: Example', () => {
     const articleFieldType = articleField ? articleField.type : null;
 
     const titleField =
-      articleFieldType instanceof GraphQLObjectType &&
+      isObjectType(articleFieldType) &&
       articleFieldType.getFields()[('title': string)];
     expect(titleField && titleField.name).to.equal('title');
     expect(titleField && titleField.type).to.equal(GraphQLString);
     expect(titleField && titleField.type.name).to.equal('String');
 
     const authorField =
-      articleFieldType instanceof GraphQLObjectType &&
+      isObjectType(articleFieldType) &&
       articleFieldType.getFields()[('author': string)];
 
     const authorFieldType = authorField ? authorField.type : null;
     const recentArticleField =
-      authorFieldType instanceof GraphQLObjectType &&
+      isObjectType(authorFieldType) &&
       authorFieldType.getFields()[('recentArticle': string)];
 
     expect(recentArticleField && recentArticleField.type).to.equal(BlogArticle);
@@ -374,12 +374,6 @@ describe('Type System: Example', () => {
     });
   });
 
-  it('prohibits nesting NonNull inside NonNull', () => {
-    expect(() => GraphQLNonNull(GraphQLNonNull(GraphQLInt))).to.throw(
-      'Can only create NonNull of a Nullable GraphQLType but got: Int!.',
-    );
-  });
-
   it('prohibits putting non-Object types in unions', () => {
     const badUnionTypes = [
       GraphQLInt,
@@ -476,7 +470,7 @@ describe('Type System: Example', () => {
   });
 });
 
-describe('Type System: List must accept GraphQL types', () => {
+describe('Type System: List must accept only types', () => {
   const types = [
     GraphQLString,
     ScalarType,
@@ -506,7 +500,7 @@ describe('Type System: List must accept GraphQL types', () => {
   });
 });
 
-describe('Type System: NonNull must accept GraphQL types', () => {
+describe('Type System: NonNull must only accept non-nullable types', () => {
   const nullableTypes = [
     GraphQLString,
     ScalarType,
@@ -536,7 +530,7 @@ describe('Type System: NonNull must accept GraphQL types', () => {
   notNullableTypes.forEach(type => {
     it(`rejects a non-type as nullable type of non-null: ${type}`, () => {
       expect(() => GraphQLNonNull(type)).to.throw(
-        `Can only create NonNull of a Nullable GraphQLType but got: ${type}.`,
+        `Expected ${type} to be a GraphQL nullable type.`,
       );
     });
   });