Skip to content

Commit c9850c1

Browse files
NaN values should be checked inside scalar's serialize method (#2438)
We shouldn't have special casing for NaN since it masks programmer mistakes. Also it makes it imposible to implement custom scalar that can encode NaN.
1 parent 200cb28 commit c9850c1

File tree

9 files changed

+23
-44
lines changed

9 files changed

+23
-44
lines changed

src/execution/execute.js

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@ import inspect from '../jsutils/inspect';
66
import memoize3 from '../jsutils/memoize3';
77
import invariant from '../jsutils/invariant';
88
import devAssert from '../jsutils/devAssert';
9-
import isInvalid from '../jsutils/isInvalid';
10-
import isNullish from '../jsutils/isNullish';
119
import isPromise from '../jsutils/isPromise';
1210
import { type ObjMap } from '../jsutils/ObjMap';
1311
import isObjectLike from '../jsutils/isObjectLike';
@@ -833,8 +831,8 @@ function completeValue(
833831
return completed;
834832
}
835833

836-
// If result value is null-ish (null, undefined, or NaN) then return null.
837-
if (isNullish(result)) {
834+
// If result value is null or undefined then return null.
835+
if (result == null) {
838836
return null;
839837
}
840838

@@ -940,7 +938,7 @@ function completeListValue(
940938
*/
941939
function completeLeafValue(returnType: GraphQLLeafType, result: mixed): mixed {
942940
const serializedResult = returnType.serialize(result);
943-
if (isInvalid(serializedResult)) {
941+
if (serializedResult === undefined) {
944942
throw new Error(
945943
`Expected a value of type "${inspect(returnType)}" but ` +
946944
`received: ${inspect(result)}`,

src/jsutils/isInvalid.js

Lines changed: 0 additions & 8 deletions
This file was deleted.

src/jsutils/isNullish.js

Lines changed: 0 additions & 8 deletions
This file was deleted.

src/utilities/__tests__/astFromValue-test.js

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,6 @@ describe('astFromValue', () => {
3434

3535
expect(astFromValue(undefined, GraphQLBoolean)).to.deep.equal(null);
3636

37-
expect(astFromValue(NaN, GraphQLInt)).to.deep.equal(null);
38-
3937
expect(astFromValue(null, GraphQLBoolean)).to.deep.equal({
4038
kind: 'NullValue',
4139
});
@@ -83,6 +81,10 @@ describe('astFromValue', () => {
8381
expect(() => astFromValue(1e40, GraphQLInt)).to.throw(
8482
'Int cannot represent non 32-bit signed integer value: 1e+40',
8583
);
84+
85+
expect(() => astFromValue(NaN, GraphQLInt)).to.throw(
86+
'Int cannot represent non-integer value: NaN',
87+
);
8688
});
8789

8890
it('converts Float values to Int/Float ASTs', () => {
@@ -205,7 +207,9 @@ describe('astFromValue', () => {
205207
value: 'value',
206208
});
207209

208-
expect(astFromValue(NaN, passthroughScalar)).to.equal(null);
210+
expect(() => astFromValue(NaN, passthroughScalar)).to.throw(
211+
'Cannot convert value to AST: NaN.',
212+
);
209213
expect(() => astFromValue(Infinity, passthroughScalar)).to.throw(
210214
'Cannot convert value to AST: Infinity.',
211215
);

src/utilities/__tests__/valueFromASTUntyped-test.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ describe('valueFromASTUntyped', () => {
5353
{ a: ['foo'] },
5454
);
5555
testCaseWithVars('$testVariable', { testVariable: null }, null);
56+
testCaseWithVars('$testVariable', { testVariable: NaN }, NaN);
5657
testCaseWithVars('$testVariable', {}, undefined);
58+
testCaseWithVars('$testVariable', null, undefined);
5759
});
5860
});

src/utilities/astFromValue.js

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@ import objectValues from '../polyfills/objectValues';
66

77
import inspect from '../jsutils/inspect';
88
import invariant from '../jsutils/invariant';
9-
import isNullish from '../jsutils/isNullish';
10-
import isInvalid from '../jsutils/isInvalid';
119
import isObjectLike from '../jsutils/isObjectLike';
1210
import isCollection from '../jsutils/isCollection';
1311

@@ -59,8 +57,8 @@ export function astFromValue(value: mixed, type: GraphQLInputType): ?ValueNode {
5957
return { kind: Kind.NULL };
6058
}
6159

62-
// undefined, NaN
63-
if (isInvalid(value)) {
60+
// undefined
61+
if (value === undefined) {
6462
return null;
6563
}
6664

@@ -107,7 +105,7 @@ export function astFromValue(value: mixed, type: GraphQLInputType): ?ValueNode {
107105
// Since value is an internally represented value, it must be serialized
108106
// to an externally represented value before converting into an AST.
109107
const serialized = type.serialize(value);
110-
if (isNullish(serialized)) {
108+
if (serialized == null) {
111109
return null;
112110
}
113111

src/utilities/valueFromAST.js

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import objectValues from '../polyfills/objectValues';
55
import keyMap from '../jsutils/keyMap';
66
import inspect from '../jsutils/inspect';
77
import invariant from '../jsutils/invariant';
8-
import isInvalid from '../jsutils/isInvalid';
98
import { type ObjMap } from '../jsutils/ObjMap';
109

1110
import { Kind } from '../language/kinds';
@@ -52,7 +51,7 @@ export function valueFromAST(
5251

5352
if (valueNode.kind === Kind.VARIABLE) {
5453
const variableName = valueNode.name.value;
55-
if (!variables || isInvalid(variables[variableName])) {
54+
if (variables == null || variables[variableName] === undefined) {
5655
// No valid return value.
5756
return;
5857
}
@@ -92,7 +91,7 @@ export function valueFromAST(
9291
coercedValues.push(null);
9392
} else {
9493
const itemValue = valueFromAST(itemNode, itemType, variables);
95-
if (isInvalid(itemValue)) {
94+
if (itemValue === undefined) {
9695
return; // Invalid: intentionally return no value.
9796
}
9897
coercedValues.push(itemValue);
@@ -101,7 +100,7 @@ export function valueFromAST(
101100
return coercedValues;
102101
}
103102
const coercedValue = valueFromAST(valueNode, itemType, variables);
104-
if (isInvalid(coercedValue)) {
103+
if (coercedValue === undefined) {
105104
return; // Invalid: intentionally return no value.
106105
}
107106
return [coercedValue];
@@ -124,7 +123,7 @@ export function valueFromAST(
124123
continue;
125124
}
126125
const fieldValue = valueFromAST(fieldNode.value, field.type, variables);
127-
if (isInvalid(fieldValue)) {
126+
if (fieldValue === undefined) {
128127
return; // Invalid: intentionally return no value.
129128
}
130129
coercedObj[field.name] = fieldValue;
@@ -157,6 +156,6 @@ export function valueFromAST(
157156
function isMissingVariable(valueNode, variables) {
158157
return (
159158
valueNode.kind === Kind.VARIABLE &&
160-
(!variables || isInvalid(variables[valueNode.name.value]))
159+
(variables == null || variables[valueNode.name.value] === undefined)
161160
);
162161
}

src/utilities/valueFromASTUntyped.js

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
import inspect from '../jsutils/inspect';
44
import invariant from '../jsutils/invariant';
55
import keyValMap from '../jsutils/keyValMap';
6-
import isInvalid from '../jsutils/isInvalid';
76
import { type ObjMap } from '../jsutils/ObjMap';
87

98
import { Kind } from '../language/kinds';
@@ -48,12 +47,8 @@ export function valueFromASTUntyped(
4847
field => field.name.value,
4948
field => valueFromASTUntyped(field.value, variables),
5049
);
51-
case Kind.VARIABLE: {
52-
const variableName = valueNode.name.value;
53-
return variables && !isInvalid(variables[variableName])
54-
? variables[variableName]
55-
: undefined;
56-
}
50+
case Kind.VARIABLE:
51+
return variables?.[valueNode.name.value];
5752
}
5853

5954
// Not reachable. All possible value nodes have been considered.

src/validation/rules/ValuesOfCorrectTypeRule.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import objectValues from '../../polyfills/objectValues';
44

55
import keyMap from '../../jsutils/keyMap';
66
import inspect from '../../jsutils/inspect';
7-
import isInvalid from '../../jsutils/isInvalid';
87
import didYouMean from '../../jsutils/didYouMean';
98
import suggestionList from '../../jsutils/suggestionList';
109

@@ -130,7 +129,7 @@ function isValidValueNode(context: ValidationContext, node: ValueNode): void {
130129
// may throw or return an invalid value to indicate failure.
131130
try {
132131
const parseResult = type.parseLiteral(node, undefined /* variables */);
133-
if (isInvalid(parseResult)) {
132+
if (parseResult === undefined) {
134133
const typeStr = inspect(locationType);
135134
context.reportError(
136135
new GraphQLError(

0 commit comments

Comments
 (0)