Skip to content
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

Restrict presence object type to JSON serializable values #898

Merged
merged 5 commits into from
Sep 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,6 @@ packages/sdk/src/api/yorkie/v1/resources_grpc_web_pb.d.ts
packages/sdk/src/api/yorkie/v1/resources_pb.d.ts
packages/sdk/test/vitest.d.ts
packages/sdk/lib

# examples
examples/react-tldraw/src/tldraw.d.ts
6 changes: 6 additions & 0 deletions examples/react-tldraw/src/tldraw.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { Indexable, Json } from '@yorkie-js-sdk/src/document/document';
import { TDUser } from '@tldraw/tldraw';

declare module '@tldraw/tldraw' {
interface TDUser extends Indexable {}
}
2 changes: 1 addition & 1 deletion examples/vanilla-quill/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ function toDeltaOperation<T extends TextValueType>(
): DeltaOperation {
const { embed, ...restAttributes } = textValue.attributes ?? {};
if (embed) {
return { insert: JSON.parse(embed), attributes: restAttributes };
return { insert: embed, attributes: restAttributes };
}

return {
Expand Down
2 changes: 1 addition & 1 deletion examples/vanilla-quill/src/type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,5 @@ export type YorkieDoc = {
export type YorkiePresence = {
username: string;
color: string;
selection: TextPosStructRange | undefined;
selection?: TextPosStructRange;
};
3 changes: 2 additions & 1 deletion packages/sdk/src/client/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import { OpSource } from '@yorkie-js-sdk/src/document/operation/operation';
import { createAuthInterceptor } from '@yorkie-js-sdk/src/client/auth_interceptor';
import { createMetricInterceptor } from '@yorkie-js-sdk/src/client/metric_interceptor';
import { validateSerializable } from '../util/validator';
import { Json } from '@yorkie-js-sdk/src/document/document';

/**
* `SyncMode` defines synchronization modes for the PushPullChanges API.
Expand Down Expand Up @@ -607,7 +608,7 @@ export class Client {
public broadcast(
docKey: DocumentKey,
topic: string,
payload: any,
payload: Json,
): Promise<void> {
if (!this.isActive()) {
throw new YorkieError(
Expand Down
15 changes: 1 addition & 14 deletions packages/sdk/src/devtools/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,7 @@
import type { PrimitiveValue } from '@yorkie-js-sdk/src/document/crdt/primitive';
import type { CRDTTreePosStruct } from '@yorkie-js-sdk/src/document/crdt/tree';
import { CounterValue } from '@yorkie-js-sdk/src/document/crdt/counter';

/**
* `Json` represents a JSON value.
*
* TODO(hackerwins): We need to replace `Indexable` with `Json`.
*/
export type Json =
| string
| number
| boolean
// eslint-disable-next-line @typescript-eslint/ban-types
| null
| { [key: string]: Json }
| Array<Json>;
import { Json } from '@yorkie-js-sdk/src/document/document';

/**
* `Client` represents a client value in devtools.
Expand Down
2 changes: 1 addition & 1 deletion packages/sdk/src/document/change/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ export class ChangeContext<P extends Indexable = Indexable> {

const reversePresence: Partial<P> = {};
for (const key of this.reversePresenceKeys) {
reversePresence[key as keyof P] = this.previousPresence[key];
reversePresence[key as keyof P] = this.previousPresence[key as keyof P];
}
return reversePresence;
}
Expand Down
22 changes: 17 additions & 5 deletions packages/sdk/src/document/document.ts
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ export interface PresenceChangedEvent<P extends Indexable>

export interface BroadcastEvent extends BaseDocEvent {
type: DocEventType.Broadcast;
value: { clientID: ActorID; topic: string; payload: any };
value: { clientID: ActorID; topic: string; payload: Json };
error?: ErrorFn;
}

Expand Down Expand Up @@ -421,13 +421,25 @@ export type DocEventCallback<P extends Indexable> =
DocEventCallbackMap<P>[DocEventTopic];

/**
* Indexable key, value
* `Json` represents the JSON data type. It is used to represent the data
* structure of the document.
*/
export type Json = JsonPrimitive | JsonArray | JsonObject;

// eslint-disable-next-line @typescript-eslint/ban-types
type JsonPrimitive = string | number | boolean | null;
type JsonArray = Array<Json>;
type JsonObject = { [key: string]: Json | undefined };

/**
* `Indexable` represents the type of the indexable object. It is used to
* represent the presence information of the client.
* @public
*/
export type Indexable = Record<string, any>;
export type Indexable = Record<string, Json>;

/**
* Document key type
* `DocumentKey` represents the key of the document.
* @public
*/
export type DocumentKey = string;
Expand Down Expand Up @@ -2058,7 +2070,7 @@ export class Document<T, P extends Indexable = Indexable> {
/**
* `broadcast` the payload to the given topic.
*/
public broadcast(topic: string, payload: any, error?: ErrorFn) {
public broadcast(topic: string, payload: Json, error?: ErrorFn) {
const broadcastEvent: LocalBroadcastEvent = {
type: DocEventType.LocalBroadcast,
value: { topic, payload },
Expand Down
12 changes: 6 additions & 6 deletions packages/sdk/src/document/json/tree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,14 +82,14 @@ function buildDescendants(
parent.append(textNode);
} else {
const { children = [] } = treeNode as ElementNode;
let { attributes } = treeNode as ElementNode;
const { attributes } = treeNode as ElementNode;
let attrs;

if (typeof attributes === 'object' && !isEmpty(attributes)) {
attributes = stringifyObjectValues(attributes);
const stringifiedAttributes = stringifyObjectValues(attributes);
attrs = new RHT();

for (const [key, value] of Object.entries(attributes)) {
for (const [key, value] of Object.entries(stringifiedAttributes)) {
attrs.set(key, value, ticket);
}
}
Expand Down Expand Up @@ -121,14 +121,14 @@ function createCRDTTreeNode(context: ChangeContext, content: TreeNode) {
root = CRDTTreeNode.create(CRDTTreeNodeID.of(ticket, 0), type, value);
} else if (content) {
const { children = [] } = content as ElementNode;
let { attributes } = content as ElementNode;
const { attributes } = content as ElementNode;
let attrs;

if (typeof attributes === 'object' && !isEmpty(attributes)) {
attributes = stringifyObjectValues(attributes);
const stringifiedAttributes = stringifyObjectValues(attributes);
attrs = new RHT();

for (const [key, value] of Object.entries(attributes)) {
for (const [key, value] of Object.entries(stringifiedAttributes)) {
attrs.set(key, value, ticket);
}
}
Expand Down
6 changes: 4 additions & 2 deletions packages/sdk/src/util/object.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
* limitations under the License.
*/

import { Indexable } from '@yorkie-js-sdk/src/document/document';

/**
* `deepcopy` returns a deep copy of the given object.
*/
Expand All @@ -40,7 +42,7 @@ export const isEmpty = (object: object) => {
/**
* `stringifyObjectValues` makes values of attributes to JSON parsable string.
*/
export const stringifyObjectValues = <A extends object>(
export const stringifyObjectValues = <A extends Indexable>(
attributes: A,
): Record<string, string> => {
const attrs: Record<string, string> = {};
Expand All @@ -53,7 +55,7 @@ export const stringifyObjectValues = <A extends object>(
/**
`parseObjectValues` returns the JSON parsable string values to the origin states.
*/
export const parseObjectValues = <A extends object>(
export const parseObjectValues = <A extends Indexable>(
attrs: Record<string, string>,
): A => {
const attributes: Record<string, unknown> = {};
Expand Down
2 changes: 1 addition & 1 deletion packages/sdk/src/util/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
/**
* `validateSerializable` returns whether the given value is serializable or not.
*/
export const validateSerializable = (value: any): boolean => {
export const validateSerializable = (value: unknown): boolean => {
try {
const serialized = JSON.stringify(value);

Expand Down
2 changes: 2 additions & 0 deletions packages/sdk/test/integration/client_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -900,6 +900,8 @@ describe.sequential('Client', function () {
eventCollector.add(error.message);
};

// @ts-ignore
// Disable type checking for testing purposes
doc.broadcast(broadcastTopic, payload, errorHandler);

await eventCollector.waitAndVerifyNthEvent(1, broadcastErrMessage);
Expand Down
13 changes: 8 additions & 5 deletions packages/sdk/test/integration/integration_helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,15 @@ export function toDocKey(title: string): string {
.replace(/[^a-z0-9-]/g, '-');
}

export async function withTwoClientsAndDocuments<T>(
export async function withTwoClientsAndDocuments<
T,
P extends Indexable = Indexable,
>(
callback: (
c1: Client,
d1: Document<T>,
d1: Document<T, P>,
c2: Client,
d2: Document<T>,
d2: Document<T, P>,
) => Promise<void>,
title: string,
syncMode: SyncMode = SyncMode.Manual,
Expand All @@ -29,8 +32,8 @@ export async function withTwoClientsAndDocuments<T>(
await client2.activate();

const docKey = `${toDocKey(title)}-${new Date().getTime()}`;
const doc1 = new yorkie.Document<T>(docKey);
const doc2 = new yorkie.Document<T>(docKey);
const doc1 = new yorkie.Document<T, P>(docKey);
const doc2 = new yorkie.Document<T, P>(docKey);

await client1.attach(doc1, { syncMode });
await client2.attach(doc2, { syncMode });
Expand Down
Loading