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.

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

Handle Long as String to be able to process all the range of values #1304

Merged
merged 1 commit into from
Feb 25, 2022

Conversation

mdulac
Copy link
Contributor

@mdulac mdulac commented Feb 24, 2022

Handle the long values between 9007199254740991 and Long.MAX_VALUE 9223372036854775807 (and same for the negative ones)

Max Integer value in JS is 9007199254740991, and we need (in my project) to cover all the range of Scala Long.
One solution could be that Long ArgBuilder handle values as String, then JS can send us bigger values.

It's the behavior of Circe when you try to parse a long field from a String value.

This is the topic of my PR :)

@mdulac mdulac changed the title Handle Long as String Handle Long as String to be able to process all the range of values Feb 24, 2022
@ghostdogpr
Copy link
Owner

For reference, here's what the spec says for Int:

When expected as an input type, only integer input values are accepted. All other input values, including strings with numeric content, must raise a query error indicating an incorrect type. If the integer input value represents a value less than -231 or greater than or equal to 231, a query error should be raised.

Numeric integer values larger than 32‐bit should either use String or a custom‐defined Scalar type, as not all platforms and transports support encoding integer numbers larger than 32‐bit.

While it's explicitly forbidden for Int scalar, Long not being an official scalar, I think it makes sense to allow String for it.

@ghostdogpr ghostdogpr merged commit e818972 into ghostdogpr:master Feb 25, 2022
@mdulac mdulac deleted the feature/long-as-string branch February 25, 2022 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants