Fix BigInt's external representation to string

[cometkim]

Description

This kinda breaking change, fixed the BigInt's external representation to vanilla string

Related ardatan/graphql-mesh#1355

Since BigInt objects can't be serialized to JSON, we're monkey-patching BigInt.prototype.toJSON. but it seems like a different path is actually being called from graphql-mesh.

So I think we should change the internal and external representation to handle it more explicitly to resolve the graphql-mesh issue and further potential issues.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How Has This Been Tested?

ardatan/graphql-mesh#1355 (comment)

Test Environment:

• OS: macOS 11.2.3
• GraphQL Scalars Version: 1.9.0
• NodeJS: 14.16.0

Checklist:

• I have followed the CONTRIBUTING doc and the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests and linter rules pass locally with my changes
• Any dependent changes have been merged and published in downstream modules

[cometkim]

This is for using test command with a suite name like: yarn test BigInt

[ardatan]

Let me know what you think about my comments @cometkim

[cometkim]

@ardatan I think I missed it. what comments do you mean?

[ardatan]

Why do we avoid using primitive bigint values? Adding toJSON or this package already fixes JSON serialization which is not related to graphql-js.
And we need a failing reproduction before accepting this PR.
https://github.com/ardatan/json-bigint-patch
Serializing BigInt as string might cause an unwanted coercion on JSON data transferred via HTTP.

[cometkim]

Rather than implicitly intending to lose precision, it's better to let clients handle string correctly. Consider the case of using a long int as an id field. Even considering the JS clients, they may prefer string over numbers with lost precision as the BigInt() argument.

In the case of protobufjs, they already have the correct cast for the typeof value ==='number' and typeof value ==='string' cases.

But since they doesn't use the bigint primitive yet, it will be fallback empty ({ low: 0, high: 0, unsigned: false }) value.

(This problem is already being reproduced in the official example of graphql-mesh, and we should fix this in graphql-mesh anyway. Need a patch for graphql-scalars or protobufjs I think)

This is not graphql related and is a specific client issue, but considering that bigint hasn't been used in JavaScript widely, it's potentially more problematic than string.

[cometkim]

Also, the patch would be avoided some cases. it makes the whole library incompatible in sandboxing environments or workers where global scope cannot be modified.

[ardatan]

Closed in favor of ardatan/graphql-mesh#1987 and protobufjs/protobuf.js#1592