Upgrade typescript & eslint to latest, fix resulting errors (#455)

Also updated the ci.yml node test versions to 18.x and 20.x, given current support: https://endoflife.date/nodejs

.github/workflows/ci.yml

@@ -12,7 +12,7 @@ jobs:
 
     strategy:
       matrix:
-        node-version: [10.x, 12.x, 14.x]
+        node-version: [18.x, 20.x]
 
     steps:
       - uses: actions/checkout@v2

package.json

@@ -34,11 +34,11 @@
     "@types/jszip": "3.1.4",
     "@types/node": "14.0.1",
     "@types/pako": "1.0.0",
-    "@typescript-eslint/eslint-plugin": "4.19.0",
-    "@typescript-eslint/parser": "4.19.0",
+    "@typescript-eslint/eslint-plugin": "6.16.0",
+    "@typescript-eslint/parser": "6.16.0",
     "acorn": "7.2.0",
     "aphrodite": "2.1.0",
-    "eslint": "6.0.0",
+    "eslint": "8.0.0",
     "eslint-plugin-prettier": "2.6.0",
     "eslint-plugin-react-hooks": "4.0.2",
     "jest": "24.3.0",
@@ -51,7 +51,7 @@
     "protobufjs": "6.8.8",
     "source-map": "0.6.1",
     "ts-jest": "24.3.0",
-    "typescript": "4.2.3",
+    "typescript": "5.3.3",
     "typescript-json-schema": "0.42.0",
     "uglify-es": "3.2.2",
     "uint8array-json-parser": "0.0.2"

src/import/file-system-entry.ts

@@ -1,17 +0,0 @@
-// The bits of this API that we care about. This is implemented by WebKitEntry
-// https://wicg.github.io/entries-api/#api-entry
-export interface FileSystemDirectoryReader {
-  readEntries(cb: (entries: FileSystemEntry[]) => void, error: (err: Error) => void): void
-}
-export interface FileSystemEntry {
-  readonly isFile: boolean
-  readonly isDirectory: boolean
-  readonly name: string
-  readonly fullPath: string
-}
-export interface FileSystemDirectoryEntry extends FileSystemEntry {
-  createReader(): FileSystemDirectoryReader
-}
-export interface FileSystemFileEntry extends FileSystemEntry {
-  file(cb: (file: File) => void, errCb: (err: Error) => void): void
-}

src/import/index.ts

@@ -1,5 +1,4 @@
 import {Profile, ProfileGroup} from '../lib/profile'
-import {FileSystemDirectoryEntry} from './file-system-entry'
 
 import {
   importFromChromeCPUProfile,

src/import/instruments.test.ts

@@ -3,7 +3,6 @@ import * as path from 'path'
 import {dumpProfile, checkProfileSnapshot} from '../lib/test-utils'
 
 import * as JSZip from 'jszip'
-import {FileSystemEntry} from './file-system-entry'
 import {importFromFileSystemDirectoryEntry} from '.'
 
 describe('importFromInstrumentsDeepCopy', () => {
@@ -24,7 +23,10 @@ describe('importFromInstrumentsDeepCopy', () => {
   })
 })
 
-class ZipBackedFileSystemEntry implements FileSystemEntry {
+// This is a bit of a weird type signature. I'm making this almost a
+// FileSystemEntry, but ignoring the parts of the API I don't use anywhere and
+// are a pain to implement.
+class ZipBackedFileSystemEntry implements Omit<FileSystemEntry, 'filesystem' | 'getParent'> {
   readonly isFile: boolean
   readonly isDirectory: boolean
   readonly name: string
@@ -73,7 +75,7 @@ class ZipBackedFileSystemEntry implements FileSystemEntry {
         const ret: FileSystemEntry[] = []
         this.zipDir.forEach((relativePath: string, file: {name: string}) => {
           if (relativePath.split('/').length === (relativePath.endsWith('/') ? 2 : 1)) {
-            ret.push(new ZipBackedFileSystemEntry(this.zip, file.name))
+            ret.push((new ZipBackedFileSystemEntry(this.zip, file.name) as any) as FileSystemEntry)
           }
         })
         cb(ret)
@@ -90,7 +92,10 @@ describe('importFromInstrumentsTrace', () => {
         JSZip.loadAsync(data).then(resolve)
       })
     })
-    const root = new ZipBackedFileSystemEntry(zip, 'simple-time-profile.trace')
+    const root: FileSystemDirectoryEntry = new ZipBackedFileSystemEntry(
+      zip,
+      'simple-time-profile.trace',
+    ) as any
     const profileGroup = await importFromFileSystemDirectoryEntry(root)
     const profile = profileGroup.profiles[profileGroup.indexToView]
     expect(dumpProfile(profile)).toMatchSnapshot()

src/import/instruments.ts

@@ -10,7 +10,6 @@ import {
 } from '../lib/profile'
 import {sortBy, getOrThrow, getOrInsert, lastOf, getOrElse, zeroPad} from '../lib/utils'
 import {ByteFormatter, TimeFormatter} from '../lib/value-formatters'
-import {FileSystemDirectoryEntry, FileSystemEntry, FileSystemFileEntry} from './file-system-entry'
 import {MaybeCompressedDataReader, TextFileContent} from './utils'
 
 function parseTSV<T>(contents: TextFileContent): T[] {

src/lib/test-utils.ts

@@ -105,6 +105,6 @@ export async function expectImportFailure(filepath: string) {
     await importProfilesFromArrayBuffer(path.basename(filepath), arrayBuffer)
     fail('Expected import to fail but it succeeded')
   } catch (error) {
-    expect(error.message).toMatchSnapshot()
+    expect((error as Error).message).toMatchSnapshot()
   }
 }

src/views/application.tsx

@@ -1,6 +1,5 @@
 import {h} from 'preact'
 import {StyleSheet, css} from 'aphrodite'
-import {FileSystemDirectoryEntry} from '../import/file-system-entry'
 
 import {ProfileGroup, SymbolRemapper} from '../lib/profile'
 import {FontFamily, FontSize, Duration} from './style'
@@ -60,6 +59,10 @@ async function importFromFileSystemDirectoryEntry(entry: FileSystemDirectoryEntr
 declare function require(x: string): any
 const exampleProfileURL = require('../../sample/profiles/stackcollapse/perf-vertx-stacks-01-collapsed-all.txt')
 
+function isFileSystemDirectoryEntry(entry: FileSystemEntry): entry is FileSystemDirectoryEntry {
+  return entry != null && entry.isDirectory
+}
+
 interface GLCanvasProps {
   canvasContext: CanvasContext | null
   theme: Theme
@@ -306,13 +309,18 @@ export class Application extends StatelessComponent<ApplicationProps> {
 
     const firstItem = ev.dataTransfer.items[0]
     if ('webkitGetAsEntry' in firstItem) {
-      const webkitEntry: FileSystemDirectoryEntry = firstItem.webkitGetAsEntry()
+      const webkitEntry: FileSystemEntry | null = firstItem.webkitGetAsEntry()
 
       // Instrument.app file format is actually a directory.
-      if (webkitEntry.isDirectory && webkitEntry.name.endsWith('.trace')) {
+      if (
+        webkitEntry &&
+        isFileSystemDirectoryEntry(webkitEntry) &&
+        webkitEntry.name.endsWith('.trace')
+      ) {
         console.log('Importing as Instruments.app .trace file')
+        const webkitDirectoryEntry: FileSystemDirectoryEntry = webkitEntry
         this.loadProfile(async () => {
-          return await importFromFileSystemDirectoryEntry(webkitEntry)
+          return await importFromFileSystemDirectoryEntry(webkitDirectoryEntry)
         })
         return
       }

src/views/flamechart-pan-zoom-view.tsx

@@ -631,7 +631,7 @@ export class FlamechartPanZoomView extends Component<FlamechartPanZoomViewProps,
       this.onMouseDrag(ev)
       return
     }
-    this.hoveredLabel = null
+
     const logicalViewSpaceMouse = new Vec2(ev.offsetX, ev.offsetY)
     const physicalViewSpaceMouse = this.logicalToPhysicalViewSpace().transformPosition(
       logicalViewSpaceMouse,
@@ -661,6 +661,21 @@ export class FlamechartPanZoomView extends Component<FlamechartPanZoomViewProps,
       }
     }
 
+    // This is a dumb hack to get around what appears to be a bug in
+    // TypeScript's reachability analysis. If I do the this.hoveredLabel = null
+    // in the outer function body, the code below accessing
+    // this.hoveredLabel!.node inside of the `if (this.hoveredLabel) {`
+    // complains that "no property node on never", indicating that it thinks
+    // that codepath is unreachable.
+    //
+    // Because this.hoveredLabel is accessed in the bound function
+    // setHoveredLabel, the codepath is obviously reachable, but the type
+    // checker is confused about this for some reason.
+    const clearHoveredLabel = () => {
+      this.hoveredLabel = null
+    }
+    clearHoveredLabel()
+
     for (let frame of this.props.flamechart.getLayers()[0] || []) {
       setHoveredLabel(frame)
     }