coerceValues: correctly handle NaN and similar values (#2047)

graphql · Jul 22, 2019 · 670bbac · 670bbac
1 parent bbab902
commit 670bbac
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 9 deletions.
diff --git a/src/utilities/__tests__/coerceValue-test.js b/src/utilities/__tests__/coerceValue-test.js
@@ -71,6 +71,11 @@ describe('coerceValue', () => {
       expectValue(result).to.equal(null);
     });
 
+    it('returns no error for NaN result', () => {
+      const result = coerceValue({ value: NaN }, TestScalar);
+      expectValue(result).to.satisfy(Number.isNaN);
+    });
+
     it('returns an error for undefined result', () => {
       const result = coerceValue({ value: undefined }, TestScalar);
       expectErrors(result).to.deep.equal(['Expected type TestScalar.']);
@@ -140,9 +145,9 @@ describe('coerceValue', () => {
     });
 
     it('returns an error for an invalid field', () => {
-      const result = coerceValue({ foo: 'abc' }, TestInputObject);
+      const result = coerceValue({ foo: NaN }, TestInputObject);
       expectErrors(result).to.deep.equal([
-        'Expected type Int at value.foo. Int cannot represent non-integer value: "abc"',
+        'Expected type Int at value.foo. Int cannot represent non-integer value: NaN',
       ]);
     });
 
@@ -184,7 +189,10 @@ describe('coerceValue', () => {
       new GraphQLInputObjectType({
         name: 'TestInputObject',
         fields: {
-          foo: { type: GraphQLInt, defaultValue },
+          foo: {
+            type: new GraphQLScalarType({ name: 'TestScalar' }),
+            defaultValue,
+          },
         },
       });
 
@@ -199,8 +207,15 @@ describe('coerceValue', () => {
     });
 
     it('returns null as value', () => {
-      const result = coerceValue({}, TestInputObject(7));
-      expectValue(result).to.deep.equal({ foo: 7 });
+      const result = coerceValue({}, TestInputObject(null));
+      expectValue(result).to.deep.equal({ foo: null });
+    });
+
+    it('returns NaN as value', () => {
+      const result = coerceValue({}, TestInputObject(NaN));
+      expectValue(result)
+        .to.have.property('foo')
+        .that.satisfy(Number.isNaN);
     });
   });
 

diff --git a/src/utilities/coerceValue.js b/src/utilities/coerceValue.js
@@ -3,7 +3,6 @@
 import { forEach, isCollection } from 'iterall';
 import objectValues from '../polyfills/objectValues';
 import inspect from '../jsutils/inspect';
-import isInvalid from '../jsutils/isInvalid';
 import didYouMean from '../jsutils/didYouMean';
 import isObjectLike from '../jsutils/isObjectLike';
 import suggestionList from '../jsutils/suggestionList';
@@ -63,7 +62,7 @@ export function coerceValue(
     // the original error.
     try {
       const parseResult = type.parseValue(value);
-      if (isInvalid(parseResult)) {
+      if (parseResult === undefined) {
         return ofErrors([
           coercionError(`Expected type ${type.name}`, blameNode, path),
         ]);
@@ -145,8 +144,8 @@ export function coerceValue(
     // Ensure every defined field is valid.
     for (const field of objectValues(fields)) {
       const fieldValue = value[field.name];
-      if (isInvalid(fieldValue)) {
-        if (!isInvalid(field.defaultValue)) {
+      if (fieldValue === undefined) {
+        if (field.defaultValue !== undefined) {
           coercedValue[field.name] = field.defaultValue;
         } else if (isNonNullType(field.type)) {
           errors = add(