Skip to content
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

Jump to bottom

BigInt support in scalars #4276

Closed
wants to merge 1 commit into from
Closed

BigInt support in scalars #4276

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 15 additions & 0 deletions src/jsutils/isNumeric.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
export function isInteger(value: unknown): value is number | bigint {
const valueTypeOf = typeof value;
if (valueTypeOf === 'number') {
return Number.isInteger(value);
}
return valueTypeOf === 'bigint';
}

export function isNumeric(value: unknown): value is number | bigint {
const valueTypeOf = typeof value;
if (valueTypeOf === 'number') {
return Number.isFinite(value);
}
return valueTypeOf === 'bigint';
}
10 changes: 10 additions & 0 deletions src/type/__tests__/scalars-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ describe('Type System: Specified scalar types', () => {
expect(coerceInputValue(1)).to.equal(1);
expect(coerceInputValue(0)).to.equal(0);
expect(coerceInputValue(-1)).to.equal(-1);
expect(coerceInputValue(1n)).to.equal(1);

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

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

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

@yaacovCR yaacovCR Nov 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
expect(coerceOutputValue(1n)).to.equal(1n);
expect(coerceOutputValue(1n)).to.equal(1);

Assuming we take the change with respect to floats.


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

const valueOf = () => 'valueOf string';
const toJSON = () => 'toJSON string';
Expand Down Expand Up @@ -497,6 +502,8 @@ describe('Type System: Specified scalar types', () => {
expect(coerceOutputValue(0)).to.equal(false);
expect(coerceOutputValue(true)).to.equal(true);
expect(coerceOutputValue(false)).to.equal(false);
expect(coerceOutputValue(1n)).to.equal(true);
expect(coerceOutputValue(0n)).to.equal(false);
expect(
coerceOutputValue({
value: true,
Expand Down Expand Up @@ -536,6 +543,8 @@ describe('Type System: Specified scalar types', () => {
expect(coerceInputValue(1)).to.equal('1');
expect(coerceInputValue(0)).to.equal('0');
expect(coerceInputValue(-1)).to.equal('-1');
// Can handle bigint in JS
expect(coerceInputValue(9007199254740993n)).to.equal('9007199254740993');

// Maximum and minimum safe numbers in JS
expect(coerceInputValue(9007199254740991)).to.equal('9007199254740991');
Expand Down Expand Up @@ -620,6 +629,7 @@ describe('Type System: Specified scalar types', () => {
expect(coerceOutputValue(123)).to.equal('123');
expect(coerceOutputValue(0)).to.equal('0');
expect(coerceOutputValue(-1)).to.equal('-1');
expect(coerceOutputValue(9007199254740993n)).to.equal('9007199254740993');

const valueOf = () => 'valueOf ID';
const toJSON = () => 'toJSON ID';
Expand Down
39 changes: 22 additions & 17 deletions src/type/scalars.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { inspect } from '../jsutils/inspect.js';
import { isInteger, isNumeric } from '../jsutils/isNumeric.js';
import { isObjectLike } from '../jsutils/isObjectLike.js';

import { GraphQLError } from '../error/GraphQLError.js';
Expand Down Expand Up @@ -40,32 +41,36 @@ 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)}`,
);
}

if (num > GRAPHQL_MAX_INT || num < GRAPHQL_MIN_INT) {
throw new GraphQLError(
'Int cannot represent non 32-bit signed integer value: ' +
inspect(coercedValue),
);
}
return num;

return Number(num);
},

coerceInputValue(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}`,
`Int cannot represent non 32-bit signed integer value: ${coercedVal}`,
);
}
return inputValue;
return coercedVal;
},

coerceInputLiteral(valueNode) {
Expand Down Expand Up @@ -96,7 +101,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).',
Expand All @@ -113,7 +118,7 @@ export const GraphQLFloat = new GraphQLScalarType<number>({
num = Number(coercedValue);
}

if (typeof num !== 'number' || !Number.isFinite(num)) {
if (!isNumeric(num)) {
JoviDeCroock marked this conversation as resolved.
Show resolved Hide resolved
throw new GraphQLError(
`Float cannot represent non numeric value: ${inspect(coercedValue)}`,
);
Expand All @@ -122,12 +127,12 @@ export const GraphQLFloat = new GraphQLScalarType<number>({
},

coerceInputValue(inputValue) {
if (typeof inputValue !== 'number' || !Number.isFinite(inputValue)) {
if (!isNumeric(inputValue)) {
throw new GraphQLError(
`Float cannot represent non numeric value: ${inspect(inputValue)}`,
);
}
return inputValue;
return typeof inputValue === 'bigint' ? Number(inputValue) : inputValue;
JoviDeCroock marked this conversation as resolved.
Show resolved Hide resolved
},

coerceInputLiteral(valueNode) {
Expand Down Expand Up @@ -163,8 +168,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)}`,
Expand Down Expand Up @@ -207,8 +212,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)}`,
Expand Down Expand Up @@ -252,7 +257,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(
Expand All @@ -264,8 +269,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)}`);
},
Expand Down
35 changes: 35 additions & 0 deletions src/utilities/__tests__/astFromValue-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,16 @@ describe('astFromValue', () => {
value: false,
});

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

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

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

expect(astFromValue(null, GraphQLBoolean)).to.deep.equal({
Expand Down Expand Up @@ -65,6 +75,16 @@ describe('astFromValue', () => {
value: '123',
});

// 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(1e4, GraphQLInt)).to.deep.equal({
kind: 'IntValue',
value: '10000',
Expand Down Expand Up @@ -111,6 +131,11 @@ describe('astFromValue', () => {
kind: 'FloatValue',
value: '1e+40',
});

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

it('converts String values to String ASTs', () => {
Expand Down Expand Up @@ -143,6 +168,11 @@ describe('astFromValue', () => {
kind: 'NullValue',
});

expect(astFromValue(9007199254740993n, GraphQLString)).to.deep.equal({
kind: 'StringValue',
value: '9007199254740993',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
value: '9007199254740993',
value: '9007199254740992',

There is a loss of precision when converting a bigint to a float

});

expect(astFromValue(undefined, GraphQLString)).to.deep.equal(null);
});

Expand All @@ -163,6 +193,11 @@ describe('astFromValue', () => {
value: 'VA\nLUE',
});

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

// Note: IntValues are used when possible.
expect(astFromValue(-1, GraphQLID)).to.deep.equal({
kind: 'IntValue',
Expand Down
5 changes: 5 additions & 0 deletions src/utilities/astFromValue.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,11 @@ export function astFromValue(
: { kind: Kind.FLOAT, value: stringNum };
}

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

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