From 588da0443c695801acbe50c1556eef9689d48d0c Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 6 Feb 2019 11:53:23 -0600 Subject: [PATCH] Some cleanup - Use whateverEmitter.event for the onWhatever methods. - Add readonly to a bunch of things. - Remove some redundancy in types. - Move initializations out of the constructor and into the declarations where it was reasonable to do so. - Disable a few no-any violations. --- packages/ide/src/client.ts | 32 +++------------- packages/ide/src/fill/client.ts | 42 +++++++-------------- packages/ide/src/fill/clipboard.ts | 12 +----- packages/ide/src/fill/dialog.ts | 19 ++-------- packages/ide/src/fill/os.ts | 4 +- packages/ide/src/retry.ts | 12 +++--- packages/ide/src/upload.ts | 23 ++++------- packages/logger/src/logger.ts | 11 ++---- packages/protocol/src/browser/client.ts | 37 +++++++++--------- packages/protocol/src/node/server.ts | 10 ++--- packages/server/src/ipc.ts | 12 +++++- packages/server/src/vscode/sharedProcess.ts | 8 +--- packages/vscode/src/fill/node-pty.ts | 6 +-- packages/vscode/src/fill/stdioElectron.ts | 6 ++- packages/vscode/src/fill/storageDatabase.ts | 2 +- packages/vscode/src/fill/windowsService.ts | 26 ++++++------- 16 files changed, 98 insertions(+), 164 deletions(-) diff --git a/packages/ide/src/client.ts b/packages/ide/src/client.ts index ce600c713..c1cb3d74f 100644 --- a/packages/ide/src/client.ts +++ b/packages/ide/src/client.ts @@ -1,6 +1,5 @@ -import { Event } from "@coder/events"; import { field, logger, time, Time } from "@coder/logger"; -import { InitData, ISharedProcessData } from "@coder/protocol"; +import { ISharedProcessData } from "@coder/protocol"; import { retry } from "./retry"; import { upload } from "./upload"; import { client } from "./fill/client"; @@ -24,14 +23,16 @@ export abstract class IdeClient { private readonly tasks = []; private finishedTaskCount = 0; private readonly loadTime: Time; - private readonly sharedProcessDataPromise: Promise; + + public readonly initData = client.initData; + public readonly sharedProcessData: Promise; + public readonly onSharedProcessActive = client.onSharedProcessActive; public constructor() { logger.info("Loading IDE"); - this.loadTime = time(2500); - this.sharedProcessDataPromise = new Promise((resolve): void => { + this.sharedProcessData = new Promise((resolve): void => { client.onSharedProcessActive(resolve); }); @@ -96,27 +97,6 @@ export abstract class IdeClient { } } - /** - * A promise that resolves with initialization data. - */ - public get initData(): Promise { - return client.initData; - } - - /** - * An event that fires every time the shared process (re-)starts. - */ - public get onSharedProcessActive(): Event { - return client.onSharedProcessActive; - } - - /** - * A promise that resolves with *initial* shared process data. - */ - public get sharedProcessData(): Promise { - return this.sharedProcessDataPromise; - } - public set notificationService(service: INotificationService) { this.retry.notificationService = service; this.upload.notificationService = service; diff --git a/packages/ide/src/fill/client.ts b/packages/ide/src/fill/client.ts index 1ae033924..5286225fa 100644 --- a/packages/ide/src/fill/client.ts +++ b/packages/ide/src/fill/client.ts @@ -9,36 +9,28 @@ import { retry } from "../retry"; */ class Connection implements ReadWriteConnection { private activeSocket: WebSocket | undefined; - private readonly messageEmitter: Emitter = new Emitter(); - private readonly closeEmitter: Emitter = new Emitter(); - private readonly upEmitter: Emitter = new Emitter(); - private readonly downEmitter: Emitter = new Emitter(); - private readonly messageBuffer: Uint8Array[] = []; - private socketTimeoutDelay = 60 * 1000; - private retryName = "Socket"; + private readonly messageBuffer = []; + private readonly socketTimeoutDelay = 60 * 1000; + private readonly retryName = "Socket"; private isUp: boolean = false; private closed: boolean = false; + private readonly messageEmitter = new Emitter(); + private readonly closeEmitter = new Emitter(); + private readonly upEmitter = new Emitter(); + private readonly downEmitter = new Emitter(); + + public readonly onUp = this.upEmitter.event; + public readonly onClose = this.closeEmitter.event; + public readonly onDown = this.downEmitter.event; + public readonly onMessage = this.messageEmitter.event; + public constructor() { retry.register(this.retryName, () => this.connect()); retry.block(this.retryName); retry.run(this.retryName); } - /** - * Register a function to be called when the connection goes up. - */ - public onUp(cb: () => void): void { - this.upEmitter.event(cb); - } - - /** - * Register a function to be called when the connection goes down. - */ - public onDown(cb: () => void): void { - this.downEmitter.event(cb); - } - public send(data: Buffer | Uint8Array): void { if (this.closed) { throw new Error("web socket is closed"); @@ -50,14 +42,6 @@ class Connection implements ReadWriteConnection { } } - public onMessage(cb: (data: Uint8Array | Buffer) => void): void { - this.messageEmitter.event(cb); - } - - public onClose(cb: () => void): void { - this.closeEmitter.event(cb); - } - public close(): void { this.closed = true; this.dispose(); diff --git a/packages/ide/src/fill/clipboard.ts b/packages/ide/src/fill/clipboard.ts index a26be853f..502d1b90e 100644 --- a/packages/ide/src/fill/clipboard.ts +++ b/packages/ide/src/fill/clipboard.ts @@ -1,11 +1,11 @@ -import { IDisposable } from "@coder/disposable"; import { Emitter } from "@coder/events"; /** * Wrapper around the native clipboard with some fallbacks. */ export class Clipboard { - private readonly enableEmitter: Emitter = new Emitter(); + private readonly enableEmitter = new Emitter(); + public readonly onPermissionChange = this.enableEmitter.event; private _isEnabled: boolean = false; /** @@ -70,14 +70,6 @@ export class Clipboard { // tslint:enable no-any } - /** - * Register a function to be called when the native clipboard is - * enabled/disabled. - */ - public onPermissionChange(cb: (enabled: boolean) => void): IDisposable { - return this.enableEmitter.event(cb); - } - /** * Read text from the clipboard. */ diff --git a/packages/ide/src/fill/dialog.ts b/packages/ide/src/fill/dialog.ts index 3389a863f..215d5e137 100644 --- a/packages/ide/src/fill/dialog.ts +++ b/packages/ide/src/fill/dialog.ts @@ -1,4 +1,3 @@ -import { IDisposable } from "@coder/disposable"; import { Emitter } from "@coder/events"; import "./dialog.scss"; @@ -27,19 +26,16 @@ export enum IKey { } export class Dialog { - private options: IDialogOptions; - private overlay: HTMLElement; + private readonly overlay: HTMLElement; private cachedActiveElement: HTMLElement | undefined; private input: HTMLInputElement | undefined; - private actionEmitter: Emitter; private errors: HTMLElement; private buttons: HTMLElement[] | undefined; - public constructor(options: IDialogOptions) { - this.options = options; - - this.actionEmitter = new Emitter(); + private actionEmitter = new Emitter(); + public onAction = this.actionEmitter.event; + public constructor(private readonly options: IDialogOptions) { const msgBox = document.createElement("div"); msgBox.classList.add("msgbox"); @@ -108,13 +104,6 @@ export class Dialog { }); } - /** - * Register a function to be called when the user performs an action. - */ - public onAction(callback: (action: IDialogAction) => void): IDisposable { - return this.actionEmitter.event(callback); - } - /** * Input value if this dialog has an input. */ diff --git a/packages/ide/src/fill/os.ts b/packages/ide/src/fill/os.ts index a59338466..57ee62763 100644 --- a/packages/ide/src/fill/os.ts +++ b/packages/ide/src/fill/os.ts @@ -13,7 +13,7 @@ class OS { public homedir(): string { if (typeof this._homedir === "undefined") { - throw new Error("not initialized"); + throw new Error("trying to access homedir before it has been set"); } return this._homedir; @@ -21,7 +21,7 @@ class OS { public tmpdir(): string { if (typeof this._tmpdir === "undefined") { - throw new Error("not initialized"); + throw new Error("trying to access tmpdir before it has been set"); } return this._tmpdir; diff --git a/packages/ide/src/retry.ts b/packages/ide/src/retry.ts index e5703764b..2902c0c41 100644 --- a/packages/ide/src/retry.ts +++ b/packages/ide/src/retry.ts @@ -21,7 +21,7 @@ interface IRetryItem { * to the user explaining what is happening with an option to immediately retry. */ export class Retry { - private items: Map; + private items = new Map(); // Times are in seconds. private readonly retryMinDelay = 1; @@ -31,17 +31,15 @@ export class Retry { private blocked: string | boolean | undefined; private notificationHandle: INotificationHandle | undefined; - private updateDelay = 1; + private readonly updateDelay = 1; private updateTimeout: number | NodeJS.Timer | undefined; - private notificationThreshold = 3; + private readonly notificationThreshold = 3; // Time in milliseconds to wait before restarting a service. (See usage below // for reasoning.) - private waitDelay = 50; + private readonly waitDelay = 50; - public constructor(private _notificationService: INotificationService) { - this.items = new Map(); - } + public constructor(private _notificationService: INotificationService) {} public set notificationService(service: INotificationService) { this._notificationService = service; diff --git a/packages/ide/src/upload.ts b/packages/ide/src/upload.ts index 03305f6ff..b088d17bc 100644 --- a/packages/ide/src/upload.ts +++ b/packages/ide/src/upload.ts @@ -1,7 +1,7 @@ import { exec } from "child_process"; import { appendFile } from "fs"; import { promisify } from "util"; -import { logger, Logger } from "@coder/logger"; +import { logger } from "@coder/logger"; import { escapePath } from "@coder/protocol"; import { NotificationService, INotificationService, ProgressService, IProgressService, IProgress, Severity } from "./fill/notification"; @@ -40,27 +40,20 @@ export class Upload { private readonly maxParallelUploads = 100; private readonly readSize = 32000; // ~32kb max while reading in the file. private readonly packetSize = 32000; // ~32kb max when writing. - private readonly logger: Logger; - private readonly currentlyUploadingFiles: Map; - private readonly queueByDirectory: Map; + private readonly logger = logger.named("Upload"); + private readonly currentlyUploadingFiles = new Map(); + private readonly queueByDirectory = new Map(); private progress: IProgress | undefined; private uploadPromise: Promise | undefined; private resolveUploadPromise: (() => void) | undefined; - private finished: number; - private uploadedFilePaths: string[]; - private total: number; + private finished = 0; + private uploadedFilePaths = []; + private total = 0; public constructor( private _notificationService: INotificationService, private _progressService: IProgressService, - ) { - this.logger = logger.named("Upload"); - this.currentlyUploadingFiles = new Map(); - this.queueByDirectory = new Map(); - this.uploadedFilePaths = []; - this.finished = 0; - this.total = 0; - } + ) {} public set notificationService(service: INotificationService) { this._notificationService = service; diff --git a/packages/logger/src/logger.ts b/packages/logger/src/logger.ts index 360fb510d..f56e62904 100644 --- a/packages/logger/src/logger.ts +++ b/packages/logger/src/logger.ts @@ -113,8 +113,8 @@ const hashStringToColor = (str: string): string => { * currently built items and appends to that. */ export abstract class Formatter { - protected format: string = ""; - protected args: string[] = []; + protected format = ""; + protected args = []; /** * Add a tag. @@ -250,14 +250,13 @@ export class Logger { public level = Level.Info; private readonly nameColor?: string; - private muted: boolean; + private muted: boolean = false; public constructor( private _formatter: Formatter, private readonly name?: string, private readonly defaultFields?: FieldArray, ) { - this.muted = false; if (name) { this.nameColor = hashStringToColor(name); } @@ -388,10 +387,6 @@ export class Logger { times = fields.filter((f) => f.value instanceof Time); } - // Format is: - // [TAG] [NAME?] MESSAGE TIME? - // field1 (type)?: value - // field2 (type)?: value this._formatter.tag(options.type.toUpperCase(), options.tagColor); if (this.name && this.nameColor) { this._formatter.tag(this.name.toUpperCase(), this.nameColor); diff --git a/packages/protocol/src/browser/client.ts b/packages/protocol/src/browser/client.ts index 0d9df4e3e..43ccc1f8c 100644 --- a/packages/protocol/src/browser/client.ts +++ b/packages/protocol/src/browser/client.ts @@ -9,27 +9,28 @@ import { EventEmitter } from "events"; * Client accepts an arbitrary connection intended to communicate with the Server. */ export class Client { - public Socket: typeof ServerSocket; + public readonly Socket: typeof ServerSocket; - private evalId: number = 0; - private evalDoneEmitter: Emitter = new Emitter(); - private evalFailedEmitter: Emitter = new Emitter(); - private evalEventEmitter: Emitter = new Emitter(); + private evalId = 0; + private readonly evalDoneEmitter = new Emitter(); + private readonly evalFailedEmitter = new Emitter(); + private readonly evalEventEmitter = new Emitter(); - private sessionId: number = 0; - private readonly sessions: Map = new Map(); + private sessionId = 0; + private readonly sessions = new Map(); - private connectionId: number = 0; - private readonly connections: Map = new Map(); + private connectionId = 0; + private readonly connections = new Map(); - private serverId: number = 0; - private readonly servers: Map = new Map(); + private serverId = 0; + private readonly servers = new Map(); private _initData: InitData | undefined; - private initDataEmitter = new Emitter(); - private initDataPromise: Promise; + private readonly initDataEmitter = new Emitter(); + private readonly initDataPromise: Promise; - private sharedProcessActiveEmitter = new Emitter(); + private readonly sharedProcessActiveEmitter = new Emitter(); + public readonly onSharedProcessActive = this.sharedProcessActiveEmitter.event; /** * @param connection Established connection to the server @@ -63,10 +64,6 @@ export class Client { return this.initDataPromise; } - public get onSharedProcessActive(): Event { - return this.sharedProcessActiveEmitter.event; - } - public run(func: (ae: ActiveEval) => void | Promise): ActiveEval; public run(func: (ae: ActiveEval, a1: T1) => void | Promise, a1: T1): ActiveEval; public run(func: (ae: ActiveEval, a1: T1, a2: T2) => void | Promise, a1: T1, a2: T2): ActiveEval; @@ -95,8 +92,8 @@ export class Client { }); return { - on: (event: string, cb: (...args: any[]) => void) => eventEmitter.on(event, cb), - emit: (event: string, ...args: any[]) => { + on: (event: string, cb: (...args: any[]) => void): EventEmitter => eventEmitter.on(event, cb), + emit: (event: string, ...args: any[]): void => { const eventsMsg = new EvalEventMessage(); eventsMsg.setId(doEval.id); eventsMsg.setEvent(event); diff --git a/packages/protocol/src/node/server.ts b/packages/protocol/src/node/server.ts index 26e5ec0b7..e966146b8 100644 --- a/packages/protocol/src/node/server.ts +++ b/packages/protocol/src/node/server.ts @@ -20,12 +20,12 @@ export interface ServerOptions { } export class Server { - private readonly sessions: Map = new Map(); - private readonly connections: Map = new Map(); - private readonly servers: Map = new Map(); - private readonly evals: Map = new Map(); + private readonly sessions = new Map(); + private readonly connections = new Map(); + private readonly servers = new Map(); + private readonly evals = new Map(); - private connectionId: number = Number.MAX_SAFE_INTEGER; + private connectionId = Number.MAX_SAFE_INTEGER; public constructor( private readonly connection: ReadWriteConnection, diff --git a/packages/server/src/ipc.ts b/packages/server/src/ipc.ts index c25a17155..b5732a9c5 100644 --- a/packages/server/src/ipc.ts +++ b/packages/server/src/ipc.ts @@ -3,7 +3,7 @@ import { ChildProcess } from "child_process"; export interface IpcMessage { readonly event: string; - readonly args: any[]; + readonly args: any[]; // tslint:disable-line no-any } export class StdioIpcHandler extends EventEmitter { @@ -15,21 +15,28 @@ export class StdioIpcHandler extends EventEmitter { super(); } + // tslint:disable-next-line no-any public on(event: string, cb: (...args: any[]) => void): this { this.listen(); + return super.on(event, cb); } + // tslint:disable-next-line no-any public once(event: string, cb: (...args: any[]) => void): this { this.listen(); + return super.once(event, cb); } + // tslint:disable-next-line no-any public addListener(event: string, cb: (...args: any[]) => void): this { this.listen(); + return super.addListener(event, cb); } + // tslint:disable-next-line no-any public send(event: string, ...args: any[]): void { const msg: IpcMessage = { event, @@ -47,7 +54,8 @@ export class StdioIpcHandler extends EventEmitter { if (this.isListening) { return; } - const onData = (data: any) => { + // tslint:disable-next-line no-any + const onData = (data: any): void => { try { const d = JSON.parse(data.toString()) as IpcMessage; this.emit(d.event, ...d.args); diff --git a/packages/server/src/vscode/sharedProcess.ts b/packages/server/src/vscode/sharedProcess.ts index e780f6092..41e917dae 100644 --- a/packages/server/src/vscode/sharedProcess.ts +++ b/packages/server/src/vscode/sharedProcess.ts @@ -26,20 +26,16 @@ export class SharedProcess { private _state: SharedProcessState = SharedProcessState.Stopped; private activeProcess: ChildProcess | undefined; private ipcHandler: StdioIpcHandler | undefined; - private readonly onStateEmitter: Emitter; + private readonly onStateEmitter = new Emitter(); + public readonly onState = this.onStateEmitter.event; public constructor( private readonly userDataDir: string, private readonly builtInExtensionsDir: string, ) { - this.onStateEmitter = new Emitter(); this.restart(); } - public get onState(): Event { - return this.onStateEmitter.event; - } - public get state(): SharedProcessState { return this._state; } diff --git a/packages/vscode/src/fill/node-pty.ts b/packages/vscode/src/fill/node-pty.ts index 439d68d6d..e07132054 100644 --- a/packages/vscode/src/fill/node-pty.ts +++ b/packages/vscode/src/fill/node-pty.ts @@ -9,11 +9,10 @@ type nodePtyType = typeof nodePty; * Implementation of nodePty for the browser. */ class Pty implements nodePty.IPty { - private readonly emitter: EventEmitter; + private readonly emitter = new EventEmitter(); private readonly cp: ChildProcess; public constructor(file: string, args: string[] | string, options: nodePty.IPtyForkOptions) { - this.emitter = new EventEmitter(); this.cp = client.spawn(file, Array.isArray(args) ? args : [args], { ...options, tty: { @@ -38,7 +37,8 @@ class Pty implements nodePty.IPty { return this.cp.title!; } - public on(event: string, listener: (...args) => void): void { + // tslint:disable-next-line no-any + public on(event: string, listener: (...args: any[]) => void): void { this.emitter.on(event, listener); } diff --git a/packages/vscode/src/fill/stdioElectron.ts b/packages/vscode/src/fill/stdioElectron.ts index 9f2de4781..a31b7b303 100644 --- a/packages/vscode/src/fill/stdioElectron.ts +++ b/packages/vscode/src/fill/stdioElectron.ts @@ -4,11 +4,13 @@ import { IpcRenderer } from "electron"; export * from "@coder/ide/src/fill/electron"; class StdioIpcRenderer extends StdioIpcHandler implements IpcRenderer { - public sendTo(windowId: number, channel: string, ...args: any[]): void { + // tslint:disable-next-line no-any + public sendTo(_windowId: number, _channel: string, ..._args: any[]): void { throw new Error("Method not implemented."); } - public sendToHost(channel: string, ...args: any[]): void { + // tslint:disable-next-line no-any + public sendToHost(_channel: string, ..._args: any[]): void { throw new Error("Method not implemented."); } diff --git a/packages/vscode/src/fill/storageDatabase.ts b/packages/vscode/src/fill/storageDatabase.ts index ffde38f75..e37670e9d 100644 --- a/packages/vscode/src/fill/storageDatabase.ts +++ b/packages/vscode/src/fill/storageDatabase.ts @@ -10,7 +10,7 @@ import { logger, field } from "@coder/logger"; class StorageDatabase implements workspaceStorage.IStorageDatabase { public readonly onDidChangeItemsExternal = Event.None; - private items = new Map(); + private readonly items = new Map(); private fetched: boolean = false; public constructor(private readonly path: string) { diff --git a/packages/vscode/src/fill/windowsService.ts b/packages/vscode/src/fill/windowsService.ts index 7c66b4f69..b2228817b 100644 --- a/packages/vscode/src/fill/windowsService.ts +++ b/packages/vscode/src/fill/windowsService.ts @@ -17,21 +17,21 @@ class WindowsService implements IWindowsService { // tslint:disable-next-line no-any public _serviceBrand: any; - private openEmitter = new Emitter(); - private focusEmitter = new Emitter(); - private blurEmitter = new Emitter(); - private maximizeEmitter = new Emitter(); - private unmaximizeEmitter = new Emitter(); - private recentlyOpenedChangeEmitter = new Emitter(); + private readonly openEmitter = new Emitter(); + private readonly focusEmitter = new Emitter(); + private readonly blurEmitter = new Emitter(); + private readonly maximizeEmitter = new Emitter(); + private readonly unmaximizeEmitter = new Emitter(); + private readonly recentlyOpenedChangeEmitter = new Emitter(); - public onWindowOpen = this.openEmitter.event; - public onWindowFocus = this.focusEmitter.event; - public onWindowBlur = this.blurEmitter.event; - public onWindowMaximize = this.maximizeEmitter.event; - public onWindowUnmaximize = this.unmaximizeEmitter.event; - public onRecentlyOpenedChange = this.recentlyOpenedChangeEmitter.event; + public readonly onWindowOpen = this.openEmitter.event; + public readonly onWindowFocus = this.focusEmitter.event; + public readonly onWindowBlur = this.blurEmitter.event; + public readonly onWindowMaximize = this.maximizeEmitter.event; + public readonly onWindowUnmaximize = this.unmaximizeEmitter.event; + public readonly onRecentlyOpenedChange = this.recentlyOpenedChangeEmitter.event; - private window = new electron.BrowserWindow(); + private readonly window = new electron.BrowserWindow(); // Dialogs public pickFileFolderAndOpen(_options: INativeOpenDialogOptions): Promise {