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

Add support for BigInt #156

Open
PeterKnego opened this issue Jan 4, 2021 · 7 comments
Open

Add support for BigInt #156

PeterKnego opened this issue Jan 4, 2021 · 7 comments

Comments

@PeterKnego
Copy link

BigInt is one of the standard built-in types in Typescript. At the moment TypedJSON does not support parsing BigInt out of the box. The underlying problem is also that the standard JSON.parse() does not support parsing bigint - it coerces bigints into number, which loses precision.

Currently BigInt can be parsed by TypedJSON via custom de-/serializes and use of an external JSON parser that supports bigint parsing: https://github.com/sidorares/json-bigint.

Suggestion: add support for BigInts to TypedJSON

@MatthiasKunnen
Copy link
Contributor

MatthiasKunnen commented Jan 18, 2021

This will be possible in the next major version using mapType which was added in #150. The feature has yet to be documented but will work like this:

@jsonObject
class BigIntTest {

    @jsonMember
    big: bigint;
}

const typedJson = new TypedJSON(BigIntTest);

TypedJSON.mapType(BigInt, {
    deserializer: json => BigInt(json),
    serializer: value => value.toString(),
});

TypedJSON.parse({
    big: '123456789123456789123456789',
}, BigIntTest);

Note that here the BigInit value is represented by a string in JSON.

@PeterKnego
Copy link
Author

PeterKnego commented Jan 19, 2021

Hi Matthias, thanks for the reply.

While using mapType will make it easier to register (de)serializers, it will not solve the underlying problem of BigInts being clipped by JSON.parse().

Since BigInt is a built-in type in Typescript I would expect it to be supported as a first-class type in TypedJSON.

@MatthiasKunnen
Copy link
Contributor

MatthiasKunnen commented Jan 19, 2021

I see where you're coming from but do not believe we would support this anytime soon due to the need for using a custom JSON.parse method. Using the third-party JSON.parse library you mentioned has the following issues:

  • Increases bundle size
  • Only solves the problem for BigInt. If, for example, decimal is approved we will need another custom JSON.parse library for that or need to maintain our own. I would much prefer using the JSON.parse method provided by the browser or Node. Especially since it is likely to be faster and better tested.

Another thing to note is that while BigInt is a built-in type in TypeScript, it appears to only be available when targeting esnext.

Presenting precise numbers in JavaScript has long been an issue and will probably remain one for a while longer. Perhaps https://github.com/tc39/proposal-json-parse-with-source will solve these problems. In the meantime I strongly recommend to represent numbers outside of IEEE 754 precision as strings in JSON. If strings are used then the example shown above will preserve precision.

I've used the decimals represented by strings approach myself in applications that deal with money and can confirm it works perfectly. PayPal uses it too.

@rjkz808
Copy link

rjkz808 commented Jan 27, 2021

Personally, I think mapTypes is not the best way to do it. It can only accept BigInt constructor, so all the BigInt fields should be declared like that (as shown in README):

TypedJSON.mapType(BigInt, {
    deserializer: (value) => value == null ? value : BigInt(json),
    serializer: (value) => value == null ? value : value.toString(),
});

@jsonObject
class MyData {
    @jsonMember
    public amount: BigInt = BigInt(0);
}

But assigning BigInt type to the class field means it'll be of type BigInt at compile-time, not a primitive bigint type. So, you can't really use it in any calculations, because code like this will not compile:

const serializer = new TypedJSON(MyData);
const data = serializer.parse('{ "amount": "2" }');

if (!data) {
    throw new Error('Failed to parse data');
}

const x: bigint = BigInt(2);
console.log(data.amount + x); // The issue is here, x is a primitive 'bigint', and data.amount is 'BigInt'

So, to overcome this issue, I usually use something like this in my code:

function jsonBigIntMember(): PropertyDecorator {
    return jsonMember({
        deserializer: (json) => (json == null ? json : BigInt(json)),
        serializer: (value) => (value == null ? value : value.toString()),
    });
}

@jsonObject
class MyData {
    @jsonBigIntMember()
    public amount: bigint = BigInt(0); // Notice it's of type 'bigint' now
}

const serializer = new TypedJSON(MyData);
const data = serializer.parse('{ "amount": "2" }');

if (!data) {
    throw new Error('Failed to parse data');
}

const x: bigint = BigInt(2);
console.log(data.amount + x); // No compilation errors!

@rjkz808
Copy link

rjkz808 commented Jan 27, 2021

@MatthiasKunnen maybe it'll be a good idea to add this decorator to the TypedJSON library itself?

export const jsonBigIntMember = jsonMember({
    deserializer: (json) => (json == null ? json : BigInt(json)),
    serializer: (value) => (value == null ? value : value.toString()),
});

@MatthiasKunnen
Copy link
Contributor

MatthiasKunnen commented Jan 27, 2021

@rjkz808, there was a mistake in my example. There should not have been a new operator for BigInt and the property type should've been bigint, not BigInt. This should work for you:

import {jsonObject, jsonMember, TypedJSON} from 'typedjson';

TypedJSON.mapType(BigInt, {
    deserializer: json => json == null ? json : BigInt(json),
    serializer: value => value == null ? value : value.toString(),
});

@jsonObject
class MappedTypes {

    @jsonMember
    cryptoKey: bigint;
}

const result = TypedJSON.parse({cryptoKey: '1234567890123456789'}, MappedTypes);
const x = BigInt(1);
console.log(result.cryptoKey + x);

@atomictag
Copy link

atomictag commented Feb 16, 2022

Hi there, thanks for this awesome library.
The mapType approach works for de-serialization but not serialization back to JSON:

    TypedJSON.mapType(BigInt, {
      deserializer: json => (json == null ? json : BigInt(json)),
      serializer: value => (value == null ? value : value.toString()),
    });

    @jsonObject
    class MappedTypes {
      @jsonMember
      cryptoKey!: bigint;
    }

    const json = { cryptoKey: "1234567890123456789" };
    const result = TypedJSON.parse(json, MappedTypes) as MappedTypes;

    // De-serialization works
    expect(result.cryptoKey + BigInt(1)).toBe(1234567890123456790n); // OK

    // Serialization throws
    TypedJSON.toPlainJson(result, MappedTypes);
    // TypeError: Could not serialize 'MappedTypes.cryptoKey': expected 'BigInt', got 'BigInt'

The problem is that the isInstanceOf sanity check done by convertSingleValue in serializer.ts does not pass.

Adding a simple extra condition to isInstanceOf in helpers.ts would solve this issue (and perhaps others of similar kind as well):

 ...
 else if(value.constructor === constructor) {
    return true;
 }

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

No branches or pull requests

4 participants