Use json-bigint for parsing server response to avoid truncation of large integers

[janfh]

Fixes #324

JSON.parse will parse integers to Number (IEEE 754 64-bit float) and is limited to values less than what might be returned from the server. This will cause truncation of values less than -9007199254740991 and greater than 9007199254740991 (Number.MAX_SAFE_INTEGER).

This PR changes SQLJob to use json-bigint for parsing the server response. Values will be deserialized to BigInt instead of Number.

Before change:

After change:

[worksofliam]

@janfh Out of curiosity, is a new package needed for this? Can't we use the built-in BigInt support?

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/BigInt#browser_compatibility

[janfh]

@janfh Out of curiosity, is a new package needed for this? Can't we use the built-in BigInt support?

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/BigInt#browser_compatibility

@worksofliam : I can try a different approach

[janfh]

Using BigInt causes other problems like JSON.stringify does not support serializing it.

It looks like this is working without the json-bigint package:

                let response: ServerResponse = JSON.parse(thisMsg, (_key, value, context) => {
                  if (typeof value === 'number' && ('' + value !== context.source)) {
                    return context.source;
                  }
                  return value;
                });

The context parameter on the reviver function is quite new, but it seems to work fine even if vscode complains about the syntax.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/parse#browser_compatibility

@worksofliam : what do you think?

[worksofliam]

@janfh Sadly Reviver has context parameter is not available until Node.js 21, and VS Code uses version 20 internally, so we will have to wait.

Personally, I think we should not merge this PR, and perhaps wait for VS Code to upgrade Node.js version instead of adding a temp work around. I do think an issue should be opened to track this, and have it fixed soon after the update.

What do you think?

[janfh]

Personally, I think we should not merge this PR, and perhaps wait for VS Code to upgrade Node.js version instead of adding a temp work around. I do think an issue should be opened to track this, and have it fixed soon after the update.

What do you think?

We can wait and see, and maybe there is another way of solving this. I guess it's kind of unfortunate that the current release will return wrong values without warning if it's big enough, but maybe it doesn't happen that often?