Skip to content

Handle NaN as input value #1369

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.

Sign up for GitHub

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
Jun 5, 2018
Merged

Handle NaN as input value #1369

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 0 additions & 4 deletions src/type/definition.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import applyToStringTag from '../jsutils/applyToStringTag';
import instanceOf from '../jsutils/instanceOf';
import inspect from '../jsutils/inspect';
import invariant from '../jsutils/invariant';
import isInvalid from '../jsutils/isInvalid';
import keyMap from '../jsutils/keyMap';
import type { ObjMap } from '../jsutils/ObjMap';
import { Kind } from '../language/kinds';
Expand Down Expand Up @@ -566,9 +565,6 @@ export class GraphQLScalarType {
// Parses an externally provided value to use as an input.
parseValue(value: mixed): mixed {
const parser = this._scalarConfig.parseValue;
if (isInvalid(value)) {
return undefined;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Safe because it only used here:

graphql-js/src/utilities/coerceValue.js

Lines 61 to 71 in 85971a2

if (isNullish(value)) {
// Explicitly return the value null.
return ofValue(null);
}
if (isScalarType(type)) {
// Scalars determine if a value is valid via parseValue(), which can
// throw to indicate failure. If it throws, maintain a reference to
// the original error.
try {
const parseResult = type.parseValue(value);

So null and undefined values are never passed to parseValue.
Even though it's not breaking anything in graphql-js it's still a minor breaking change since there is a chance that someone using parseValue directly.

return parser ? parser(value) : value;
}

Expand Down
14 changes: 14 additions & 0 deletions src/utilities/__tests__/coerceValue-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,13 @@ describe('coerceValue', () => {
]);
});

it('returns a single error for NaN input as int', () => {
const result = coerceValue(NaN, GraphQLInt);
expectErrors(result).to.deep.equal([
'Expected type Int; Int cannot represent non-integer value: NaN',
]);
});

it('returns a single error for Infinity input as int', () => {
const result = coerceValue(Infinity, GraphQLInt);
expectErrors(result).to.deep.equal([
Expand Down Expand Up @@ -133,6 +140,13 @@ describe('coerceValue', () => {
]);
});

it('returns a single error for NaN input', () => {
const result = coerceValue(NaN, GraphQLFloat);
expectErrors(result).to.deep.equal([
'Expected type Float; Float cannot represent non numeric value: NaN',
]);
});

it('returns a single error for Infinity input', () => {
const result = coerceValue(Infinity, GraphQLFloat);
expectErrors(result).to.deep.equal([
Expand Down
5 changes: 2 additions & 3 deletions src/utilities/coerceValue.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import { forEach, isCollection } from 'iterall';
import inspect from '../jsutils/inspect';
import isInvalid from '../jsutils/isInvalid';
import isNullish from '../jsutils/isNullish';
import orList from '../jsutils/orList';
import suggestionList from '../jsutils/suggestionList';
import { GraphQLError } from '../error';
Expand Down Expand Up @@ -46,7 +45,7 @@ export function coerceValue(
): CoercedValue {
// A value must be provided if the type is non-null.
if (isNonNullType(type)) {
if (isNullish(value)) {
if (value == null) {
Copy link
Member Author

Choose a reason for hiding this comment

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

== null block null and undefined but not a NaN and simular values

graphql-js/src/jsutils/isNullish.js

Lines 13 to 15 in 85971a2

export default function isNullish(value: mixed): boolean %checks {
return value === null || value === undefined || value !== value;
}

return ofErrors([
coercionError(
`Expected non-nullable type ${inspect(type)} not to be null`,
Expand All @@ -58,7 +57,7 @@ export function coerceValue(
return coerceValue(value, type.ofType, blameNode, path);
}

if (isNullish(value)) {
if (value == null) {
// Explicitly return the value null.
return ofValue(null);
}
Expand Down