From 3f34936acd5ef199edf056671f5b39d5c2b88902 Mon Sep 17 00:00:00 2001 From: Laurent Nguyen Date: Mon, 6 Jul 2020 17:16:24 +0200 Subject: [PATCH] Add more files to strict compile. Update CONTRIBUTING.md (#63) * Add more files to strict compile. Update CONTRIBUTING.md to recommend FluentUI use * Remove eslint-disable and use non-null assertion --- CONTRIBUTING.md | 9 +++++---- src/Bindings/BindingHandlersRegisterer.ts | 7 +------ src/Common/Logger.ts | 10 +++++----- src/Contracts/Diagnostics.ts | 2 +- src/Controls/Heatmap/Heatmap.ts | 9 +++++---- src/Explorer/Notebook/NTeractUtil.ts | 2 +- .../Notebook/NotebookComponent/types.ts | 6 +++--- src/ReactDevTools.ts | 2 +- src/Shared/StorageUtility.ts | 4 ++-- src/Shared/StringUtility.ts | 4 ++-- src/Shared/Telemetry/TelemetryProcessor.ts | 10 ++++++---- tsconfig.strict.json | 20 +++++++++++++++---- 12 files changed, 48 insertions(+), 37 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d2688e4f1..bee66a903 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -30,12 +30,13 @@ For IE support, polyfill is preferred over new usage of lodash or underscore. We ### Typescript * Follow this [typescript style guide](https://github.com/excelmicro/typescript) which is based on [airbnb's style guide](https://github.com/airbnb/javascript). * Conventions speficic to this project: - * Use double-quotes for string - * Don't use null, use undefined - * Pascal case for private static readonly fields - * Camel case for classnames in markup + - Use double-quotes for string + - Don't use `null`, use `undefined` + - Pascal case for private static readonly fields + - Camel case for classnames in markup * Don't use class unless necessary * Code related to notebooks should be dynamically imported so that it is loaded from a separate bundle only if the account is notebook-enabled. There are already top-level notebook components which are dynamically imported and their dependencies can be statically imported from these files. +* Prefer using [Fluent UI controls](https://developer.microsoft.com/en-us/fluentui#/controls/web) over creating your own, in order to maintain consistency and support a11y. ### React * Prefer using React class components over function components and hooks unless you have a simple component and require no nested functions: diff --git a/src/Bindings/BindingHandlersRegisterer.ts b/src/Bindings/BindingHandlersRegisterer.ts index 4c1679e77..f2254757b 100644 --- a/src/Bindings/BindingHandlersRegisterer.ts +++ b/src/Bindings/BindingHandlersRegisterer.ts @@ -1,11 +1,6 @@ import * as ko from "knockout"; import * as ReactBindingHandler from "./ReactBindingHandler"; -interface RestorePoint { - readonly element: JQuery; - readonly width: number; -} - export class BindingHandlersRegisterer { public static registerBindingHandlers() { ko.bindingHandlers.setTemplateReady = { @@ -17,7 +12,7 @@ export class BindingHandlersRegisterer { bindingContext?: ko.BindingContext ) { const value = ko.unwrap(wrappedValueAccessor()); - bindingContext.$data.isTemplateReady(value); + bindingContext?.$data.isTemplateReady(value); } } as ko.BindingHandler; diff --git a/src/Common/Logger.ts b/src/Common/Logger.ts index aade162b9..bb19035f0 100644 --- a/src/Common/Logger.ts +++ b/src/Common/Logger.ts @@ -59,13 +59,13 @@ function _generateLogEntry( level: Diagnostics.LogEntryLevel, message: string, area: string, - code: number + code?: number ): Diagnostics.LogEntry { return { timestamp: new Date().getUTCSeconds(), - level: level, - message: message, - area: area, - code: code + level, + message, + area, + code }; } diff --git a/src/Contracts/Diagnostics.ts b/src/Contracts/Diagnostics.ts index b6dfc5d28..6a523bfd5 100644 --- a/src/Contracts/Diagnostics.ts +++ b/src/Contracts/Diagnostics.ts @@ -46,7 +46,7 @@ export interface LogEntry { /** * The message code. */ - code: number; + code?: number; /** * Any additional data to be logged. */ diff --git a/src/Controls/Heatmap/Heatmap.ts b/src/Controls/Heatmap/Heatmap.ts index f6bbed531..05d96ae0b 100644 --- a/src/Controls/Heatmap/Heatmap.ts +++ b/src/Controls/Heatmap/Heatmap.ts @@ -198,7 +198,7 @@ export class Heatmap { let timeSelected: string = data.points[0].x; timeSelected = timeSelected.replace(" ", "T"); timeSelected = `${timeSelected}Z`; - let xAxisIndex; + let xAxisIndex = 0; for (let i = 0; i < this._chartData.xAxisPoints.length; i++) { if (this._chartData.xAxisPoints[i] === timeSelected) { xAxisIndex = i; @@ -234,7 +234,8 @@ export function handleMessage(event: MessageEvent) { return; } Plotly.purge(Heatmap.elementId); - document.getElementById(Heatmap.elementId).innerHTML = ""; + + document.getElementById(Heatmap.elementId)!.innerHTML = ""; const data = event.data.data; const chartData: DataPayload = data.chartData; const chartSettings: HeatmapCaptions = data.chartSettings; @@ -259,8 +260,8 @@ export function handleMessage(event: MessageEvent) { noDataMessageContent.classList.add("dark-theme"); } - document.getElementById(Heatmap.elementId).appendChild(chartTitleElement); - document.getElementById(Heatmap.elementId).appendChild(noDataMessageElement); + document.getElementById(Heatmap.elementId)!.appendChild(chartTitleElement); + document.getElementById(Heatmap.elementId)!.appendChild(noDataMessageElement); } } diff --git a/src/Explorer/Notebook/NTeractUtil.ts b/src/Explorer/Notebook/NTeractUtil.ts index 1dace7511..b54afb530 100644 --- a/src/Explorer/Notebook/NTeractUtil.ts +++ b/src/Explorer/Notebook/NTeractUtil.ts @@ -4,7 +4,7 @@ import { NotebookContentRecordProps, selectors } from "@nteract/core"; * A bunch of utilities to interact with nteract */ export default class NTeractUtil { - public static getCurrentCellType(content: NotebookContentRecordProps): "markdown" | "code" | "raw" { + public static getCurrentCellType(content: NotebookContentRecordProps): "markdown" | "code" | "raw" | undefined { if (!content) { return undefined; } diff --git a/src/Explorer/Notebook/NotebookComponent/types.ts b/src/Explorer/Notebook/NotebookComponent/types.ts index 7cf6f2827..54c627d80 100644 --- a/src/Explorer/Notebook/NotebookComponent/types.ts +++ b/src/Explorer/Notebook/NotebookComponent/types.ts @@ -5,10 +5,10 @@ import { Notebook } from "../../../Common/Constants"; import { CellId } from "@nteract/commutable"; export interface CdbRecordProps { - databaseAccountName: string; - defaultExperience: string; + databaseAccountName: string | undefined; + defaultExperience: string | undefined; kernelRestartDelayMs: number; - hoveredCellId: CellId; + hoveredCellId: CellId | undefined; } export type CdbRecord = Immutable.RecordOf; diff --git a/src/ReactDevTools.ts b/src/ReactDevTools.ts index 6c983147e..19512901f 100644 --- a/src/ReactDevTools.ts +++ b/src/ReactDevTools.ts @@ -1,3 +1,3 @@ if (window.parent !== window) { - window.__REACT_DEVTOOLS_GLOBAL_HOOK__ = window.parent.__REACT_DEVTOOLS_GLOBAL_HOOK__; + (window as any).__REACT_DEVTOOLS_GLOBAL_HOOK__ = (window.parent as any).__REACT_DEVTOOLS_GLOBAL_HOOK__; } diff --git a/src/Shared/StorageUtility.ts b/src/Shared/StorageUtility.ts index bc3de00eb..657bf8d6e 100644 --- a/src/Shared/StorageUtility.ts +++ b/src/Shared/StorageUtility.ts @@ -5,7 +5,7 @@ export class LocalStorageUtility { return !!localStorage.getItem(StorageKey[key]); } - public static getEntryString(key: StorageKey): string { + public static getEntryString(key: StorageKey): string | null { return localStorage.getItem(StorageKey[key]); } @@ -39,7 +39,7 @@ export class SessionStorageUtility { return !!sessionStorage.getItem(StorageKey[key]); } - public static getEntryString(key: StorageKey): string { + public static getEntryString(key: StorageKey): string | null { return sessionStorage.getItem(StorageKey[key]); } diff --git a/src/Shared/StringUtility.ts b/src/Shared/StringUtility.ts index 3ce50913b..122f014bb 100644 --- a/src/Shared/StringUtility.ts +++ b/src/Shared/StringUtility.ts @@ -1,9 +1,9 @@ export class StringUtility { - public static toNumber(num: string): number { + public static toNumber(num: string | null): number { return Number(num); } - public static toBoolean(valueStr: string): boolean { + public static toBoolean(valueStr: string | null): boolean { return valueStr === "true"; } } diff --git a/src/Shared/Telemetry/TelemetryProcessor.ts b/src/Shared/Telemetry/TelemetryProcessor.ts index 50500f46b..24e763f8e 100644 --- a/src/Shared/Telemetry/TelemetryProcessor.ts +++ b/src/Shared/Telemetry/TelemetryProcessor.ts @@ -82,33 +82,35 @@ export default class TelemetryProcessor { } public static traceOpen(action: Action, data?: any, timestamp?: number): number { + const validTimestamp = timestamp || Date.now(); MessageHandler.sendMessage({ type: MessageTypes.TelemetryInfo, data: { action: Action[action], actionModifier: ActionModifiers.Open, - timestamp: timestamp || Date.now(), + timestamp: validTimestamp, data: JSON.stringify(data) } }); appInsights.startTrackEvent(Action[action]); - return timestamp; + return validTimestamp; } public static traceMark(action: Action, data?: any, timestamp?: number): number { + const validTimestamp = timestamp || Date.now(); MessageHandler.sendMessage({ type: MessageTypes.TelemetryInfo, data: { action: Action[action], actionModifier: ActionModifiers.Mark, - timestamp: timestamp || Date.now(), + timestamp: validTimestamp, data: JSON.stringify(data) } }); appInsights.startTrackEvent(Action[action]); - return timestamp; + return validTimestamp; } private static getData(data?: any): any { diff --git a/tsconfig.strict.json b/tsconfig.strict.json index 2d6db7001..975649041 100644 --- a/tsconfig.strict.json +++ b/tsconfig.strict.json @@ -8,11 +8,14 @@ }, "files": [ "./src/AuthType.ts", + "./src/Bindings/BindingHandlersRegisterer.ts", "./src/Bindings/ReactBindingHandler.ts", "./src/Common/ArrayHashMap.ts", "./src/Common/Constants.ts", "./src/Common/DeleteFeedback.ts", "./src/Common/HashMap.ts", + "./src/Common/HeadersUtility.ts", + "./src/Common/Logger.ts", "./src/Common/MessageHandler.ts", "./src/Common/ObjectCache.ts", "./src/Common/ThemeUtility.ts", @@ -23,8 +26,10 @@ "./src/Contracts/Diagnostics.ts", "./src/Contracts/ExplorerContracts.ts", "./src/Contracts/Versions.ts", + "./src/Controls/Heatmap/Heatmap.ts", "./src/Controls/Heatmap/HeatmapDatatypes.ts", "./src/Definitions/plotly.js-cartesian-dist.d-min.ts", + "./src/Explorer/Controls/GitHub/GitHubStyleConstants.ts", "./src/Explorer/Controls/Toolbar/IToolbarAction.ts", "./src/Explorer/Controls/Toolbar/IToolbarDisplayable.ts", "./src/Explorer/Controls/Toolbar/IToolbarDropDown.ts", @@ -35,8 +40,11 @@ "./src/Explorer/Notebook/FileSystemUtil.ts", "./src/Explorer/Notebook/NotebookComponent/actions.ts", "./src/Explorer/Notebook/NotebookComponent/loadTransform.ts", + "./src/Explorer/Notebook/NotebookComponent/reducers.ts", + "./src/Explorer/Notebook/NotebookComponent/types.ts", "./src/Explorer/Notebook/NotebookContentItem.ts", "./src/Explorer/Notebook/NotebookUtil.ts", + "./src/Explorer/Notebook/NTeractUtil.ts", "./src/Explorer/Panes/Tables/Validators/EntityPropertyNameValidator.ts", "./src/Explorer/Panes/Tables/Validators/EntityPropertyValidationCommon.ts", "./src/Explorer/Tables/Constants.ts", @@ -44,17 +52,21 @@ "./src/GitHub/GitHubConnector.ts", "./src/NotebookWorkspaceManager/NotebookWorkspaceResourceProviderMockClients.ts", "./src/PlatformType.ts", + "./src/quickstart.ts", + "./src/ReactDevTools.ts", "./src/ResourceProvider/IResourceProviderClient.ts", + "./src/setupTests.ts", + "./src/Shared/appInsights.ts", + "./src/Shared/ExplorerSettings.ts", + "./src/Shared/StorageUtility.ts", "./src/Shared/StringUtility.ts", "./src/Shared/Telemetry/TelemetryConstants.ts", - "./src/Shared/appInsights.ts", + "./src/Shared/Telemetry/TelemetryProcessor.ts", "./src/Utils/GitHubUtils.ts", "./src/Utils/MessageValidation.ts", "./src/Utils/OfferUtils.ts", "./src/Utils/StringUtils.ts", - "./src/quickstart.ts", - "./src/setupTests.ts", "./src/workers/upload/definitions.ts" ], "include": [] -} +} \ No newline at end of file