JohnWeisz / TypedJSON

Typed JSON parsing and serializing for TypeScript that preserves type information.
MIT License
603 stars 64 forks source link

Add support for BigInt #156

Open PeterKnego opened 3 years ago

PeterKnego commented 3 years ago

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 commented 3 years ago

This will be possible in the next major version using mapType which was added in https://github.com/JohnWeisz/TypedJSON/pull/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 commented 3 years ago

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 commented 3 years ago

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:

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 commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

@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 commented 2 years ago

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;
 }