Skip to content

Commit 7eac3e5

Browse files
authored
build(srcts): Turn on TypeScript strict mode (#154)
Fix the 40 errors strict mode reports: - Type the untyped locals and callback parameters. - Return `T | undefined` from StatusArr.remove()/last() and the Node wrappers, and use `?.` where the last status may be missing. - Walk the ancestor and descendant ids without reassigning a `string | null` variable. - Mark GraphAtStep fields set by its init methods with `!`. - Read the timeline position from the native mouse event. - Replace utils/ArrayHelper.ts with Array#flatMap and utils/queryString.ts with URLSearchParams.
1 parent 1702759 commit 7eac3e5

17 files changed

Lines changed: 66 additions & 109 deletions

File tree

‎AGENTS.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,6 @@ Read it before working in `R/` or `srcts/`.
5555

5656
## Conventions
5757

58-
- Source is TypeScript (`tsconfig.json`, `strict` off for now). Lint is eslint 10 + typescript-eslint + prettier 3 (`eslint.config.mjs`, `prettier.config.js`).
58+
- Source is TypeScript (`tsconfig.json`, `strict` on). Lint is eslint 10 + typescript-eslint + prettier 3 (`eslint.config.mjs`, `prettier.config.js`).
5959
- Add a NEWS.md entry for user-facing changes (CONTRIBUTING.md).
6060
- New top-level files need an entry in `.Rbuildignore`, or R CMD check will warn.

‎inst/reactlog/reactlogAsset/reactlog.js‎

Lines changed: 5 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎inst/reactlog/reactlogAsset/reactlog.js.map‎

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎memory-bank/jsBuild.md‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ blank lines.
3131
| File | Role |
3232
| --- | --- |
3333
| `package.json` | npm scripts, `pre-commit` / `lint-staged` hook config, dependencies |
34-
| `tsconfig.json` | `tsc` settings: `noEmit`, `target` es2021, `moduleResolution` bundler, `strict` off |
34+
| `tsconfig.json` | `tsc` settings: `noEmit`, `target` es2021, `moduleResolution` bundler, `strict` on |
3535
| `srcts/modules.d.ts` | `declare module` for untyped `cytoscape` / `cytoscape-dagre`, and the `window` globals |
3636
| `srcts/cyto/cytoFlowType.ts` | Hand-written types for the parts of cytoscape the app uses |
3737
| `eslint.config.mjs` / `prettier.config.js` | Lint and format rules |
@@ -68,9 +68,9 @@ blank lines.
6868
- `typescript` is pinned to `~6.0`. TypeScript 7 (the Go port) does not offer
6969
the JS API that `typescript-eslint` needs, and `typescript-eslint` 8 lists
7070
`typescript <6.1` as its peer.
71-
- `strict` is off. `npx tsc -p . --strict` reports about 40 errors, mostly
72-
missing null checks, implicit `any`, and class fields that are set outside
73-
the constructor. Fix those in their own change.
71+
- `strict` is on. `GraphAtStep` fields filled by `initStepInfo()` and
72+
`updateFinalFilteredGraph()` (called from the constructor) are marked with
73+
`!`. `no-explicit-any` is still off, so `any` still skips checks.
7474
- cytoscape 3.15 (the locked version) has no bundled types, so it is declared
7575
as an untyped module and the app uses the hand-written types in
7676
`cyto/cytoFlowType.ts`. Newer cytoscape ships `index.d.ts`, but upgrading it

‎memory-bank/viewerUi.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ Key files:
2424
| `srcts/layout/progressBar.ts` | Timeline fill, ticks, click/drag to seek |
2525
| `srcts/layout/logEntry.ts` | Step / time / session / status text and the raw JSON panel |
2626
| `srcts/cyto/cytoOn.ts` | Mouse events on the graph (hover, tap, double click) |
27-
| `srcts/utils/queryString.ts`, `srcts/utils/shinytest.ts` | Read `?test=1` |
27+
| `srcts/utils/shinytest.ts` | Read `?test=1` |
2828
| `inst/reactlog/reactlogAsset/` | `reactlog.css`, font CSS, `images/*.svg`, built `reactlog.js` |
2929

3030
## The HTML template
@@ -157,7 +157,7 @@ Node time labels are separate: `rlog.displayTimeOnNodes` is true unless `window.
157157

158158
## Query params and test mode
159159

160-
`parseQuery()` (`srcts/utils/queryString.ts`) turns `window.location.search` into an object. The only key read is `test`: `inTestMode = parseQuery().test === "1"` (`srcts/utils/shinytest.ts`). In test mode:
160+
`srcts/utils/shinytest.ts` reads the `test` query parameter with `URLSearchParams`: `inTestMode` is true when the URL has `?test=1`. In test mode:
161161

162162
- the header time and session show `(shinytest)` (`logEntry.ts`),
163163
- `Node.calculationTimeStr` returns `(shinytest)`.

‎srcts/cyto/cytoOn.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,8 @@ let onMouseOver = function (cyto: CytoscapeType) {
7777
};
7878

7979
let onClick = function (cyto: CytoscapeType) {
80-
let cytoClickedBefore, cytoClickedTimeout;
80+
let cytoClickedBefore: CytoEvent["target"] | null = null;
81+
let cytoClickedTimeout: ReturnType<typeof setTimeout> | undefined;
8182

8283
return function (evt: CytoEvent) {
8384
// remove focus on search

‎srcts/graph/Graph.ts‎

Lines changed: 20 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import { GhostEdge } from "./GhostEdge";
1414
import { HoverStatus } from "./HoverStatus";
1515
import { expectPrevStatus } from "./StatusArr";
1616

17-
import { flatMap } from "../utils/ArrayHelper";
1817
import console from "../utils/console";
1918

2019
import type { CytoscapeType } from "../cyto/cytoFlowType";
@@ -106,7 +105,7 @@ class Graph {
106105

107106
let reactId = data.reactId;
108107
let depOnReactId = data.depOnReactId;
109-
let hasMatchingEdge = function (edges) {
108+
let hasMatchingEdge = function (edges: Array<Edge | GhostEdge>) {
110109
return _some(edges, function (edge) {
111110
if (edge.reactId === reactId && edge.depOnReactId === depOnReactId) {
112111
return true;
@@ -148,7 +147,7 @@ class Graph {
148147
// highlight all the edges (and ghost edges) that have the same source and target
149148
let reactId = data.reactId;
150149
let depOnReactId = data.depOnReactId;
151-
let selectMatchingEdges = function (edge) {
150+
let selectMatchingEdges = function (edge: Edge | GhostEdge) {
152151
if (edge.reactId === reactId && edge.depOnReactId === depOnReactId) {
153152
onFn(edge);
154153
}
@@ -239,15 +238,15 @@ class Graph {
239238
let originalReactId = reactId;
240239
let seenMap = new Set<ReactIdType>();
241240
let reactIdArr = [reactId];
242-
while (reactIdArr.length > 0) {
243-
reactId = reactIdArr.pop();
244-
if (!seenMap.has(reactId)) {
245-
this.parentNodeIds(reactId).forEach(function (parentReactId) {
241+
let curId: ReactIdType | undefined;
242+
while ((curId = reactIdArr.pop()) !== undefined) {
243+
if (!seenMap.has(curId)) {
244+
this.parentNodeIds(curId).forEach(function (parentReactId) {
246245
if (parentReactId) {
247246
reactIdArr.push(parentReactId);
248247
}
249248
});
250-
seenMap.add(reactId);
249+
seenMap.add(curId);
251250
}
252251
}
253252
seenMap.delete(originalReactId);
@@ -260,11 +259,11 @@ class Graph {
260259
let originalReactId = reactId;
261260
let seenMap = new Set<ReactIdType>();
262261
let reactIdArr = [reactId];
263-
while (reactIdArr.length > 0) {
264-
reactId = reactIdArr.pop();
265-
if (!seenMap.has(reactId)) {
266-
reactIdArr = reactIdArr.concat(this.childrenNodeIds(reactId));
267-
seenMap.add(reactId);
262+
let curId: ReactIdType | undefined;
263+
while ((curId = reactIdArr.pop()) !== undefined) {
264+
if (!seenMap.has(curId)) {
265+
reactIdArr = reactIdArr.concat(this.childrenNodeIds(curId));
266+
seenMap.add(curId);
268267
}
269268
}
270269
seenMap.delete(originalReactId);
@@ -294,23 +293,23 @@ class Graph {
294293
familyTreeNodeIdsForDatas(datas: Array<SomeGraphData>): Array<ReactIdType> {
295294
let self = this;
296295
return _union(
297-
flatMap(datas, function (data) {
296+
datas.flatMap(function (data) {
298297
return self.familyTreeNodeIds(data);
299298
})
300299
);
301300
}
302301
decendentNodeIdsForDatas(datas: Array<SomeGraphData>): Array<ReactIdType> {
303302
let self = this;
304303
return _union(
305-
flatMap(datas, function (data: SomeGraphData) {
304+
datas.flatMap(function (data) {
306305
return self.decendentNodeIds(data);
307306
})
308307
);
309308
}
310309
ancestorNodeIdsForDatas(datas: Array<SomeGraphData>): Array<ReactIdType> {
311310
let self = this;
312311
return _union(
313-
flatMap(datas, function (data) {
312+
datas.flatMap(function (data) {
314313
return self.ancestorNodeIds(data);
315314
})
316315
);
@@ -475,7 +474,8 @@ class Graph {
475474
node = this.nodes.get(data.reactId);
476475
switch (data.action) {
477476
case LogStates.exit: {
478-
let activeEnterNode = this.nodes.get(_last(this.activeNodeEnter));
477+
lastNodeId = _last(this.activeNodeEnter);
478+
let activeEnterNode = lastNodeId && this.nodes.get(lastNodeId);
479479
if (activeEnterNode) {
480480
activeEnterNode.enterStatus.reset();
481481
}
@@ -491,9 +491,8 @@ class Graph {
491491
}
492492
case LogStates.invalidateEnd: {
493493
// turn off the previously active node
494-
let curActiveNode = this.nodes.get(
495-
_last(this.activeInvalidateEnter)
496-
);
494+
lastNodeId = _last(this.activeInvalidateEnter);
495+
let curActiveNode = lastNodeId && this.nodes.get(lastNodeId);
497496
if (curActiveNode) {
498497
curActiveNode.invalidateStatus.resetActive();
499498
}
@@ -572,7 +571,7 @@ class Graph {
572571

573572
if (edge) {
574573
node = this.nodes.get(edge.reactId);
575-
if (node && node.statusLast().action === LogStates.isolateEnter) {
574+
if (node && node.statusLast()?.action === LogStates.isolateEnter) {
576575
edge.status = "isolate";
577576
} else {
578577
edge.status = "normal";

‎srcts/graph/GraphAtStep.ts‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -47,22 +47,22 @@ class GraphAtStep {
4747
hoverData: SomeGraphData | null | undefined;
4848
stickyDatas: Array<SomeGraphData>;
4949

50-
finalFilteredGraph: Graph;
50+
finalFilteredGraph!: Graph;
5151
finalCompleteGraph: Graph;
5252
finalCyto: any;
5353

5454
cytoLayout: any;
5555

56-
steps: Array<number>;
57-
stepsVisible: Array<number>;
58-
filteredStepsVisible: Array<number>;
59-
stepsAsyncStart: Array<number>;
60-
stepsAsyncStop: Array<number>;
61-
stepsIdle: Array<number>;
62-
stepsOutputCalc: Array<number>;
63-
stepsUserMark: Array<number>;
64-
minStep: number;
65-
maxStep: number;
56+
steps!: Array<number>;
57+
stepsVisible!: Array<number>;
58+
filteredStepsVisible!: Array<number>;
59+
stepsAsyncStart!: Array<number>;
60+
stepsAsyncStop!: Array<number>;
61+
stepsIdle!: Array<number>;
62+
stepsOutputCalc!: Array<number>;
63+
stepsUserMark!: Array<number>;
64+
minStep!: number;
65+
maxStep!: number;
6666

6767
constructor(log: LogType) {
6868
this.originalLog = log;
@@ -557,7 +557,7 @@ class GraphAtStep {
557557
let nodesLRB = cyNodes.diff(graphNodes);
558558
// .removeStyle()
559559

560-
let onLayoutReady = [];
560+
let onLayoutReady: Array<() => void> = [];
561561
let someNodeHasNewLabel = false;
562562

563563
// enter visible nodes

‎srcts/graph/Node.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ class Node {
8282
}
8383
}
8484

85-
get calculationTimeStr(): string {
85+
get calculationTimeStr(): string | null {
8686
if (_isNil(this.calculationTime)) {
8787
return null;
8888
}
@@ -115,7 +115,7 @@ class Node {
115115
this.statusArr.add(logEntry as StatusEntry);
116116
return this.statusArr;
117117
}
118-
statusRemove(logEntry: LogEntryAnyType): StatusEntry {
118+
statusRemove(logEntry: LogEntryAnyType): StatusEntry | undefined {
119119
if (logEntry.action === LogStates.exit) {
120120
let startEntryTime = this.calculationStartMap.get(logEntry.ctxId);
121121
if (!_isNil(startEntryTime)) {
@@ -125,7 +125,7 @@ class Node {
125125
}
126126
return this.statusArr.remove();
127127
}
128-
statusLast(): StatusEntry {
128+
statusLast(): StatusEntry | undefined {
129129
return this.statusArr.last();
130130
}
131131
get inEnter(): boolean {

‎srcts/graph/StatusArr.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ class StatusArr {
1111
constructor(statusArr: StatusArr | Array<StatusEntry> = []) {
1212
if (statusArr instanceof StatusArr) {
1313
this.statusArr = _cloneDeep(statusArr.statusArr);
14-
} else if (Array.isArray(statusArr)) {
14+
} else {
1515
this.statusArr = statusArr;
1616
}
1717
}
@@ -21,10 +21,10 @@ class StatusArr {
2121
add(obj: StatusEntry) {
2222
return this.statusArr.push(obj);
2323
}
24-
remove(): StatusEntry {
24+
remove(): StatusEntry | undefined {
2525
return this.statusArr.pop();
2626
}
27-
last(): StatusEntry {
27+
last(): StatusEntry | undefined {
2828
return _last(this.statusArr);
2929
}
3030
containsStatus(status: string) {
@@ -46,15 +46,15 @@ type StatusEntry = {
4646

4747
let expectPrevStatus = function (
4848
curStatus: StatusEntry,
49-
prevStatus: StatusEntry,
49+
prevStatus: StatusEntry | undefined,
5050
expectedAction: string
5151
) {
52-
function onError(msg: string) {
52+
function onError(msg: string): never {
5353
console.error("curStatus: ", curStatus);
5454
console.error("prevStatus: ", prevStatus);
5555
throw msg;
5656
}
57-
if (prevStatus.action !== expectedAction) {
57+
if (prevStatus?.action !== expectedAction) {
5858
onError(`prior node status does not have "${expectedAction}" status`);
5959
}
6060
if (prevStatus.ctxId !== curStatus.ctxId) {

0 commit comments

Comments
 (0)