Skip to content

Better Predicates #1137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 12, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/execution/__tests__/executor-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
20 changes: 12 additions & 8 deletions src/execution/execute.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -1002,6 +1005,7 @@ function completeValue(
}

// Not reachable. All possible output types have been considered.
/* istanbul ignore next */
throw new Error(
`Cannot complete value of unexpected type "${String(
(returnType: empty),
Expand All @@ -1015,7 +1019,7 @@ function completeValue(
*/
function completeListValue(
exeContext: ExecutionContext,
returnType: GraphQLList<*>,
returnType: GraphQLList<GraphQLOutputType>,
fieldNodes: $ReadOnlyArray<FieldNode>,
info: GraphQLResolveInfo,
path: ResponsePath,
Expand Down Expand Up @@ -1138,7 +1142,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 ` +
Expand Down
10 changes: 4 additions & 6 deletions src/execution/values.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 ` +
Expand Down Expand Up @@ -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.`,
Expand All @@ -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 ` +
Expand Down
23 changes: 23 additions & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -112,6 +134,7 @@ export type {
GraphQLLeafType,
GraphQLCompositeType,
GraphQLAbstractType,
GraphQLWrappingType,
GraphQLNullableType,
GraphQLNamedType,
Thunk,
Expand Down
44 changes: 44 additions & 0 deletions src/jsutils/instanceOf.js
Original file line number Diff line number Diff line change
@@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@leebyron Not sure if anybody still using alternative package managers(e.g. bower) but maybe it worth to prefix this string with "If you are using NPM package" to reduce possible confusion.

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;
}
1 change: 1 addition & 0 deletions src/jsutils/invariant.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
*/

export default function invariant(condition: mixed, message: string) {
/* istanbul ignore else */
if (!condition) {
throw new Error(message);
}
Expand Down
4 changes: 2 additions & 2 deletions src/subscription/__tests__/subscribe-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.',
);
});

Expand Down
20 changes: 7 additions & 13 deletions src/type/__tests__/definition-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.`,
);
});
});
Expand Down
Loading