Skip to content

Commit

Permalink
Review feedback
Browse files Browse the repository at this point in the history
BREAKING CHANGE: `useBigInt64` deprecated from Encoder and > 32 bit ints now get encoded as a uint64 rather than float64
  • Loading branch information
robdmoore committed Feb 3, 2024
1 parent eeeec65 commit 6039e48
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 35 deletions.
5 changes: 2 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@ console.log(buffer);
| ------------------- | -------------- | ----------------------------- |
| extensionCodec | ExtensionCodec | `ExtensionCodec.defaultCodec` |
| context | user-defined | - |
| useInt64 | boolean | false |
| forceBigIntToInt64 | boolean | false |
| maxDepth | number | `100` |
| initialBufferSize | number | `2048` |
Expand Down Expand Up @@ -381,9 +380,9 @@ This library does not handle decoding BigInt by default, but you have three opti

**Encoding**

This library will encode a BigInt into a MessagePack int64/uint64 if it is > 32-bit OR you set `forceBigIntToInt64` to `true`. This library will encode a `number` that is > 32-bit into a MessagePack int64/uint64 if it is > 32-bit ONLY if you set `useInt64` to true, otherwise it encodes it as a MessagePack float64.
This library will encode a BigInt into a MessagePack int64/uint64 if it is > 32-bit OR you set `forceBigIntToInt64` to `true`. This library will encode a `number` that is > 32-bit into a MessagePack int64/uint64 if it is > 32-bit.

If you set `forceBigIntToInt64` to `true` the note:
If you set `forceBigIntToInt64` to `true` note:

