Add spec for Long custom scalar type#26
Conversation
The `Long` data type is commonly used in many applications because the largest value that can be held in an `Int` value is `2^31-1`. See: graphql/graphql-spec#73
martinbonnin
left a comment
There was a problem hiding this comment.
Thanks for looking into this.
I left a few comments mainly FYI. Feel free to use them or reject them, this repo is made so that we don't have all to agree. Just resolve the comments you reject so we can know when to merge this.
Co-authored-by: Martin Bonnin <martin@mbonnin.net>
Co-authored-by: Martin Bonnin <martin@mbonnin.net>
This makes the wording more consistent and removes all references to JSON.
There's a separate suggestion to update the guide lines to not mention being liberal in what is accepted for inputs. See: graphql#40
|
Though it makes sense to use Using a numeric format when serializing to JSON would make programming languages which don't have native support for parsing arbitrarily long numbers in JSON have to use a custom JSON parser for all payloads sent using the most common HTTP encoding. That's not really viable, and will result in few people adopting Long. Instead, I'd recommend that you require string in both directions, at least for JSON. Note this text from the spec:
Serializing to string is perfectly valid, and is a safe fallback for basically everything. Even binary can serialize as string via base64/etc. |
|
@jakobmerrild did you want to update this to use Strings in both directions? (or limit the range to something that JS can parse?) |
Yes! Sorry, I've been busy with other work. I'll try to get to this later today |
Ultimately, not every language can easily parse arbitrarily large JSON numbers. As such we are better off using a string serialization scheme which then allows the adaptors to use whichever methods they see fit to turn the string into a different representation after coercion, e.g. Long or BigInt
The recent changes represent a significant departure from the original spec, so we update the date :)
|
Aaaand, I got distracted again. Have updated the spec to:
|
|
@jakobmerrild thanks for updating this PR. We have a few CI failures. Mind looking into those? I think we can merge after that. |
|
All good ✅ @jakobmerrild is it ok to merge this pull request? |
|
All good from my end 👍 |
It's the important end :) As detailed in the README, feedbacks may not block the merging of a new scalar specification. The authors has final say. Merging this. |
|
Thanks for the contribution! |
The
Longdata type is commonly used in many applications because the largest value that can be held in anIntvalue is2^31-1. See: graphql/graphql-spec#73I'm honestly torn about whether or not output strings should be allowed 👀 . As discussed in the linked issue common clients (JavaScript) does not read JSON numbers above
2^53-1intonumberwithout loss of precision.Also accepting
Stringinputs without acceptingStringoutputs seem inconsistent.