yorkie-js-sdk
yorkie-js-sdk copied to clipboard
Restrict Presence Type to JSON Serializable Types
Description:
Currently, the presence type is defined to extend Indexable, and when transmitting presence via protobuf, it is converted to a string using JSON.stringify.
- presence type: https://github.com/yorkie-team/yorkie-js-sdk/blob/effd58643673765c0515c17c552723959a8e70e1/src/document/document.ts#L388
- protobuf: https://github.com/yorkie-team/yorkie-js-sdk/blob/02e9bc0044ba4ab1e643f182996feb4ab1ceefb2/src/api/yorkie/v1/resources.proto#L307-L309
- PR that changes presence to use
JSON.stringify: https://github.com/yorkie-team/yorkie-js-sdk/pull/311
The current type <P extends Indexable> allows any type as its value since type Indexable = Record<string, any>. Consequently, values that are not JSON serializable, such as byte array, Date, and Long, may not work correctly. Therefore, it is essential to restrict the type of P(presence) to only allow JSON serializable types not Indexable.
Reference) JSON type used in other libraries:
- JSON type generated when using Supabase Type Generation:
export type Json = string | number | boolean | null | { [key: string]: Json } | Json[];
- JSON type defined in liveblocks:
export type Json = JsonScalar | JsonArray | JsonObject;
export type JsonScalar = string | number | boolean | null;
export type JsonArray = Json[];
export type JsonObject = { [key: string]: Json | undefined };
Why: Using non-JSON serializable values in the presence type may lead to unexpected behavior.
Feel free to send a pull request.
I'll give it a try :)