From 1691d6dc4490550bffe81146cb46bb5e9ddb4139 Mon Sep 17 00:00:00 2001 From: Yourim Cha <81357083+chacha912@users.noreply.github.com> Date: Mon, 20 May 2024 17:45:26 +0900 Subject: [PATCH] Enhance type inference in Document.subscribe This commit improves the type inference for the types used in Document.subscribe. Additionally, it removes the code that forces casting in the test code to align with these improvements. --------- Co-authored-by: Hackerwins --- src/document/crdt/tree.ts | 46 +++++++++++----- src/document/document.ts | 83 ++++++++++++++++++----------- src/document/operation/operation.ts | 2 +- test/integration/client_test.ts | 14 ++--- test/integration/document_test.ts | 29 ++++++---- test/integration/tree_test.ts | 40 +++++++------- test/unit/document/document_test.ts | 28 +++++++--- 7 files changed, 154 insertions(+), 88 deletions(-) diff --git a/src/document/crdt/tree.ts b/src/document/crdt/tree.ts index 457ab99c9..31ad93411 100644 --- a/src/document/crdt/tree.ts +++ b/src/document/crdt/tree.ts @@ -87,16 +87,37 @@ export enum TreeChangeType { /** * `TreeChange` represents the change in the tree. */ -export interface TreeChange { - actor: ActorID; - type: TreeChangeType; - from: number; - to: number; - fromPath: Array; - toPath: Array; - value?: Array | { [key: string]: any } | Array; - splitLevel?: number; -} +export type TreeChange = + | { + actor: ActorID; + type: TreeChangeType.Content; + from: number; + to: number; + fromPath: Array; + toPath: Array; + value?: Array; + splitLevel?: number; + } + | { + actor: ActorID; + type: TreeChangeType.Style; + from: number; + to: number; + fromPath: Array; + toPath: Array; + value: { [key: string]: string }; + splitLevel?: number; + } + | { + actor: ActorID; + type: TreeChangeType.RemoveStyle; + from: number; + to: number; + fromPath: Array; + toPath: Array; + value?: Array; + splitLevel?: number; + }; /** * `CRDTTreePos` represent a position in the tree. It is used to identify a @@ -876,7 +897,6 @@ export class CRDTTree extends CRDTGCElement { const [toParent, toLeft] = this.findNodesAndSplitText(range[1], editedAt); const changes: Array = []; - const value = attributesToRemove ? attributesToRemove : undefined; this.traverseInPosRange( fromParent, fromLeft, @@ -893,13 +913,13 @@ export class CRDTTree extends CRDTGCElement { } changes.push({ + actor: editedAt.getActorID()!, type: TreeChangeType.RemoveStyle, from: this.toIndex(fromParent, fromLeft), to: this.toIndex(toParent, toLeft), fromPath: this.toPath(fromParent, fromLeft), toPath: this.toPath(toParent, toLeft), - actor: editedAt.getActorID()!, - value, + value: attributesToRemove, }); } }, diff --git a/src/document/document.ts b/src/document/document.ts index ef7d677b4..f222811e0 100644 --- a/src/document/document.ts +++ b/src/document/document.ts @@ -369,6 +369,28 @@ export interface PresenceChangedEvent

value: { clientID: ActorID; presence: P }; } +type DocEventCallbackMap

= { + default: NextFn< + | SnapshotEvent + | LocalChangeEvent + | RemoteChangeEvent + >; + presence: NextFn< + | InitializedEvent

+ | WatchedEvent

+ | UnwatchedEvent

+ | PresenceChangedEvent

+ >; + 'my-presence': NextFn | PresenceChangedEvent

>; + others: NextFn | UnwatchedEvent

| PresenceChangedEvent

>; + connection: NextFn; + sync: NextFn; + all: NextFn>; +}; +export type DocEventTopic = keyof DocEventCallbackMap; +export type DocEventCallback

= + DocEventCallbackMap

[DocEventTopic]; + /** * Indexable key, value * @public @@ -710,7 +732,7 @@ export class Document { * The callback will be called when the document is changed. */ public subscribe( - nextOrObserver: Observer | NextFn, + next: DocEventCallbackMap

['default'], error?: ErrorFn, complete?: CompleteFn, ): Unsubscribe; @@ -721,7 +743,7 @@ export class Document { */ public subscribe( type: 'presence', - next: NextFn>, + next: DocEventCallbackMap

['presence'], error?: ErrorFn, complete?: CompleteFn, ): Unsubscribe; @@ -731,7 +753,7 @@ export class Document { */ public subscribe( type: 'my-presence', - next: NextFn>, + next: DocEventCallbackMap

['my-presence'], error?: ErrorFn, complete?: CompleteFn, ): Unsubscribe; @@ -742,7 +764,7 @@ export class Document { */ public subscribe( type: 'others', - next: NextFn>, + next: DocEventCallbackMap

['others'], error?: ErrorFn, complete?: CompleteFn, ): Unsubscribe; @@ -752,7 +774,7 @@ export class Document { */ public subscribe( type: 'connection', - next: NextFn>, + next: DocEventCallbackMap

['connection'], error?: ErrorFn, complete?: CompleteFn, ): Unsubscribe; @@ -762,7 +784,7 @@ export class Document { */ public subscribe( type: 'sync', - next: NextFn>, + next: DocEventCallbackMap

['sync'], error?: ErrorFn, complete?: CompleteFn, ): Unsubscribe; @@ -775,7 +797,9 @@ export class Document { TOperationInfo extends OperationInfoOf, >( targetPath: TPath, - next: NextFn>, + next: NextFn< + LocalChangeEvent | RemoteChangeEvent + >, error?: ErrorFn, complete?: CompleteFn, ): Unsubscribe; @@ -784,7 +808,7 @@ export class Document { */ public subscribe( type: 'all', - next: NextFn>, + next: DocEventCallbackMap

['all'], error?: ErrorFn, complete?: CompleteFn, ): Unsubscribe; @@ -795,11 +819,13 @@ export class Document { TPath extends PathOf, TOperationInfo extends OperationInfoOf, >( - arg1: TPath | string | Observer> | NextFn>, + arg1: TPath | DocEventTopic | DocEventCallbackMap

['default'], arg2?: - | NextFn> - | NextFn> - | NextFn> + | NextFn< + | LocalChangeEvent + | RemoteChangeEvent + > + | DocEventCallback

| ErrorFn, arg3?: ErrorFn | CompleteFn, arg4?: CompleteFn, @@ -809,7 +835,7 @@ export class Document { throw new Error('Second argument must be a callback function'); } if (arg1 === 'presence') { - const callback = arg2 as NextFn>; + const callback = arg2 as DocEventCallbackMap

['presence']; return this.eventStream.subscribe( (event) => { for (const docEvent of event) { @@ -830,21 +856,19 @@ export class Document { ); } if (arg1 === 'my-presence') { - const callback = arg2 as NextFn>; + const callback = arg2 as DocEventCallbackMap

['my-presence']; return this.eventStream.subscribe( (event) => { for (const docEvent of event) { if ( docEvent.type !== DocEventType.Initialized && - docEvent.type !== DocEventType.Watched && - docEvent.type !== DocEventType.Unwatched && docEvent.type !== DocEventType.PresenceChanged ) { continue; } if ( - docEvent.type !== DocEventType.Initialized && + docEvent.type === DocEventType.PresenceChanged && docEvent.value.clientID !== this.changeID.getActorID() ) { continue; @@ -858,7 +882,7 @@ export class Document { ); } if (arg1 === 'others') { - const callback = arg2 as NextFn>; + const callback = arg2 as DocEventCallbackMap

['others']; return this.eventStream.subscribe( (event) => { for (const docEvent of event) { @@ -880,7 +904,7 @@ export class Document { ); } if (arg1 === 'connection') { - const callback = arg2 as NextFn>; + const callback = arg2 as DocEventCallbackMap

['connection']; return this.eventStream.subscribe( (event) => { for (const docEvent of event) { @@ -895,7 +919,7 @@ export class Document { ); } if (arg1 === 'sync') { - const callback = arg2 as NextFn>; + const callback = arg2 as DocEventCallbackMap

['sync']; return this.eventStream.subscribe( (event) => { for (const docEvent of event) { @@ -910,31 +934,28 @@ export class Document { ); } if (arg1 === 'all') { - const callback = arg2 as NextFn>; + const callback = arg2 as DocEventCallbackMap

['all']; return this.eventStream.subscribe(callback, arg3, arg4); } const target = arg1; - const callback = arg2 as NextFn>; + const callback = arg2 as NextFn< + | LocalChangeEvent + | RemoteChangeEvent + >; return this.eventStream.subscribe( (event) => { for (const docEvent of event) { if ( - docEvent.type !== DocEventType.Snapshot && docEvent.type !== DocEventType.LocalChange && docEvent.type !== DocEventType.RemoteChange ) { continue; } - if (docEvent.type === DocEventType.Snapshot) { - target === '$' && callback(docEvent); - continue; - } - - const targetOps: Array = []; + const targetOps: Array = []; for (const op of docEvent.value.operations) { if (this.isSameElementOrChildOf(op.path, target)) { - targetOps.push(op); + targetOps.push(op as TOperationInfo); } } targetOps.length && @@ -949,7 +970,7 @@ export class Document { ); } if (typeof arg1 === 'function') { - const callback = arg1 as NextFn>; + const callback = arg1 as DocEventCallbackMap

['default']; const error = arg2 as ErrorFn; const complete = arg3 as CompleteFn; return this.eventStream.subscribe( diff --git a/src/document/operation/operation.ts b/src/document/operation/operation.ts index e117397b9..dbe691a0b 100644 --- a/src/document/operation/operation.ts +++ b/src/document/operation/operation.ts @@ -149,7 +149,7 @@ export type TreeEditOpInfo = { path: string; from: number; to: number; - value: TreeNode; + value?: Array; splitLevel: number; fromPath: Array; toPath: Array; diff --git a/test/integration/client_test.ts b/test/integration/client_test.ts index bc2a78979..d87bf4008 100644 --- a/test/integration/client_test.ts +++ b/test/integration/client_test.ts @@ -140,7 +140,7 @@ describe.sequential('Client', function () { const unsub1 = { syncEvent: d1.subscribe('sync', (event) => { - eventCollectorSync1.add(event.value as DocumentSyncStatus); + eventCollectorSync1.add(event.value); }), doc: d1.subscribe((event) => { eventCollectorD1.add(event.type); @@ -148,7 +148,7 @@ describe.sequential('Client', function () { }; const unsub2 = { syncEvent: d2.subscribe('sync', (event) => { - eventCollectorSync2.add(event.value as DocumentSyncStatus); + eventCollectorSync2.add(event.value); }), doc: d2.subscribe((event) => { eventCollectorD2.add(event.type); @@ -236,7 +236,7 @@ describe.sequential('Client', function () { // 02. c2 changes the sync mode to realtime sync mode. const eventCollector = new EventCollector(); const unsub1 = d2.subscribe('sync', (event) => { - eventCollector.add(event.value as DocumentSyncStatus); + eventCollector.add(event.value); }); await c2.changeSyncMode(d2, SyncMode.Realtime); await eventCollector.waitFor(DocumentSyncStatus.Synced); // sync occurs when resuming @@ -414,7 +414,7 @@ describe.sequential('Client', function () { const eventCollector = new EventCollector(); const unsub1 = d2.subscribe('sync', (event) => { - eventCollector.add(event.value as DocumentSyncStatus); + eventCollector.add(event.value); }); // 01. c2 attach the doc with realtime sync mode at first. @@ -491,7 +491,7 @@ describe.sequential('Client', function () { // and sync with push-only mode: CP(2, 2) -> CP(3, 2) const eventCollector = new EventCollector(); const unsub = d1.subscribe('sync', (event) => { - eventCollector.add(event.value as DocumentSyncStatus); + eventCollector.add(event.value); }); d1.update((root) => { root.counter.increase(1); @@ -569,7 +569,7 @@ describe.sequential('Client', function () { assert.equal(d1.getRoot().tree.toXML(), '

12

34

'); assert.equal(d2.getRoot().tree.toXML(), '

12

34

'); - d1.update((root: any) => { + d1.update((root) => { root.tree.edit(2, 2, { type: 'text', value: 'a' }); }); await c1.sync(); @@ -595,7 +595,7 @@ describe.sequential('Client', function () { c2.changeSyncMode(d2, SyncMode.Realtime); - d2.update((root: any) => { + d2.update((root) => { root.tree.edit(2, 2, { type: 'text', value: 'b' }); }); await eventCollectorD1.waitAndVerifyNthEvent(3, DocEventType.RemoteChange); diff --git a/test/integration/document_test.ts b/test/integration/document_test.ts index baaf41e76..aa11ebf84 100644 --- a/test/integration/document_test.ts +++ b/test/integration/document_test.ts @@ -179,11 +179,16 @@ describe('Document', function () { let expectedEventValue: Array; const eventCollectorD1 = new EventCollector(); const eventCollectorD2 = new EventCollector(); - // TODO(chacha912): Remove any type after specifying the type of DocEvent - const unsub1 = d1.subscribe((event: any) => { + const unsub1 = d1.subscribe((event) => { + if (event.type === DocEventType.Snapshot) { + return; + } eventCollectorD1.add({ type: event.type, value: event.value.operations }); }); - const unsub2 = d2.subscribe((event: any) => { + const unsub2 = d2.subscribe((event) => { + if (event.type === DocEventType.Snapshot) { + return; + } eventCollectorD2.add({ type: event.type, value: event.value.operations }); }); @@ -297,13 +302,16 @@ describe('Document', function () { const eventCollector = new EventCollector(); const eventCollectorForTodos = new EventCollector(); const eventCollectorForCounter = new EventCollector(); - const unsub = d1.subscribe((event: any) => { + const unsub = d1.subscribe((event) => { + if (event.type === DocEventType.Snapshot) { + return; + } eventCollector.add(event.value.operations); }); - const unsubTodo = d1.subscribe('$.todos', (event: any) => { + const unsubTodo = d1.subscribe('$.todos', (event) => { eventCollectorForTodos.add(event.value.operations); }); - const unsubCounter = d1.subscribe('$.counter', (event: any) => { + const unsubCounter = d1.subscribe('$.counter', (event) => { eventCollectorForCounter.add(event.value.operations); }); @@ -384,13 +392,16 @@ describe('Document', function () { const eventCollector = new EventCollector(); const eventCollectorForTodos0 = new EventCollector(); const eventCollectorForObjC1 = new EventCollector(); - const unsub = d1.subscribe((event: any) => { + const unsub = d1.subscribe((event) => { + if (event.type === DocEventType.Snapshot) { + return; + } eventCollector.add(event.value.operations); }); - const unsubTodo = d1.subscribe('$.todos.0', (event: any) => { + const unsubTodo = d1.subscribe('$.todos.0', (event) => { eventCollectorForTodos0.add(event.value.operations); }); - const unsubObj = d1.subscribe('$.obj.c1', (event: any) => { + const unsubObj = d1.subscribe('$.obj.c1', (event) => { eventCollectorForObjC1.add(event.value.operations); }); diff --git a/test/integration/tree_test.ts b/test/integration/tree_test.ts index fc5653db9..42b75361f 100644 --- a/test/integration/tree_test.ts +++ b/test/integration/tree_test.ts @@ -237,7 +237,7 @@ describe('Tree', () => { from: 1, to: 1, value: [{ type: 'text', value: 'X' }], - } as any, + }, ], ); }); @@ -309,7 +309,7 @@ describe('Tree', () => { fromPath: [0, 0, 0, 1], toPath: [0, 0, 0, 1], value: [{ type: 'text', value: 'X' }], - } as any, + }, ], ); }); @@ -4322,7 +4322,7 @@ describe('TreeChange', () => { from: 0, to: 4, value: undefined, - } as any, + }, ], ); @@ -4341,13 +4341,13 @@ describe('TreeChange', () => { from: 1, to: 2, value: undefined, - } as any, + }, { type: 'tree-edit', from: 0, to: 3, value: undefined, - } as any, + }, ], ); }, task.name); @@ -4393,13 +4393,13 @@ describe('TreeChange', () => { from: 1, to: 3, value: undefined, - } as any, + }, { type: 'tree-edit', from: 1, to: 1, value: [{ type: 'text', value: 'c' }], - } as any, + }, ], ); @@ -4418,19 +4418,19 @@ describe('TreeChange', () => { from: 2, to: 2, value: [{ type: 'text', value: 'c' }], - } as any, + }, { type: 'tree-edit', from: 3, to: 4, value: undefined, - } as any, + }, { type: 'tree-edit', from: 1, to: 2, value: undefined, - } as any, + }, ], ); }, task.name); @@ -4478,7 +4478,7 @@ describe('TreeChange', () => { from: 0, to: 4, value: undefined, - } as any, + }, ], ); @@ -4497,13 +4497,13 @@ describe('TreeChange', () => { from: 2, to: 2, value: [{ type: 'text', value: 'c' }], - } as any, + }, { type: 'tree-edit', from: 0, to: 5, value: undefined, - } as any, + }, ], ); }, task.name); @@ -4549,13 +4549,13 @@ describe('TreeChange', () => { from: 1, to: 2, value: [{ type: 'text', value: 'b' }], - } as any, + }, { type: 'tree-edit', from: 2, to: 2, value: [{ type: 'text', value: 'c' }], - } as any, + }, ], ); @@ -4574,13 +4574,13 @@ describe('TreeChange', () => { from: 2, to: 2, value: [{ type: 'text', value: 'c' }], - } as any, + }, { type: 'tree-edit', from: 1, to: 2, value: [{ type: 'text', value: 'b' }], - } as any, + }, ], ); }, task.name); @@ -4628,13 +4628,13 @@ describe('TreeChange', () => { from: 0, to: 1, value: { value: 'changed' }, - } as any, + }, { type: 'tree-edit', from: 0, to: 2, value: undefined, - } as any, + }, ], ); @@ -4648,7 +4648,7 @@ describe('TreeChange', () => { from: 0, to: 2, value: undefined, - } as any, + }, ], ); }, task.name); diff --git a/test/unit/document/document_test.ts b/test/unit/document/document_test.ts index d9c0eea5e..d6efd2c70 100644 --- a/test/unit/document/document_test.ts +++ b/test/unit/document/document_test.ts @@ -18,7 +18,7 @@ import { describe, it, assert, vi, afterEach } from 'vitest'; import { EventCollector } from '@yorkie-js-sdk/test/helper/helper'; import { MaxTimeTicket } from '@yorkie-js-sdk/src/document/time/ticket'; -import { Document } from '@yorkie-js-sdk/src/document/document'; +import { Document, DocEventType } from '@yorkie-js-sdk/src/document/document'; import { OperationInfo } from '@yorkie-js-sdk/src/document/operation/operation'; import { JSONArray, Text, Counter, Tree } from '@yorkie-js-sdk/src/yorkie'; import { CounterType } from '@yorkie-js-sdk/src/document/crdt/counter'; @@ -967,8 +967,10 @@ describe.sequential('Document', function () { type EventForTest = Array; const eventCollector = new EventCollector(); - // TODO(chacha912): Remove any type after specifying the type of DocEvent - const unsub = doc.subscribe((event: any) => { + const unsub = doc.subscribe((event) => { + if (event.type === DocEventType.Snapshot) { + return; + } eventCollector.add(event.value.operations); }); @@ -1010,7 +1012,10 @@ describe.sequential('Document', function () { const doc = new Document('test-doc'); type EventForTest = Array; const eventCollector = new EventCollector(); - const unsub = doc.subscribe((event: any) => { + const unsub = doc.subscribe((event) => { + if (event.type === DocEventType.Snapshot) { + return; + } eventCollector.add(event.value.operations); }); @@ -1040,7 +1045,10 @@ describe.sequential('Document', function () { const doc = new Document('test-doc'); type EventForTest = Array; const eventCollector = new EventCollector(); - const unsub = doc.subscribe((event: any) => { + const unsub = doc.subscribe((event) => { + if (event.type === DocEventType.Snapshot) { + return; + } eventCollector.add(event.value.operations); }); @@ -1091,7 +1099,10 @@ describe.sequential('Document', function () { const doc = new Document('test-doc'); type EventForTest = Array; const eventCollector = new EventCollector(); - const unsub = doc.subscribe((event: any) => { + const unsub = doc.subscribe((event) => { + if (event.type === DocEventType.Snapshot) { + return; + } eventCollector.add(event.value.operations); }); @@ -1119,7 +1130,10 @@ describe.sequential('Document', function () { const doc = new Document('test-doc'); type EventForTest = Array; const eventCollector = new EventCollector(); - const unsub = doc.subscribe((event: any) => { + const unsub = doc.subscribe((event) => { + if (event.type === DocEventType.Snapshot) { + return; + } eventCollector.add(event.value.operations); });