Skip to content

Commit

Permalink
Fix 'astFromValue' to correctly handle integers and strings (#1181)
Browse files Browse the repository at this point in the history
  • Loading branch information
@IvanGoncharov @leebyron
IvanGoncharov authored and leebyron committed Jan 8, 2018
1 parent b03cd11 commit 9e52e12
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 74 deletions.
86 changes: 40 additions & 46 deletions src/type/__tests__/introspection-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -819,7 +819,7 @@ describe('Introspection', () => {
const TestInputObject = new GraphQLInputObjectType({
name: 'TestInputObject',
fields: {
a: { type: GraphQLString, defaultValue: 'foo' },
a: { type: GraphQLString, defaultValue: 'tes\t de\fault' },
b: { type: GraphQLList(GraphQLString) },
c: { type: GraphQLString, defaultValue: null },
},
Expand All @@ -839,15 +839,13 @@ describe('Introspection', () => {
const schema = new GraphQLSchema({ query: TestType });
const request = `
{
__schema {
types {
kind
__type(name: "TestInputObject") {
kind
name
inputFields {
name
inputFields {
name
type { ...TypeRef }
defaultValue
}
type { ...TypeRef }
defaultValue
}
}
}
Expand All @@ -870,46 +868,42 @@ describe('Introspection', () => {
}
`;

return expect(graphqlSync(schema, request)).to.containSubset({
return expect(graphqlSync(schema, request)).to.deep.equal({
data: {
__schema: {
types: [
__type: {
kind: 'INPUT_OBJECT',
name: 'TestInputObject',
inputFields: [
{
kind: 'INPUT_OBJECT',
name: 'TestInputObject',
inputFields: [
{
name: 'a',
type: {
kind: 'SCALAR',
name: 'String',
ofType: null,
},
defaultValue: '"foo"',
},
{
name: 'b',
type: {
kind: 'LIST',
name: null,
ofType: {
kind: 'SCALAR',
name: 'String',
ofType: null,
},
},
defaultValue: null,
},
{
name: 'c',
type: {
kind: 'SCALAR',
name: 'String',
ofType: null,
},
defaultValue: 'null',
name: 'a',
type: {
kind: 'SCALAR',
name: 'String',
ofType: null,
},
defaultValue: '"tes\\t de\\fault"',
},
{
name: 'b',
type: {
kind: 'LIST',
name: null,
ofType: {
kind: 'SCALAR',
name: 'String',
ofType: null,
},
],
},
defaultValue: null,
},
{
name: 'c',
type: {
kind: 'SCALAR',
name: 'String',
ofType: null,
},
defaultValue: 'null',
},
],
},
Expand Down
29 changes: 27 additions & 2 deletions src/utilities/__tests__/astFromValue-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@ describe('astFromValue', () => {
});

it('converts Int values to Int ASTs', () => {
expect(astFromValue(-1, GraphQLInt)).to.deep.equal({
kind: 'IntValue',
value: '-1',
});

expect(astFromValue(123.0, GraphQLInt)).to.deep.equal({
kind: 'IntValue',
value: '123',
Expand All @@ -81,6 +86,11 @@ describe('astFromValue', () => {
});

it('converts Float values to Int/Float ASTs', () => {
expect(astFromValue(-1, GraphQLFloat)).to.deep.equal({
kind: 'IntValue',
value: '-1',
});

expect(astFromValue(123.0, GraphQLFloat)).to.deep.equal({
kind: 'IntValue',
value: '123',
Expand Down Expand Up @@ -115,7 +125,7 @@ describe('astFromValue', () => {

expect(astFromValue('VA\nLUE', GraphQLString)).to.deep.equal({
kind: 'StringValue',
value: 'VA\\nLUE',
value: 'VA\nLUE',
});

expect(astFromValue(123, GraphQLString)).to.deep.equal({
Expand Down Expand Up @@ -149,15 +159,30 @@ describe('astFromValue', () => {
// Note: EnumValues cannot contain non-identifier characters
expect(astFromValue('VA\nLUE', GraphQLID)).to.deep.equal({
kind: 'StringValue',
value: 'VA\\nLUE',
value: 'VA\nLUE',
});

// Note: IntValues are used when possible.
expect(astFromValue(-1, GraphQLID)).to.deep.equal({
kind: 'IntValue',
value: '-1',
});

expect(astFromValue(123, GraphQLID)).to.deep.equal({
kind: 'IntValue',
value: '123',
});

expect(astFromValue('123', GraphQLID)).to.deep.equal({
kind: 'IntValue',
value: '123',
});

expect(astFromValue('01', GraphQLID)).to.deep.equal({
kind: 'StringValue',
value: '01',
});

expect(astFromValue(false, GraphQLID)).to.deep.equal({
kind: 'StringValue',
value: 'false',
Expand Down
16 changes: 16 additions & 0 deletions src/utilities/__tests__/schemaPrinter-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,22 @@ describe('Type System Printer', () => {
`);
});

it('Prints String Field With String Arg With Default', () => {
const output = printSingleFieldSchema({
type: GraphQLString,
args: { argOne: { type: GraphQLString, defaultValue: 'tes\t de\fault' } },
});
expect(output).to.equal(dedent`
schema {
query: Root
}
type Root {
singleField(argOne: String = "tes\t de\fault"): String
}
`);
});

it('Prints String Field With Int Arg With Default Null', () => {
const output = printSingleFieldSchema({
type: GraphQLString,
Expand Down
47 changes: 21 additions & 26 deletions src/utilities/astFromValue.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,7 @@ import { forEach, isCollection } from 'iterall';

import isNullish from '../jsutils/isNullish';
import isInvalid from '../jsutils/isInvalid';
import type {
ValueNode,
IntValueNode,
FloatValueNode,
StringValueNode,
BooleanValueNode,
NullValueNode,
EnumValueNode,
ListValueNode,
ObjectValueNode,
} from '../language/ast';
import type { ValueNode } from '../language/ast';
import * as Kind from '../language/kinds';
import type { GraphQLInputType } from '../type/definition';
import {
Expand Down Expand Up @@ -64,7 +54,7 @@ export function astFromValue(value: mixed, type: GraphQLInputType): ?ValueNode {

// only explicit null, not undefined, NaN
if (_value === null) {
return ({ kind: Kind.NULL }: NullValueNode);
return { kind: Kind.NULL };
}

// undefined, NaN
Expand All @@ -84,7 +74,7 @@ export function astFromValue(value: mixed, type: GraphQLInputType): ?ValueNode {
valuesNodes.push(itemNode);
}
});
return ({ kind: Kind.LIST, values: valuesNodes }: ListValueNode);
return { kind: Kind.LIST, values: valuesNodes };
}
return astFromValue(_value, itemType);
}
Expand All @@ -108,7 +98,7 @@ export function astFromValue(value: mixed, type: GraphQLInputType): ?ValueNode {
});
}
});
return ({ kind: Kind.OBJECT, fields: fieldNodes }: ObjectValueNode);
return { kind: Kind.OBJECT, fields: fieldNodes };
}

if (isScalarType(type) || isEnumType(type)) {
Expand All @@ -121,34 +111,32 @@ export function astFromValue(value: mixed, type: GraphQLInputType): ?ValueNode {

// Others serialize based on their corresponding JavaScript scalar types.
if (typeof serialized === 'boolean') {
return ({ kind: Kind.BOOLEAN, value: serialized }: BooleanValueNode);
return { kind: Kind.BOOLEAN, value: serialized };
}

// JavaScript numbers can be Int or Float values.
if (typeof serialized === 'number') {
const stringNum = String(serialized);
return /^[0-9]+$/.test(stringNum)
? ({ kind: Kind.INT, value: stringNum }: IntValueNode)
: ({ kind: Kind.FLOAT, value: stringNum }: FloatValueNode);
return integerStringRegExp.test(stringNum)
? { kind: Kind.INT, value: stringNum }
: { kind: Kind.FLOAT, value: stringNum };
}

if (typeof serialized === 'string') {
// Enum types use Enum literals.
if (isEnumType(type)) {
return ({ kind: Kind.ENUM, value: serialized }: EnumValueNode);
return { kind: Kind.ENUM, value: serialized };
}

// ID types can use Int literals.
if (type === GraphQLID && /^[0-9]+$/.test(serialized)) {
return ({ kind: Kind.INT, value: serialized }: IntValueNode);
if (type === GraphQLID && integerStringRegExp.test(serialized)) {
return { kind: Kind.INT, value: serialized };
}

// Use JSON stringify, which uses the same string encoding as GraphQL,
// then remove the quotes.
return ({
return {
kind: Kind.STRING,
value: JSON.stringify(serialized).slice(1, -1),
}: StringValueNode);
value: serialized,
};
}

throw new TypeError('Cannot convert value to AST: ' + String(serialized));
Expand All @@ -157,3 +145,10 @@ export function astFromValue(value: mixed, type: GraphQLInputType): ?ValueNode {
/* istanbul ignore next */
throw new Error(`Unknown type: ${(type: empty)}.`);
}

/**
* IntValue:
* - NegativeSign? 0
* - NegativeSign? NonZeroDigit ( Digit+ )?
*/
const integerStringRegExp = /^-?(0|[1-9][0-9]*)$/;

0 comments on commit 9e52e12

Please sign in to comment.