Skip to content

astFromValue - JavaScript BigInt support #4088

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

astFromValue - JavaScript BigInt support #4088

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9284dab
JavaScript BigInt support
ardatan May 8, 2024
1781164
Add more tests
ardatan May 9, 2024
47f71f8
reduce diff, don't modify existing tests
yaacovCR Oct 13, 2024
e78d663
add a few tests for scalars
yaacovCR Oct 13, 2024
62f92fe
add failing test
yaacovCR Oct 13, 2024
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
7 changes: 7 additions & 0 deletions src/jsutils/isInteger.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export function isInteger(value: unknown): value is number | bigint {
const valueTypeOf = typeof value;
if (valueTypeOf === 'number') {
return Number.isInteger(value);
}
return valueTypeOf === 'bigint';
}
7 changes: 7 additions & 0 deletions src/jsutils/isNumeric.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export function isNumeric(value: unknown): value is number | bigint {
const valueTypeOf = typeof value;
if (valueTypeOf === 'number') {
return Number.isFinite(value);
}
return valueTypeOf === 'bigint';
}
11 changes: 11 additions & 0 deletions src/type/__tests__/scalars-test.ts
View file
Original file line number Diff line number Diff line change
@@ -21,6 +21,7 @@ describe('Type System: Specified scalar types', () => {
expect(parseValue(1)).to.equal(1);
expect(parseValue(0)).to.equal(0);
expect(parseValue(-1)).to.equal(-1);
expect(parseValue(1n)).to.equal(1);

expect(() => parseValue(9876504321)).to.throw(
'Int cannot represent non 32-bit signed integer value: 9876504321',
@@ -119,6 +120,7 @@ describe('Type System: Specified scalar types', () => {
expect(serialize(1e5)).to.equal(100000);
expect(serialize(false)).to.equal(0);
expect(serialize(true)).to.equal(1);
expect(serialize(1n)).to.equal(1);

const customValueOfObj = {
value: 5,
@@ -190,6 +192,7 @@ describe('Type System: Specified scalar types', () => {
expect(parseValue(-1)).to.equal(-1);
expect(parseValue(0.1)).to.equal(0.1);
expect(parseValue(Math.PI)).to.equal(Math.PI);
expect(parseValue(1n)).to.equal(1);

expect(() => parseValue(NaN)).to.throw(
'Float cannot represent non numeric value: NaN',
@@ -280,6 +283,7 @@ describe('Type System: Specified scalar types', () => {
expect(serialize('-1.1')).to.equal(-1.1);
expect(serialize(false)).to.equal(0.0);
expect(serialize(true)).to.equal(1.0);
expect(serialize(1n)).to.equal(1n);

const customValueOfObj = {
value: 5.5,
@@ -380,6 +384,7 @@ describe('Type System: Specified scalar types', () => {
expect(serialize(-1.1)).to.equal('-1.1');
expect(serialize(true)).to.equal('true');
expect(serialize(false)).to.equal('false');
expect(serialize(9007199254740993n)).to.equal('9007199254740993');

const valueOf = () => 'valueOf string';
const toJSON = () => 'toJSON string';
@@ -493,6 +498,8 @@ describe('Type System: Specified scalar types', () => {

expect(serialize(1)).to.equal(true);
expect(serialize(0)).to.equal(false);
expect(serialize(1n)).to.equal(true);
expect(serialize(0n)).to.equal(false);
expect(serialize(true)).to.equal(true);
expect(serialize(false)).to.equal(false);
expect(
@@ -539,6 +546,9 @@ describe('Type System: Specified scalar types', () => {
expect(parseValue(9007199254740991)).to.equal('9007199254740991');
expect(parseValue(-9007199254740991)).to.equal('-9007199254740991');

// Can handle bigint in JS
expect(parseValue(9007199254740993n)).to.equal('9007199254740993');

expect(() => parseValue(undefined)).to.throw(
'ID cannot represent value: undefined',
);
@@ -614,6 +624,7 @@ describe('Type System: Specified scalar types', () => {
expect(serialize(123)).to.equal('123');
expect(serialize(0)).to.equal('0');
expect(serialize(-1)).to.equal('-1');
expect(serialize(9007199254740993n)).to.equal('9007199254740993');

const valueOf = () => 'valueOf ID';
const toJSON = () => 'toJSON ID';
34 changes: 19 additions & 15 deletions src/type/scalars.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { inspect } from '../jsutils/inspect.js';
import { isInteger } from '../jsutils/isInteger.js';
import { isNumeric } from '../jsutils/isNumeric.js';
import { isObjectLike } from '../jsutils/isObjectLike.js';

import { GraphQLError } from '../error/GraphQLError.js';
@@ -40,7 +42,7 @@ export const GraphQLInt = new GraphQLScalarType<number>({
num = Number(coercedValue);
}

if (typeof num !== 'number' || !Number.isInteger(num)) {
if (!isInteger(num)) {
throw new GraphQLError(
`Int cannot represent non-integer value: ${inspect(coercedValue)}`,
);
@@ -51,21 +53,22 @@ export const GraphQLInt = new GraphQLScalarType<number>({
inspect(coercedValue),
);
}
return num;
return Number(num);
},

parseValue(inputValue) {
if (typeof inputValue !== 'number' || !Number.isInteger(inputValue)) {
if (!isInteger(inputValue)) {
throw new GraphQLError(
`Int cannot represent non-integer value: ${inspect(inputValue)}`,
);
}
if (inputValue > GRAPHQL_MAX_INT || inputValue < GRAPHQL_MIN_INT) {
const coercedVal = Number(inputValue);
if (coercedVal > GRAPHQL_MAX_INT || coercedVal < GRAPHQL_MIN_INT) {
throw new GraphQLError(
`Int cannot represent non 32-bit signed integer value: ${inputValue}`,
);
}
return inputValue;
return coercedVal;
},

parseConstLiteral(valueNode) {
@@ -96,7 +99,7 @@ export const GraphQLInt = new GraphQLScalarType<number>({
},
});

export const GraphQLFloat = new GraphQLScalarType<number>({
export const GraphQLFloat = new GraphQLScalarType<number | bigint>({
name: 'Float',
description:
'The `Float` scalar type represents signed double-precision fractional values as specified by [IEEE 754](https://en.wikipedia.org/wiki/IEEE_floating_point).',
@@ -113,16 +116,17 @@ export const GraphQLFloat = new GraphQLScalarType<number>({
num = Number(coercedValue);
}

if (typeof num !== 'number' || !Number.isFinite(num)) {
if (!isNumeric(num)) {
throw new GraphQLError(
`Float cannot represent non numeric value: ${inspect(coercedValue)}`,
);
}

return num;
},

parseValue(inputValue) {
if (typeof inputValue !== 'number' || !Number.isFinite(inputValue)) {
if (!isNumeric(inputValue)) {
throw new GraphQLError(
`Float cannot represent non numeric value: ${inspect(inputValue)}`,
);
@@ -163,8 +167,8 @@ export const GraphQLString = new GraphQLScalarType<string>({
if (typeof coercedValue === 'boolean') {
return coercedValue ? 'true' : 'false';
}
if (typeof coercedValue === 'number' && Number.isFinite(coercedValue)) {
return coercedValue.toString();
if (isNumeric(coercedValue)) {
return String(coercedValue);
}
throw new GraphQLError(
`String cannot represent value: ${inspect(outputValue)}`,
@@ -207,8 +211,8 @@ export const GraphQLBoolean = new GraphQLScalarType<boolean>({
if (typeof coercedValue === 'boolean') {
return coercedValue;
}
if (Number.isFinite(coercedValue)) {
return coercedValue !== 0;
if (isNumeric(coercedValue)) {
return Number(coercedValue) !== 0;
}
throw new GraphQLError(
`Boolean cannot represent a non boolean value: ${inspect(coercedValue)}`,
@@ -252,7 +256,7 @@ export const GraphQLID = new GraphQLScalarType<string>({
if (typeof coercedValue === 'string') {
return coercedValue;
}
if (Number.isInteger(coercedValue)) {
if (isInteger(coercedValue)) {
return String(coercedValue);
}
throw new GraphQLError(
@@ -264,8 +268,8 @@ export const GraphQLID = new GraphQLScalarType<string>({
if (typeof inputValue === 'string') {
return inputValue;
}
if (typeof inputValue === 'number' && Number.isInteger(inputValue)) {
return inputValue.toString();
if (isInteger(inputValue)) {
return String(inputValue);
}
throw new GraphQLError(`ID cannot represent value: ${inspect(inputValue)}`);
},
40 changes: 40 additions & 0 deletions src/utilities/__tests__/astFromValue-test.ts
View file
Original file line number Diff line number Diff line change
@@ -46,6 +46,16 @@ describe('astFromValue', () => {
value: true,
});

expect(astFromValue(0n, GraphQLBoolean)).to.deep.equal({
kind: 'BooleanValue',
value: false,
});

expect(astFromValue(1n, GraphQLBoolean)).to.deep.equal({
kind: 'BooleanValue',
value: true,
});

const NonNullBoolean = new GraphQLNonNull(GraphQLBoolean);
expect(astFromValue(0, NonNullBoolean)).to.deep.equal({
kind: 'BooleanValue',
@@ -69,6 +79,11 @@ describe('astFromValue', () => {
value: '10000',
});

expect(astFromValue(1n, GraphQLInt)).to.deep.equal({
kind: 'IntValue',
value: '1',
});

// GraphQL spec does not allow coercing non-integer values to Int to avoid
// accidental data loss.
expect(() => astFromValue(123.5, GraphQLInt)).to.throw(
@@ -80,6 +95,16 @@ describe('astFromValue', () => {
'Int cannot represent non 32-bit signed integer value: 1e+40',
);

// Note: outside the bounds of 32bit signed int.
expect(() => astFromValue(9007199254740991, GraphQLInt)).to.throw(
'Int cannot represent non 32-bit signed integer value: 9007199254740991',
);

// Note: outside the bounds of 32bit signed int as BigInt.
expect(() => astFromValue(9007199254740991n, GraphQLInt)).to.throw(
'Int cannot represent non 32-bit signed integer value: 9007199254740991',
);

expect(() => astFromValue(NaN, GraphQLInt)).to.throw(
'Int cannot represent non-integer value: NaN',
);
@@ -96,6 +121,11 @@ describe('astFromValue', () => {
value: '123',
});

expect(astFromValue(9007199254740993n, GraphQLFloat)).to.deep.equal({
Copy link
Contributor

Choose a reason for hiding this comment

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

The change in type of this test is because astFromValue is not safe, set to be replaced by valueToLiteral() for all remaining uses in #3814

But a bigint of sufficient size, -- as far as I know -- cannot be converted to a JS float.

We also in this PR do not properly handle GraphQLFloat scalar parsing and serialization of small BigInts, I will push a failing test to demonstrate what I mean.

We also probably should update the logic within the new valueToLiteral() utility. It's possible that overall we can just delay landing this PR until after #3814

kind: 'IntValue',
value: '9007199254740993',
});

expect(astFromValue(123.5, GraphQLFloat)).to.deep.equal({
kind: 'FloatValue',
value: '123.5',
@@ -133,6 +163,11 @@ describe('astFromValue', () => {
value: '123',
});

expect(astFromValue(9007199254740993n, GraphQLString)).to.deep.equal({
kind: 'StringValue',
value: '9007199254740993',
});

expect(astFromValue(false, GraphQLString)).to.deep.equal({
kind: 'StringValue',
value: 'false',
@@ -183,6 +218,11 @@ describe('astFromValue', () => {
value: '01',
});

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

expect(() => astFromValue(false, GraphQLID)).to.throw(
'ID cannot represent value: false',
);
5 changes: 5 additions & 0 deletions src/utilities/astFromValue.ts
View file
Original file line number Diff line number Diff line change
@@ -118,6 +118,11 @@ export function astFromValue(
: { kind: Kind.FLOAT, value: stringNum };
}

if (typeof serialized === 'bigint') {
const stringNum = String(serialized);
return { kind: Kind.INT, value: stringNum };
}

if (typeof serialized === 'string') {
// Enum types use Enum literals.
if (isEnumType(type)) {
Loading