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

Native support for data type of BigInt #90

Open
dotansimha opened this issue May 25, 2022 · 10 comments
Open

Native support for data type of BigInt #90

dotansimha opened this issue May 25, 2022 · 10 comments
Assignees
Labels
enhancement New feature or request

Comments

@dotansimha
Copy link
Collaborator

No description provided.

@dotansimha dotansimha added the enhancement New feature or request label May 25, 2022
@dabbott
Copy link

dabbott commented Jun 14, 2022

Note that true BigInt support may be a bit tricky for React Native, since React Native's Hermes JS engine doesn't support the BigInt type yet. It is in the works though facebook/hermes#510!

@azf20 azf20 added this to GraphQL API Jul 1, 2022
@matthewlilley
Copy link

matthewlilley commented Jul 13, 2022

Yeah, I'm a bit hessitent to enforce BigInt just yet. There's good but not enough support for all users, yet. We use JSBI to represent BigInt in our dApps, which I think would be a great middle ground.

https://github.com/GoogleChromeLabs/jsbi

@ardatan
Copy link
Member

ardatan commented Sep 6, 2022

Available in the latest release!

@ardatan ardatan closed this as completed Sep 6, 2022
@ardatan ardatan moved this to Done in GraphQL API Sep 6, 2022
@matthewlilley
Copy link

@ardatan Is there some way to disable this? We don't use native bigint for a few reasons, one being that we aren't able to serialize it, and do a lot of SSR. We use JSBI in our SDKs also to provide maximum browser coverage. This is a bit of a pain for us atm and preventing updating.

@matthewlilley
Copy link

Worked around for now, but would be nice if we were able to transform BigInt & BigDecimal ourselves, depending on needs.

@ardatan
Copy link
Member

ardatan commented Sep 13, 2022

@matthewlilley Graph Client uses json-bigint-patch to handle BigInt parsing and serialization. BigInts are primitive of JavaScript today and supported by the major browsers.
Could you explain more about the issue you have? Maybe we can help you.

@matthewlilley
Copy link

@ardatan I'm unable to build with type safety enabled now.

Screenshot_804

If I cast to BigInt, like so

Screenshot_805

Error: Do not know how to serialize a BigInt

@matthewlilley
Copy link

matthewlilley commented Sep 13, 2022

I think it's bad to assume and even worse enforce that everyone would like native bigint support based on the schema, since the majority of subgraphs written use BigInt for convininance within mappings, not because they want to be parsed as that on the client.

And in a case like this it makes even less sense, because it's a timestamp which will never be BigInt, it's a unsigned 32-bit integer...

Just for context, this totally breaks Next.js, it makes caching impossible because it expects serializability via JSON.stringify.... this should be opt-in/out, I expect many other frameworks and libraries will suffer from issues with this too.

I'm forced to // @ts-ignore in many places or unable to compile.

Maybe type BigIntIsh = bigint | number | string makes more sense?

It's parsing ERC20 decimals as BigInt too, this is a unsigned 8 bit integer, it makes no sense to ever be represented as a BigInt.

This update was also a breaking change, but wasn't versioned this way.

@azf20
Copy link
Contributor

azf20 commented Sep 13, 2022

Agree with @matthewlilley, often users will use BigInt simply for convenience / as a default, and users don't expect the native type

@azf20 azf20 reopened this Sep 13, 2022
@matthewlilley
Copy link

Agree with @matthewlilley, often users will use BigInt simply for convenience / as a default, and users don't expect the native type

To contiue this conversation, we plan to eventually migrate to native bigint, but given the speed at which we can iterate subgraphs this would need to happen over a long time period, so support for incremental adoption of native bigint , or other transforms is critical.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

No branches or pull requests

5 participants