TypeScript Error in Devtools: TransactionEvent

[gwbaik9717]

Description:

There is a type-related error in the existing code within the tools/devtools/src/devtools/tabs/History.tsx directory related to TransactionEvent. The TypeScript error message is as follows:

Property 'source' does not exist on type 'DocEvent<Indexable, OperationInfo>'.

Why:

This change will help prevent potential bugs, ensure type safety, and improve overall code quality.

[KimKyuHoi]

@devleejb Could I give it a try? i want to challenge it

[KimKyuHoi]

@gwbaik9717
The part I am concerned about is when I look at the current export type of DocEvent,

export type DocEvent<P extends Indexable = Indexable, T = OperationInfo> =
  | StatusChangedEvent
  | ConnectionChangedEvent
  | SyncStatusChangedEvent
  | SnapshotEvent
  | LocalChangeEvent<T, P>
  | RemoteChangeEvent<T, P>
  | InitializedEvent<P>
  | WatchedEvent<P>
  | UnwatchedEvent<P>
  | PresenceChangedEvent<P>;

Type 'DocEvent<Indexable, OperationInfo>' does not have property 'source' because within the DocEvent type, ConnectionChangeEvent and SyncStatusChangedEvent do not have a source property. It seems like this error occurs.

Is it correct to add a source to ConnectionChangeEvent and SyncStatusChangedEvent, or is it correct to use the type guard function in the History.tsx file to check whether it is an event type with a source property and then access it?

for example

function hasSource(event: DocEvent): event is StatusChangedEvent | LocalChangeEvent | RemoteChangeEvent {
  return 'source' in event;
}

After writing this

   const firstEvent = event.event[0];

    let source: string;
    if (hasSource(firstEvent)) {
      source = firstEvent.source;
    } else {
      source = 'unknown'; 
    }

Is it correct to display it like this?

[devleejb]

@chacha912
Is there any progress?

[chacha912]

@devleejb I tried to make this work, but I got stuck because it needed too many changes in the devtools types and other areas.

Thank you, @KimKyuHoi , for your interest.

I think we could fix this by following these steps:

1. First, we could add source to the BaseDocEvent type. This would make sure all DocEvents have a source property. Then, we could add the source property to ConnectionChangeEvent and SyncStatusChangedEvent. This should solve the problem for now.

2. In the future, we could make it even better like this: The main issue is that when we set up devtools, we only keep the events that devtools needs. Right now, devtools uses events that have information to rebuild the document.

yorkie-js-sdk/src/devtools/index.ts

Lines 84 to 98 in 3e6d0b2

	if (
	event.some(
	(docEvent) =>
	docEvent.type !== DocEventType.StatusChanged &&
	docEvent.type !== DocEventType.Snapshot &&
	docEvent.type !== DocEventType.LocalChange &&
	docEvent.type !== DocEventType.RemoteChange &&
	docEvent.type !== DocEventType.Initialized &&
	docEvent.type !== DocEventType.Watched &&
	docEvent.type !== DocEventType.Unwatched &&
	docEvent.type !== DocEventType.PresenceChanged,
	)
	) {
	return;
	}

We could create a new type for these specific events:

 /**
  * `EventsForDocRebuild` is a list of events used to rebuild a document.
  */
 export type EventsForDocRebuild<
   P extends Indexable = Indexable,
   T = OperationInfo,
 > = Array<
   | StatusChangedEvent
   | SnapshotEvent
   | LocalChangeEvent<T, P>
   | RemoteChangeEvent<T, P>
   | InitializedEvent<P>
   | WatchedEvent<P>
   | UnwatchedEvent<P>
   | PresenceChangedEvent<P>
 >;

 /**
  * `isEventsForDocRebuild` checks if an event can be used to rebuild a document.
  */
 export function isEventsForDocRebuild(
   event: TransactionEvent,
 ): event is EventsForDocRebuild {
   const typesForDocRebuild = [
     DocEventType.StatusChanged,
     DocEventType.Snapshot,
     DocEventType.LocalChange,
     DocEventType.RemoteChange,
     DocEventType.Initialized,
     DocEventType.Watched,
     DocEventType.Unwatched,
     DocEventType.PresenceChanged,
   ];

   return event.every((docEvent) =>
     typesForDocRebuild.includes(docEvent.type),
   );
 }

This way, we can be more sure that we're using the right events for rebuilding documents, and it's easier for TypeScript to check if we're doing it correctly.
I attempted the second method, but I encountered issues with the type declaration location and importing into devtools after building, which blocked my progress. For now, I think it would be fine to proceed with the first method.

[KimKyuHoi]

@chacha912
Certainly, when looking at future methods, the second method is clear. However, I think that if you choose the second method, you will have to go around a lot. Then, I'll try the first method.