From 8cb76ce40bce8e2e6d7175bb2e79ac7c7651ce99 Mon Sep 17 00:00:00 2001 From: Alf Eaton Date: Tue, 5 Dec 2023 09:42:01 +0000 Subject: [PATCH] [ide-react] Address some migration TODOs (#16033) * Add type for externalUpdate * Log clientTracking.getConnectedUsers error * Remove update debugging code * Use ErrorMetadata type * Use Message type * Remove unused document:opened event GitOrigin-RevId: 3a1d1e785dca37d6b91cd650fbcb4e5decb6343b --- .../features/history/services/types/shared.ts | 2 ++ .../context/editor-manager-context.tsx | 9 +++---- .../context/online-users-context.tsx | 8 ++++-- .../ide-react/create-ide-event-emitter.ts | 5 ---- .../js/features/ide-react/editor/document.ts | 25 ++++++++++++++----- .../features/ide-react/editor/share-js-doc.ts | 14 ----------- .../ide-react/editor/types/document.ts | 21 ++++++++-------- 7 files changed, 41 insertions(+), 43 deletions(-) diff --git a/services/web/frontend/js/features/history/services/types/shared.ts b/services/web/frontend/js/features/history/services/types/shared.ts index e9951cceca..47785cd139 100644 --- a/services/web/frontend/js/features/history/services/types/shared.ts +++ b/services/web/frontend/js/features/history/services/types/shared.ts @@ -11,6 +11,8 @@ export interface Meta { users: Nullable[] start_ts: number end_ts: number + type?: 'external' // TODO + source?: 'git-bridge' // TODO origin?: { kind: | 'dropbox' diff --git a/services/web/frontend/js/features/ide-react/context/editor-manager-context.tsx b/services/web/frontend/js/features/ide-react/context/editor-manager-context.tsx index 3c9921a6cc..c2b96473e4 100644 --- a/services/web/frontend/js/features/ide-react/context/editor-manager-context.tsx +++ b/services/web/frontend/js/features/ide-react/context/editor-manager-context.tsx @@ -19,7 +19,6 @@ import { debugConsole } from '@/utils/debugging' import { Document } from '@/features/ide-react/editor/document' import { useLayoutContext } from '@/shared/context/layout-context' import { GotoLineOptions } from '@/features/ide-react/types/goto-line-options' -import _ from 'lodash' import { Doc } from '../../../../../types/doc' import { useFileTreeData } from '@/shared/context/file-tree-data-context' import { findDocEntityById } from '@/features/ide-react/util/find-doc-entity-by-id' @@ -30,6 +29,7 @@ import customLocalStorage from '@/infrastructure/local-storage' import useEventListener from '@/shared/hooks/use-event-listener' import { EditorType } from '@/features/ide-react/editor/types/editor-type' import { DocId } from '../../../../../types/project-settings' +import { Update } from '@/features/history/services/types/update' interface GotoOffsetOptions { gotoOffset: number @@ -237,14 +237,13 @@ export const EditorManagerProvider: FC = ({ children }) => { (doc: Doc, document: Document) => { attachErrorHandlerToDocument(doc, document) - // TODO: MIGRATION: Add a type for `update` - document.on('externalUpdate', (update: any) => { + document.on('externalUpdate', (update: Update) => { if (ignoringExternalUpdates) { return } if ( - _.property(['meta', 'type'])(update) === 'external' && - _.property(['meta', 'source'])(update) === 'git-bridge' + update.meta.type === 'external' && + update.meta.source === 'git-bridge' ) { return } diff --git a/services/web/frontend/js/features/ide-react/context/online-users-context.tsx b/services/web/frontend/js/features/ide-react/context/online-users-context.tsx index afa0107737..c3ff903691 100644 --- a/services/web/frontend/js/features/ide-react/context/online-users-context.tsx +++ b/services/web/frontend/js/features/ide-react/context/online-users-context.tsx @@ -18,6 +18,7 @@ import { Doc } from '../../../../../types/doc' import { useFileTreeData } from '@/shared/context/file-tree-data-context' import { findDocEntityById } from '@/features/ide-react/util/find-doc-entity-by-id' import useSocketListener from '@/features/ide-react/hooks/use-socket-listener' +import { debugConsole } from '@/utils/debugging' type OnlineUser = { id: string @@ -176,9 +177,12 @@ export const OnlineUsersProvider: FC = ({ children }) => { const handleProjectJoined = () => { socket.emit( 'clientTracking.getConnectedUsers', - // eslint-disable-next-line n/handle-callback-err (error: Error, connectedUsers: ConnectedUser[]) => { - // TODO: MIGRATION: Handle error (although the original code doesn't) + if (error) { + // TODO: handle this error or ignore it? + debugConsole.error(error) + return + } const newOnlineUsers: OnlineUsersContextValue['onlineUsers'] = {} for (const user of connectedUsers) { if (user.client_id === socket.publicId) { diff --git a/services/web/frontend/js/features/ide-react/create-ide-event-emitter.ts b/services/web/frontend/js/features/ide-react/create-ide-event-emitter.ts index 8959e9f0dc..94127b238e 100644 --- a/services/web/frontend/js/features/ide-react/create-ide-event-emitter.ts +++ b/services/web/frontend/js/features/ide-react/create-ide-event-emitter.ts @@ -8,10 +8,6 @@ import { FileTreeFindResult } from '@/features/ide-react/types/file-tree' export type IdeEvents = { 'project:joined': [{ project: Project; permissionsLevel: PermissionsLevel }] - - // TODO: MIGRATION: This doesn't seem to be used. Investigate whether it can be removed - 'document:opened': [doc: ShareJsDoc] - 'document:closed': [doc: ShareJsDoc] 'doc:changed': [{ doc_id: string }] 'doc:saved': [{ doc_id: string }] @@ -27,7 +23,6 @@ export type IdeEvents = { 'comment:start_adding': [] 'references:should-reindex': [] 'history:toggle': [] - 'entity:deleted': [entity: FileTreeFindResult] } diff --git a/services/web/frontend/js/features/ide-react/editor/document.ts b/services/web/frontend/js/features/ide-react/editor/document.ts index 3786884b60..76d1e49e43 100644 --- a/services/web/frontend/js/features/ide-react/editor/document.ts +++ b/services/web/frontend/js/features/ide-react/editor/document.ts @@ -37,7 +37,24 @@ const MAX_PENDING_OP_SIZE = 64 type JoinCallback = (error?: Error) => void type LeaveCallback = JoinCallback -type Update = Record +type Update = + | { + v: number + doc: string + } + | { + v: number + doc: string + op: AnyOperation[] + meta: { + type?: string + source: string + user_id: string + ts: number + } + hash?: string + lastV?: number + } type Message = { meta: { @@ -101,9 +118,6 @@ export class Document extends EventEmitter { if (this.cm6) { this.cm6.on('change', this.checkConsistency) } - if (this.doc) { - this.ideEventEmitter.emit('document:opened', this.doc) - } } detachFromCM6() { @@ -183,8 +197,7 @@ export class Document extends EventEmitter { private onUpdateAppliedHandler = (update: any) => this.onUpdateApplied(update) - // TODO: MIGRATION: Create proper types for error and message - private onErrorHandler = (error: Error, message: { doc_id: string }) => { + private onErrorHandler = (error: Error, message: ErrorMetadata) => { // 'otUpdateError' are emitted per doc socket.io room, hence we can be // sure that message.doc_id exists. if (message.doc_id !== this.doc_id) { diff --git a/services/web/frontend/js/features/ide-react/editor/share-js-doc.ts b/services/web/frontend/js/features/ide-react/editor/share-js-doc.ts index 92aea65653..eb819c1cbd 100644 --- a/services/web/frontend/js/features/ide-react/editor/share-js-doc.ts +++ b/services/web/frontend/js/features/ide-react/editor/share-js-doc.ts @@ -65,20 +65,6 @@ export class ShareJsDoc extends EventEmitter { this.connection = { send: (update: Update) => { this.startInflightOpTimeout(update) - // TODO: MIGRATION: Work out whether we can get rid of this. It looks as - // though it's here for debugging and isn't used - - // if ( - // window.disconnectOnUpdate != null && - // Math.random() < window.disconnectOnUpdate - // ) { - // debugConsole.log('Disconnecting on update', update) - // this.socket.disconnect() - // } - // if (window.dropUpdates != null && Math.random() < window.dropUpdates) { - // debugConsole.log('Simulating a lost update', update) - // return - // } if (this.track_changes && this.track_changes_id_seeds) { if (update.meta == null) { update.meta = {} diff --git a/services/web/frontend/js/features/ide-react/editor/types/document.ts b/services/web/frontend/js/features/ide-react/editor/types/document.ts index 744d8f1b5b..44d36c0e48 100644 --- a/services/web/frontend/js/features/ide-react/editor/types/document.ts +++ b/services/web/frontend/js/features/ide-react/editor/types/document.ts @@ -8,14 +8,13 @@ export type ShareJsOperation = AnyOperation[] export type TrackChangesIdSeeds = { inflight: string; pending: string } -export type Message = Record -// TODO: MIGRATION: Make an accurate and more specific type -// { -// v: Version -// open?: boolean -// meta?: { -// type: string -// } -// doc?: string -// snapshot?: string -// } +// TODO: check the properties of this type +export type Message = { + v: Version + open?: boolean + meta?: { + type?: string + } + doc?: string + snapshot?: string +}