yorkie-js-sdk icon indicating copy to clipboard operation
yorkie-js-sdk copied to clipboard

Restrict Presence Type to JSON Serializable Types

Open chacha912 opened this issue 11 months ago • 1 comments

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:

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.

chacha912 avatar Jul 27 '23 12:07 chacha912

Feel free to send a pull request.

hackerwins avatar Jul 28 '23 02:07 hackerwins