-
Notifications
You must be signed in to change notification settings - Fork 11
Add JS Map support for encoding & decoding #5
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
Changes from 1 commit
a1addc5
7342b02
9f1341c
a537057
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,6 +38,16 @@ export type DecoderOptions<ContextType = undefined> = Readonly< | |
*/ | ||
useRawBinaryStrings: boolean; | ||
|
||
/** | ||
* If true, the decoder will use the Map object to store map values. If false, it will use plain | ||
* objects. Defaults to false. | ||
* | ||
* Besides the type of container, the main difference is that Map objects support a wider range | ||
* of key types. Plain objects only support string keys, while Map objects support strings, | ||
* numbers, bigints, and Uint8Arrays. | ||
*/ | ||
useMap: boolean; | ||
|
||
/** | ||
* Maximum string length. | ||
* | ||
|
@@ -82,18 +92,22 @@ const STATE_ARRAY = "array"; | |
const STATE_MAP_KEY = "map_key"; | ||
const STATE_MAP_VALUE = "map_value"; | ||
|
||
type MapKeyType = string | number; | ||
type MapKeyType = string | number | bigint | Uint8Array; | ||
|
||
const isValidMapKeyType = (key: unknown): key is MapKeyType => { | ||
return typeof key === "string" || typeof key === "number"; | ||
}; | ||
function isValidMapKeyType(key: unknown, useMap: boolean): key is MapKeyType { | ||
if (useMap) { | ||
return typeof key === "string" || typeof key === "number" || typeof key === "bigint" || key instanceof Uint8Array; | ||
jannotti marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
// Plain objects support a more limited set of key types | ||
return typeof key === "string"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a breaking change, should we keep the existing logic looking for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's pretty strange to me that the upstream library was getting away with just coercing number keys to strings on objects, so I wanted to do away with that behavior now that there's a better alternative. Although, since it is a breaking change, I think I'd be willing to add another decoding option that could control that behavior if it's really what you wanted, since this has been an established practice in the upstream library for a while. |
||
} | ||
|
||
type StackMapState = { | ||
type: typeof STATE_MAP_KEY | typeof STATE_MAP_VALUE; | ||
size: number; | ||
key: MapKeyType | null; | ||
readCount: number; | ||
map: Record<string, unknown>; | ||
map: Record<string, unknown> | Map<MapKeyType, unknown>; | ||
}; | ||
|
||
type StackArrayState = { | ||
|
@@ -107,6 +121,8 @@ class StackPool { | |
private readonly stack: Array<StackState> = []; | ||
private stackHeadPosition = -1; | ||
|
||
constructor(private readonly useMap: boolean) {} | ||
|
||
public get length(): number { | ||
return this.stackHeadPosition + 1; | ||
} | ||
|
@@ -130,7 +146,7 @@ class StackPool { | |
state.type = STATE_MAP_KEY; | ||
state.readCount = 0; | ||
state.size = size; | ||
state.map = {}; | ||
state.map = this.useMap ? new Map() : {}; | ||
} | ||
|
||
private getUninitializedStateFromPool() { | ||
|
@@ -214,6 +230,7 @@ export class Decoder<ContextType = undefined> { | |
private readonly context: ContextType; | ||
private readonly intMode: IntMode; | ||
private readonly useRawBinaryStrings: boolean; | ||
private readonly useMap: boolean; | ||
private readonly maxStrLength: number; | ||
private readonly maxBinLength: number; | ||
private readonly maxArrayLength: number; | ||
|
@@ -227,20 +244,23 @@ export class Decoder<ContextType = undefined> { | |
private view = EMPTY_VIEW; | ||
private bytes = EMPTY_BYTES; | ||
private headByte = HEAD_BYTE_REQUIRED; | ||
private readonly stack = new StackPool(); | ||
private readonly stack: StackPool; | ||
|
||
public constructor(options?: DecoderOptions<ContextType>) { | ||
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.useRawBinaryStrings = options?.useRawBinaryStrings ?? false; | ||
this.useMap = options?.useMap ?? false; | ||
this.maxStrLength = options?.maxStrLength ?? UINT32_MAX; | ||
this.maxBinLength = options?.maxBinLength ?? UINT32_MAX; | ||
this.maxArrayLength = options?.maxArrayLength ?? UINT32_MAX; | ||
this.maxMapLength = options?.maxMapLength ?? UINT32_MAX; | ||
this.maxExtLength = options?.maxExtLength ?? UINT32_MAX; | ||
this.keyDecoder = options?.keyDecoder !== undefined ? options.keyDecoder : sharedCachedKeyDecoder; | ||
|
||
this.stack = new StackPool(this.useMap); | ||
} | ||
|
||
private reinitializeState() { | ||
|
@@ -404,7 +424,7 @@ export class Decoder<ContextType = undefined> { | |
this.complete(); | ||
continue DECODE; | ||
} else { | ||
object = {}; | ||
object = this.useMap ? new Map() : {}; | ||
} | ||
} else if (headByte < 0xa0) { | ||
// fixarray (1001 xxxx) 0x90 - 0x9f | ||
|
@@ -571,10 +591,11 @@ export class Decoder<ContextType = undefined> { | |
continue DECODE; | ||
} | ||
} else if (state.type === STATE_MAP_KEY) { | ||
if (!isValidMapKeyType(object)) { | ||
throw new DecodeError("The type of key must be string or number but " + typeof object); | ||
if (!isValidMapKeyType(object, this.useMap)) { | ||
const acceptableTypes = this.useMap ? "string, number, bigint, or Uint8Array" : "string"; | ||
throw new DecodeError(`The type of key must be ${acceptableTypes} but got ${typeof object}`); | ||
} | ||
if (object === "__proto__") { | ||
if (!this.useMap && object === "__proto__") { | ||
throw new DecodeError("The key __proto__ is not allowed"); | ||
} | ||
|
||
|
@@ -584,7 +605,11 @@ export class Decoder<ContextType = undefined> { | |
} else { | ||
// it must be `state.type === State.MAP_VALUE` here | ||
|
||
state.map[state.key!] = object; | ||
if (this.useMap) { | ||
(state.map as Map<MapKeyType, unknown>).set(state.key!, object); | ||
} else { | ||
(state.map as Record<string, unknown>)[state.key as string] = object; | ||
} | ||
state.readCount++; | ||
|
||
if (state.readCount === state.size) { | ||
|
@@ -650,7 +675,7 @@ export class Decoder<ContextType = undefined> { | |
} | ||
|
||
private decodeString(byteLength: number, headerOffset: number): string | Uint8Array { | ||
if (!this.useRawBinaryStrings || this.stateIsMapKey()) { | ||
if (!this.useRawBinaryStrings || (!this.useMap && this.stateIsMapKey())) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I read this right, does it mean if you are using maps and you set There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's correct, and your suggestion makes sense There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added two options for this in 7342b02 |
||
return this.decodeUtf8String(byteLength, headerOffset); | ||
} | ||
return this.decodeBinary(byteLength, headerOffset); | ||
|
Uh oh!
There was an error while loading. Please reload this page.