- A bigint is encoded in 8 byte binaries even if it's a small integer
- A bigint must be smaller than the max value of the uint64 and larger than the min value of the int64. Otherwise the behavior is undefined.
Expand Down
2 changes: 1 addition & 1 deletion src/Decoder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ export class Decoder<ContextType = undefined> {
this.extensionCodec = options?.extensionCodec ?? (ExtensionCodec.defaultCodec as ExtensionCodecType<ContextType>);
this.context = (options as { context: ContextType } | undefined)?.context as ContextType; // needs a type assertion because EncoderOptions has no context property when ContextType is undefined

this.intMode = options?.intMode ?? options?.useBigInt64 ? IntMode.AS_ENCODED : IntMode.UNSAFE_NUMBER;
this.intMode = options?.intMode ?? (options?.useBigInt64 ? IntMode.AS_ENCODED : IntMode.UNSAFE_NUMBER);
this.maxStrLength = options?.maxStrLength ?? UINT32_MAX;
this.maxBinLength = options?.maxBinLength ?? UINT32_MAX;
this.maxArrayLength = options?.maxArrayLength ?? UINT32_MAX;
Expand Down
36 changes: 8 additions & 28 deletions src/Encoder.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { utf8Count, utf8Encode } from "./utils/utf8";
import { ExtensionCodec, ExtensionCodecType } from "./ExtensionCodec";
import { setInt64, setUint64 } from "./utils/int";
import { ensureUint8Array } from "./utils/typedArrays";
import type { ExtData } from "./ExtData";
import type { ContextOf } from "./context";
import { setInt64, setUint64 } from "./utils/int";

export const DEFAULT_MAX_DEPTH = 100;
export const DEFAULT_INITIAL_BUFFER_SIZE = 2048;
Expand All @@ -12,13 +12,6 @@ export type EncoderOptions<ContextType = undefined> = Partial<
Readonly<{
extensionCodec: ExtensionCodecType<ContextType>;

/**
* Encodes a `number` greater than 32-bit as Int64 or Uint64 if it's set to true, otherwise encode as float64.
*
* Defaults to false.
*/
useInt64: boolean;

/**
* Encodes bigint as Int64 or Uint64 if it's set to true, regardless of the size of bigint number.
* {@link forceIntegerToFloat} does not affect bigint.
Expand All @@ -29,9 +22,6 @@ export type EncoderOptions<ContextType = undefined> = Partial<
*/
forceBigIntToInt64: boolean;

/** @deprecated Alias of `forceBigIntToInt64` */
useBigInt64: boolean;

/**
* The maximum depth in nested objects and arrays.
*
Expand Down Expand Up @@ -86,7 +76,6 @@ export class Encoder<ContextType = undefined> {
private readonly extensionCodec: ExtensionCodecType<ContextType>;
private readonly context: ContextType;
private readonly forceBigIntToInt64: boolean;
private readonly useInt64: boolean;
private readonly maxDepth: number;
private readonly initialBufferSize: number;
private readonly sortKeys: boolean;
Expand All @@ -102,8 +91,7 @@ export class Encoder<ContextType = undefined> {
this.extensionCodec = options?.extensionCodec ?? (ExtensionCodec.defaultCodec as ExtensionCodecType<ContextType>);
this.context = (options as { context: ContextType } | undefined)?.context as ContextType; // needs a type assertion because EncoderOptions has no context property when ContextType is undefined

this.forceBigIntToInt64 = options?.forceBigIntToInt64 ?? options?.useBigInt64 ?? false;
this.useInt64 = options?.useInt64 ?? false;
this.forceBigIntToInt64 = options?.forceBigIntToInt64 ?? false;
this.maxDepth = options?.maxDepth ?? DEFAULT_MAX_DEPTH;
this.initialBufferSize = options?.initialBufferSize ?? DEFAULT_INITIAL_BUFFER_SIZE;
this.sortKeys = options?.sortKeys ?? false;
Expand Down Expand Up @@ -150,15 +138,9 @@ export class Encoder<ContextType = undefined> {
} else if (typeof object === "boolean") {
this.encodeBoolean(object);
} else if (typeof object === "number") {
if (!this.forceIntegerToFloat) {
this.encodeNumber(object);
} else {
this.encodeNumberAsFloat(object);
}
this.encodeNumber(object);
} else if (typeof object === "string") {
this.encodeString(object);
} else if (this.forceBigIntToInt64 && typeof object === "bigint") {
this.encodeBigIntAsInt64(object);
} else {
this.encodeObject(object, depth);
}
Expand Down Expand Up @@ -213,12 +195,10 @@ export class Encoder<ContextType = undefined> {
// uint 32
this.writeU8(0xce);
this.writeU32(object);
} else if (this.useInt64) {
} else {
// uint 64
this.writeU8(0xcf);
this.writeU64(object);
} else {
this.encodeNumberAsFloat(object);
}
} else {
if (object >= -0x20) {
Expand All @@ -236,12 +216,10 @@ export class Encoder<ContextType = undefined> {
// int 32
this.writeU8(0xd2);
this.writeI32(object);
} else if (this.useInt64) {
} else {
// int 64
this.writeU8(0xd3);
this.writeI64(object);
} else {
this.encodeNumberAsFloat(object);
}
}
} else {
Expand All @@ -262,7 +240,9 @@ export class Encoder<ContextType = undefined> {
}

private encodeBigInt(object: bigint) {
if (object >= 0) {
if (this.forceBigIntToInt64) {
this.encodeBigIntAsInt64(object);
} else if (object >= 0) {
if (object < 0x100000000 || this.forceIntegerToFloat) {
// uint 32 or lower, or force to float
this.encodeNumber(Number(object));
Expand Down
17 changes: 14 additions & 3 deletions test/bigint64.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import assert from "assert";
import { encode, decode, IntMode, ExtensionCodec, DecodeError } from "../src";
import { getInt64, getUint64 } from "src/utils/int";
import { getInt64, getUint64 } from "../src/utils/int";

describe("useBigInt64: true", () => {
before(function () {
Expand All @@ -27,14 +27,25 @@ describe("useBigInt64: true", () => {
assert.deepStrictEqual(decode(encoded, { useBigInt64: true }), value);
});

it("encodes and decodes values with numbers and bigints", () => {
it("encodes and decodes values with numbers and bigints - MIXED", () => {
const value = {
ints: [0, Number.MAX_SAFE_INTEGER, Number.MIN_SAFE_INTEGER],
nums: [Number.NaN, Math.PI, Math.E, Number.POSITIVE_INFINITY, Number.NEGATIVE_INFINITY],
bigints: [BigInt(Number.MAX_SAFE_INTEGER) + BigInt(1), BigInt(Number.MIN_SAFE_INTEGER) - BigInt(1)],
};
const encoded = encode(value, { forceBigIntToInt64: true });
const decoded = decode(encoded, { intMode: IntMode.MIXED });
assert.deepStrictEqual(decoded, value);
});

it("encodes and decodes values with numbers and bigints - AS_ENCODED", () => {
const value = {
ints: [0, Math.pow(2, 32) - 1, -1 * Math.pow(2, 31)],
nums: [Number.NaN, Math.PI, Math.E, Number.POSITIVE_INFINITY, Number.NEGATIVE_INFINITY],
bigints: [BigInt(0), BigInt(Number.MAX_SAFE_INTEGER) + BigInt(1), BigInt(Number.MIN_SAFE_INTEGER) - BigInt(1)],
};
const encoded = encode(value, { forceBigIntToInt64: true });
const decoded = decode(encoded, { useBigInt64: true });
const decoded = decode(encoded, { intMode: IntMode.AS_ENCODED });
assert.deepStrictEqual(decoded, value);
});
});
Expand Down

0 comments on commit 6039e48

Please sign in to comment.