Skip to content

Commit 0062abf

Browse files
leebyronleebyron
committed
Amends this PR with a few additions:
* Generalizes building a value from an AST, since "scalar" could be misleading, and supporting variable values within custom scalar literals can be valuable. * Replaces isNullish with isInvalid since `null` is a meaningful value as a result of literal parsing. * Exports this new utility from the top level
1 parent 0ca6cf0 commit 0062abf

11 files changed

Lines changed: 127 additions & 103 deletions

‎.eslintrc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@
181181
"no-unneeded-ternary": 2,
182182
"no-unreachable": 2,
183183
"no-unused-expressions": 2,
184-
"no-unused-vars": [2, {"vars": "all", "args": "after-used"}],
184+
"no-unused-vars": [2, {"vars": "all", "args": "after-used", "argsIgnorePattern": "^_"}],
185185
"no-use-before-define": 0,
186186
"no-useless-call": 2,
187187
"no-useless-escape": 2,

‎src/index.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,9 +330,12 @@ export {
330330
// Create a GraphQLType from a GraphQL language AST.
331331
typeFromAST,
332332

333-
// Create a JavaScript value from a GraphQL language AST.
333+
// Create a JavaScript value from a GraphQL language AST with a Type.
334334
valueFromAST,
335335

336+
// Create a JavaScript value from a GraphQL language AST without a Type.
337+
valueFromASTUntyped,
338+
336339
// Create a GraphQL language AST from a JavaScript value.
337340
astFromValue,
338341

‎src/type/definition.js‎

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@
88
*/
99

1010
import invariant from '../jsutils/invariant';
11-
import isNullish from '../jsutils/isNullish';
11+
import isInvalid from '../jsutils/isInvalid';
1212
import type {ObjMap} from '../jsutils/ObjMap';
1313
import * as Kind from '../language/kinds';
1414
import { assertValidName } from '../utilities/assertValidName';
15-
import { scalarValueFromAST } from '../utilities/scalarValueFromAST';
15+
import { valueFromASTUntyped } from '../utilities/valueFromASTUntyped';
1616
import type {
1717
ScalarTypeDefinitionNode,
1818
ObjectTypeDefinitionNode,
@@ -340,30 +340,30 @@ export class GraphQLScalarType {
340340
}
341341

342342
// Determines if an internal value is valid for this type.
343-
// Equivalent to checking for if the parsedValue is nullish.
344343
isValidValue(value: mixed): boolean {
345-
return !isNullish(this.parseValue(value));
344+
return !isInvalid(this.parseValue(value));
346345
}
347346

348347
// Parses an externally provided value to use as an input.
349348
parseValue(value: mixed): mixed {
350349
const parser = this._scalarConfig.parseValue;
351-
if (isNullish(value)) {
350+
if (isInvalid(value)) {
352351
return undefined;
353352
}
354353
return parser ? parser(value) : value;
355354
}
356355

357356
// Determines if an internal value is valid for this type.
358-
// Equivalent to checking for if the parsedLiteral is nullish.
359-
isValidLiteral(valueNode: ValueNode): boolean {
360-
return !isNullish(this.parseLiteral(valueNode));
357+
isValidLiteral(valueNode: ValueNode, variables: ?ObjMap<mixed>): boolean {
358+
return !isInvalid(this.parseLiteral(valueNode, variables));
361359
}
362360

363361
// Parses an externally provided literal value to use as an input.
364-
parseLiteral(valueNode: ValueNode): mixed {
362+
parseLiteral(valueNode: ValueNode, variables: ?ObjMap<mixed>): mixed {
365363
const parser = this._scalarConfig.parseLiteral;
366-
return (parser || scalarValueFromAST)(valueNode);
364+
return parser ?
365+
parser(valueNode, variables) :
366+
valueFromASTUntyped(valueNode, variables);
367367
}
368368

369369
toString(): string {
@@ -385,7 +385,10 @@ export type GraphQLScalarTypeConfig<TInternal, TExternal> = {
385385
astNode?: ?ScalarTypeDefinitionNode;
386386
serialize: (value: mixed) => ?TExternal;
387387
parseValue?: (value: mixed) => ?TInternal;
388-
parseLiteral?: (valueNode: ValueNode) => ?TInternal;
388+
parseLiteral?: (
389+
valueNode: ValueNode,
390+
variables: ?ObjMap<mixed>,
391+
) => ?TInternal;
389392
};
390393

391394

@@ -956,12 +959,14 @@ export class GraphQLEnumType/* <T> */ {
956959
}
957960
}
958961

959-
isValidLiteral(valueNode: ValueNode): boolean {
962+
isValidLiteral(valueNode: ValueNode, _variables: ?ObjMap<mixed>): boolean {
963+
// Note: variables will be resolved to a value before calling this function.
960964
return valueNode.kind === Kind.ENUM &&
961965
this._getNameLookup()[valueNode.value] !== undefined;
962966
}
963967

964-
parseLiteral(valueNode: ValueNode): ?any/* T */ {
968+
parseLiteral(valueNode: ValueNode, _variables: ?ObjMap<mixed>): ?any/* T */ {
969+
// Note: variables will be resolved to a value before calling this function.
965970
if (valueNode.kind === Kind.ENUM) {
966971
const enumValue = this._getNameLookup()[valueNode.value];
967972
if (enumValue) {

‎src/type/scalars.js‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ export const GraphQLInt = new GraphQLScalarType({
5353
return num;
5454
}
5555
}
56-
return null;
56+
return undefined;
5757
}
5858
});
5959

@@ -83,7 +83,7 @@ export const GraphQLFloat = new GraphQLScalarType({
8383
parseLiteral(ast) {
8484
return ast.kind === Kind.FLOAT || ast.kind === Kind.INT ?
8585
parseFloat(ast.value) :
86-
null;
86+
undefined;
8787
}
8888
});
8989

@@ -105,7 +105,7 @@ export const GraphQLString = new GraphQLScalarType({
105105
serialize: coerceString,
106106
parseValue: coerceString,
107107
parseLiteral(ast) {
108-
return ast.kind === Kind.STRING ? ast.value : null;
108+
return ast.kind === Kind.STRING ? ast.value : undefined;
109109
}
110110
});
111111

@@ -115,7 +115,7 @@ export const GraphQLBoolean = new GraphQLScalarType({
115115
serialize: Boolean,
116116
parseValue: Boolean,
117117
parseLiteral(ast) {
118-
return ast.kind === Kind.BOOLEAN ? ast.value : null;
118+
return ast.kind === Kind.BOOLEAN ? ast.value : undefined;
119119
}
120120
});
121121

@@ -132,6 +132,6 @@ export const GraphQLID = new GraphQLScalarType({
132132
parseLiteral(ast) {
133133
return ast.kind === Kind.STRING || ast.kind === Kind.INT ?
134134
ast.value :
135-
null;
135+
undefined;
136136
}
137137
});

src/utilities/__tests__/scalarValueFromAST-test.js renamed to src/utilities/__tests__/valueFromASTUntyped-test.js

Lines changed: 23 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -9,21 +9,21 @@
99

1010
import { describe, it } from 'mocha';
1111
import { expect } from 'chai';
12-
import { scalarValueFromAST } from '../scalarValueFromAST';
12+
import { valueFromASTUntyped } from '../valueFromASTUntyped';
1313
import { parseValue } from '../../language';
1414

15-
describe('scalarValueFromAST', () => {
15+
describe('valueFromASTUntyped', () => {
1616

1717
function testCase(valueText, expected) {
1818
expect(
19-
scalarValueFromAST(parseValue(valueText))
19+
valueFromASTUntyped(parseValue(valueText))
2020
).to.deep.equal(expected);
2121
}
2222

23-
function testNegativeCase(valueText, errorMsg) {
23+
function testCaseWithVars(valueText, variables, expected) {
2424
expect(
25-
() => scalarValueFromAST(parseValue(valueText))
26-
).to.throw(errorMsg);
25+
valueFromASTUntyped(parseValue(valueText), variables)
26+
).to.deep.equal(expected);
2727
}
2828

2929
it('parses simple values', () => {
@@ -44,34 +44,30 @@ describe('scalarValueFromAST', () => {
4444

4545
it('parses input objects', () => {
4646
testCase(
47-
'{ int: 123, requiredBool: false }',
48-
{ int: 123, requiredBool: false }
47+
'{ int: 123, bool: false }',
48+
{ int: 123, bool: false }
4949
);
5050
testCase(
51-
'{ foo: [{ bar: "baz"}]}',
51+
'{ foo: [ { bar: "baz"} ] }',
5252
{ foo: [ { bar: 'baz'} ] }
5353
);
5454
});
5555

56-
it('rejects enum values and query variables', () => {
57-
testNegativeCase('TEST_ENUM_VALUE', 'Scalar value can not contain Enum.');
58-
testNegativeCase(
59-
'$test_variable',
60-
'Scalar value can not contain Query variable.'
61-
);
62-
testNegativeCase('[TEST_ENUM_VALUE]', 'Scalar value can not contain Enum.');
63-
testNegativeCase(
64-
'[$test_variable]',
65-
'Scalar value can not contain Query variable.'
66-
);
67-
testNegativeCase(
68-
'{foo: TEST_ENUM_VALUE}',
69-
'Scalar value can not contain Enum.'
70-
);
71-
testNegativeCase(
72-
'{bar: $test_variable}',
73-
'Scalar value can not contain Query variable.'
56+
it('parses enum values as plain strings', () => {
57+
testCase('TEST_ENUM_VALUE', 'TEST_ENUM_VALUE');
58+
testCase('[TEST_ENUM_VALUE]', [ 'TEST_ENUM_VALUE' ]);
59+
});
60+
61+
it('parses variables', () => {
62+
testCaseWithVars('$testVariable', { testVariable: 'foo' }, 'foo');
63+
testCaseWithVars('[$testVariable]', { testVariable: 'foo' }, [ 'foo' ]);
64+
testCaseWithVars(
65+
'{a:[$testVariable]}',
66+
{ testVariable: 'foo' },
67+
{ a: [ 'foo' ] }
7468
);
69+
testCaseWithVars('$testVariable', { testVariable: null }, null);
70+
testCaseWithVars('$testVariable', {}, undefined);
7571
});
7672

7773
});

‎src/utilities/buildClientSchema.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ export function buildClientSchema(
236236
return new GraphQLScalarType({
237237
name: scalarIntrospection.name,
238238
description: scalarIntrospection.description,
239-
serialize: id => id,
239+
serialize: value => value,
240240
});
241241
}
242242

‎src/utilities/index.js‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,16 +55,15 @@ export {
5555
// Create a GraphQLType from a GraphQL language AST.
5656
export { typeFromAST } from './typeFromAST';
5757

58-
// Create a JavaScript value from a GraphQL language AST.
58+
// Create a JavaScript value from a GraphQL language AST with a type.
5959
export { valueFromAST } from './valueFromAST';
6060

61+
// Create a JavaScript value from a GraphQL language AST without a type.
62+
export { valueFromASTUntyped } from './valueFromASTUntyped';
63+
6164
// Create a GraphQL language AST from a JavaScript value.
6265
export { astFromValue } from './astFromValue';
6366

64-
// Create a JavaScript value from a GraphQL language AST representation
65-
// of Scalar.
66-
export { scalarValueFromAST } from './scalarValueFromAST';
67-
6867
// A helper to use within recursive-descent visitors which need to be aware of
6968
// the GraphQL type system.
7069
export { TypeInfo } from './TypeInfo';

‎src/utilities/isValidLiteralValue.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ export function isValidLiteralValue(
109109
);
110110

111111
// Scalars determine if a literal values is valid.
112-
if (!type.isValidLiteral(valueNode)) {
112+
if (!type.isValidLiteral(valueNode, null)) {
113113
return [ `Expected type "${type.name}", found ${print(valueNode)}.` ];
114114
}
115115

‎src/utilities/scalarValueFromAST.js‎

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

‎src/utilities/valueFromAST.js‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -151,14 +151,14 @@ export function valueFromAST(
151151
'Must be input type'
152152
);
153153

154-
const parsed = type.parseLiteral(valueNode);
155-
if (isNullish(parsed) && !type.isValidLiteral(valueNode)) {
156-
// Invalid values represent a failure to parse correctly, in which case
157-
// no value is returned.
158-
return;
154+
// Scalar and Enum values implement methods which perform this translation.
155+
if (type.isValidLiteral(valueNode, variables)) {
156+
return type.parseLiteral(valueNode, variables);
159157
}
160158

161-
return parsed;
159+
// Invalid values represent a failure to parse correctly, in which case
160+
// no value is returned.
161+
return;
162162
}
163163

164164
// Returns true if the provided valueNode is a variable which is not defined

0 commit comments

Comments
 (0)