From c8ab29be87c21912fe105bdc8d65d7c0b0c463c8 Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Mon, 12 Oct 2026 06:44:25 +0000 Subject: [PATCH 1/2] Check every index read in the app and the embed: the flag is on The last noUncheckedIndexedAccess findings in the app, in render.component.ts (62) and diagram.service.ts (23), handled without adding a non-null assertion; the app and the embeddable diagram now have the flag in their own tsconfig.app.json, so their builds enforce it. Turning it on also checked the templates, which tsc alone does not: five reads, four of them a sample's expression value passed to cr-expression-tag, which now draws only when there is a value to draw. The counted check moves on to what is left: the two workspace libraries, reactome-table and reactome-gsa-form, 84 findings in 10 files. When they reach zero the flag goes into tsconfig.json and the check goes. One stated change: a diagram link with no input or output already threw; it now says which link. Part of #345. Co-Authored-By: Claude Opus 5.5 --- indexed-access-baseline.json | 14 +++- .../found-table/found-table.component.html | 17 +++-- .../not-found-table.component.html | 15 ++-- .../tabs/result-tab/result-tab.component.html | 20 +++--- .../src/app/render/render.component.ts | 54 +++++++++------ .../src/app/services/diagram.service.ts | 68 ++++++++++++------- .../tsconfig.app.json | 5 +- scripts/check-indexed-access.mjs | 47 +++++++++---- tsconfig.app.json | 5 +- 9 files changed, 161 insertions(+), 84 deletions(-) diff --git a/indexed-access-baseline.json b/indexed-access-baseline.json index c84acffd..09f0bf9b 100644 --- a/indexed-access-baseline.json +++ b/indexed-access-baseline.json @@ -1,7 +1,15 @@ { - "total": 85, + "total": 84, "files": { - "projects/pathway-browser/src/app/render/render.component.ts": 62, - "projects/pathway-browser/src/app/services/diagram.service.ts": 23 + "projects/reactome-gsa-form/src/lib/dataset-form/datasets/select-dataset/dataset-types/local-data/local-data.component.ts": 4, + "projects/reactome-gsa-form/src/lib/state/dataset-source/dataset-source.effect.ts": 2, + "projects/reactome-gsa-form/src/lib/state/dataset/dataset.effect.ts": 1, + "projects/reactome-gsa-form/src/lib/state/dataset/dataset.reducer.ts": 3, + "projects/reactome-gsa-form/src/lib/state/dataset/dataset.selector.ts": 4, + "projects/reactome-gsa-form/src/lib/state/method/method.effect.ts": 2, + "projects/reactome-gsa-form/src/lib/state/method/method.reducer.ts": 1, + "projects/reactome-gsa-form/src/lib/utilities/species/species.pipe.ts": 1, + "projects/reactome-table/src/lib/state/table.store.ts": 63, + "projects/reactome-table/src/lib/utils/table.util.ts": 3 } } diff --git a/projects/pathway-browser/src/app/details/tabs/result-tab/found-table/found-table.component.html b/projects/pathway-browser/src/app/details/tabs/result-tab/found-table/found-table.component.html index 70f57db0..ef263c3d 100644 --- a/projects/pathway-browser/src/app/details/tabs/result-tab/found-table/found-table.component.html +++ b/projects/pathway-browser/src/app/details/tabs/result-tab/found-table/found-table.component.html @@ -34,7 +34,7 @@ @for (resource of resources(); track resource) { - @let resourceColumn = resourceColumnIds()[$index]; + @let resourceColumn = resourceColumnIds()[$index] ?? 'entities-' + resource; @let prefix = resourceToIdentifiersPrefix.get(resource)!; {{ resource | titlecase }} entities @@ -64,12 +64,15 @@
- + @let value = found.exp[i]; + @if (value !== undefined) { + + }
> diff --git a/projects/pathway-browser/src/app/details/tabs/result-tab/not-found-table/not-found-table.component.html b/projects/pathway-browser/src/app/details/tabs/result-tab/not-found-table/not-found-table.component.html index 7704d401..dbd513a0 100644 --- a/projects/pathway-browser/src/app/details/tabs/result-tab/not-found-table/not-found-table.component.html +++ b/projects/pathway-browser/src/app/details/tabs/result-tab/not-found-table/not-found-table.component.html @@ -19,12 +19,15 @@ {{ expressionColumnNames()[i] }} - + @let value = notFound.exp[i]; + @if (value !== undefined) { + + } > } diff --git a/projects/pathway-browser/src/app/details/tabs/result-tab/result-tab.component.html b/projects/pathway-browser/src/app/details/tabs/result-tab/result-tab.component.html index c3c79ee7..8d94ca25 100644 --- a/projects/pathway-browser/src/app/details/tabs/result-tab/result-tab.component.html +++ b/projects/pathway-browser/src/app/details/tabs/result-tab/result-tab.component.html @@ -55,8 +55,9 @@

@for (sample of analysis.samples(); track sample; let i = $index) { - @if (selected.values[i] !== undefined) { - + @let value = selected.values[i]; + @if (value !== undefined) { + } } @@ -209,12 +210,15 @@

{{ expressionColumnNames()[i] }} - + @let value = pathway.entities.exp[i]; + @if (value !== undefined) { + + } > } diff --git a/projects/pathway-browser/src/app/render/render.component.ts b/projects/pathway-browser/src/app/render/render.component.ts index 87d59527..98619293 100644 --- a/projects/pathway-browser/src/app/render/render.component.ts +++ b/projects/pathway-browser/src/app/render/render.component.ts @@ -417,11 +417,12 @@ export class RenderComponent { */ private async frameCanvas(scale: number): Promise { const { diagram, instances } = this.exportableInstances(); - if (diagram && instances.length) { + const [instance] = instances; + if (diagram && instance) { // White, not transparent. Every animated format in play here either has no // alpha channel or only a single transparent index, so a transparent // background composites to black rather than to nothing. - return diagram.exportCanvas(instances[0], { full: true, scale, bg: '#ffffff' }); + return diagram.exportCanvas(instance, { full: true, scale, bg: '#ffffff' }); } // An illustration is inline SVG rather than a canvas, so a frame has to be @@ -779,6 +780,8 @@ export class RenderComponent { const a = points[from]; const b = points[to]; + // In range: both are indexes of points, from the stack. + if (!a || !b) continue; const dx = b.x - a.x; const dy = b.y - a.y; const span = Math.hypot(dx, dy); @@ -787,6 +790,7 @@ export class RenderComponent { let at = -1; for (let index = from + 1; index < to; index++) { const point = points[index]; + if (!point) continue; // Distance to the chord, or to its endpoint when the chord is a point. const distance = span === 0 @@ -840,7 +844,7 @@ export class RenderComponent { private transformScale(element: Element): number { const transform = element.getAttribute('transform') ?? element.getAttribute('style') ?? ''; const scale = /scale\(\s*([\d.]+)/.exec(transform); - const value = scale ? parseFloat(scale[1]) : 1; + const value = parseFloat(scale?.[1] ?? '1'); return Number.isFinite(value) && value > 0 ? value : 1; } @@ -1053,18 +1057,19 @@ export class RenderComponent { const rgb = /^rgba?\(([^)]+)\)$/.exec(text); if (rgb) { - const parts = rgb[1].split(/[,\s/]+/).map((part) => parseFloat(part)); + const parts = (rgb[1] ?? '').split(/[,\s/]+/).map((part) => parseFloat(part)); if (parts.length >= 3 && parts.slice(0, 3).every((part) => Number.isFinite(part))) { const own = parts.length > 3 && Number.isFinite(parts[3]) ? parts[3] : 1; return withAlpha(parts.slice(0, 3), own); } } const hex = /^#?([0-9a-f]{6})$/i.exec(text); - if (hex) { - return withAlpha([0, 2, 4].map((at) => parseInt(hex[1].slice(at, at + 2), 16))); + if (hex?.[1]) { + const digits = hex[1]; + return withAlpha([0, 2, 4].map((at) => parseInt(digits.slice(at, at + 2), 16))); } const short = /^#?([0-9a-f]{3})$/i.exec(text); - if (short) { + if (short?.[1]) { return withAlpha([...short[1]].map((digit) => parseInt(digit + digit, 16))); } return null; @@ -1153,6 +1158,8 @@ export class RenderComponent { for (let index = 0; index < polygon.length; index++) { const current = polygon[index]; const previous = polygon[(index + polygon.length - 1) % polygon.length]; + // In range: both indexes are taken within the polygon's length. + if (!current || !previous) continue; const currentIn = inside(current); const previousIn = inside(previous); if (currentIn) { @@ -1187,18 +1194,20 @@ export class RenderComponent { for (let at = 0; at < points.length - 1; at++) { const from = points[at]; const to = points[at + 1]; + if (!from || !to) continue; const dx = to.x - from.x; const dy = to.y - from.y; let enter = 0; let leave = 1; let inside = true; - for (const [p, q] of [ + const edges: [p: number, q: number][] = [ [-dx, from.x - box.x1], [dx, box.x2 - from.x], [-dy, from.y - box.y1], [dy, box.y2 - from.y], - ]) { + ]; + for (const [p, q] of edges) { if (p === 0) { // Parallel to this edge: outside it means the whole segment is out. if (q < 0) { @@ -1219,8 +1228,9 @@ export class RenderComponent { const start = { x: from.x + dx * enter, y: from.y + dy * enter }; const end = { x: from.x + dx * leave, y: from.y + dy * leave }; - if (!run.length) run.push(start); - else if (!near(run[run.length - 1], start)) { + const last = run.at(-1); + if (!last) run.push(start); + else if (!near(last, start)) { if (run.length > 1) runs.push(run); run = [start]; } @@ -1266,8 +1276,10 @@ export class RenderComponent { const scale = this.pixels(edge.style('arrow-scale')) || 1; const size = Math.max(6, width * 4 * scale); - const tip = points[points.length - 1]; - const from = points[points.length - 2]; + const tip = points.at(-1); + const from = points.at(-2); + // Two points at least: checked on entry. + if (!tip || !from) return []; const run = Math.hypot(tip.x - from.x, tip.y - from.y) || 1; // Along the line, and across it. const ax = (tip.x - from.x) / run; @@ -1387,14 +1399,15 @@ export class RenderComponent { if (this.diagram()) { const { instances } = this.exportableInstances(); - if (!instances.length) throw new Error('no diagram to export'); + const [instance] = instances; + if (!instance) throw new Error('no diagram to export'); // One instance here by construction: this page never opens the // comparison view. // // full:false exports what the viewport shows, which is the point when the // viewport has been framed on one event. - const framed = this.frameSelection(instances[0]); - return instances[0].svg({ full: !framed }); + const framed = this.frameSelection(instance); + return instance.svg({ full: !framed }); } // Through the illustration's own service, which knows that its styling has @@ -1456,12 +1469,13 @@ export class RenderComponent { } const { diagram, instances } = this.exportableInstances(); - if (instances.length) { - const framed = this.frameSelection(instances[0]); + const [instance] = instances; + if (instance) { + const framed = this.frameSelection(instance); if (jpeg && diagram) { - return asJpeg(diagram.exportCanvas(instances[0], { full: !framed, scale, bg })); + return asJpeg(diagram.exportCanvas(instance, { full: !framed, scale, bg })); } - return instances[0].png({ full: !framed, scale, bg }); + return instance.png({ full: !framed, scale, bg }); } // An illustration has no cytoscape instance to ask, so it goes through the diff --git a/projects/pathway-browser/src/app/services/diagram.service.ts b/projects/pathway-browser/src/app/services/diagram.service.ts index 687655ee..6063b690 100644 --- a/projects/pathway-browser/src/app/services/diagram.service.ts +++ b/projects/pathway-browser/src/app/services/diagram.service.ts @@ -63,10 +63,12 @@ const isFinitePoint = (point: Position | undefined | null): point is Position => const closestToAverage = (positions: Position[]): Position => { const average = avg(positions); - let closest = positions[0]; + const [first, ...rest] = positions; + // No positions has no closest; the average (of nothing) stands in for it. + if (!first) return average; + let closest = first; let min = squaredDist(closest, average); - for (let i = 1; i < positions.length; i++) { - const pos = positions[i]; + for (const pos of rest) { const dist = squaredDist(pos, average); if (dist < min) { min = dist; @@ -169,7 +171,9 @@ export class DiagramService { } pick(values: T[]): T { - return values[this.random(0, values.length - 1)]; + const value = values[this.random(0, values.length - 1)]; + if (value === undefined) throw new Error('There is nothing to pick from.'); + return value; } private readonly COMPARTMENT_SHIFT = 35; @@ -183,7 +187,7 @@ export class DiagramService { .get(`${CONTENT_SERVICE}/data/query/${id}/normalPathway`, { responseType: 'text', }) - .pipe(map((data) => data.split('\t')[0])); + .pipe(map((data) => data.split('\t')[0] ?? data)); } public getCHEBIStructure(ids: Set): Map> { @@ -258,7 +262,11 @@ export class DiagramService { node.standardIdentifier?.startsWith('chebi:') || node.chebiIdentifier !== undefined ) - .map((node) => (node.chebiIdentifier || node.standardIdentifier)!.split(':')[1]) + // An identifier without its "chebi:" prefix has no id after one. + .flatMap((node) => { + const [, id] = (node.chebiIdentifier || node.standardIdentifier)!.split(':'); + return id === undefined ? [] : [id]; + }) ) ), })), @@ -314,7 +322,7 @@ export class DiagramService { const eventIdToSubPathwayId = new Map( (graph.subpathways?.flatMap((subpathway) => subpathway.events - .map((event) => [event, subpathway.dbId]) + .map((event): [number, number] => [event, subpathway.dbId]) .filter((entry) => subpathwayIds.has(entry[1])) ) as [number, number][]) || [] ); @@ -331,10 +339,15 @@ export class DiagramService { ); const mappingList: [number, Graph.Node][] = graph.nodes .flatMap((node) => { - if (node.children?.length === 1 && node.diagramIds?.length !== 1) { + const [onlyChild] = node.children ?? []; + if ( + node.children?.length === 1 && + onlyChild !== undefined && + node.diagramIds?.length !== 1 + ) { // Consider homomer complex like their constituents for interactors return node.diagramIds - ?.map((id) => [id, dbIdToGraphNode.get(node.children[0])]) + ?.map((id) => [id, dbIdToGraphNode.get(onlyChild)]) .filter((entry) => entry[1] !== undefined) as [number, Graph.Node][]; } else { return node.diagramIds?.map((id) => [id, node]) as [number, Graph.Node][]; @@ -376,7 +389,8 @@ export class DiagramService { //compartment nodes const compartmentNodes: cytoscape.NodeDefinition[] = diagram?.compartments.flatMap((item) => { - const propToRects = (prop: Prop): { [p: string]: number } => ({ + const sides = ['left', 'top', 'right', 'bottom'] as const; + const propToRects = (prop: Prop): Record<(typeof sides)[number], number> => ({ left: scale(prop.x), top: scale(prop.y), right: scale(prop.x + prop.width), @@ -386,9 +400,9 @@ export class DiagramService { const innerCR = 10; let outerCR; if (item.insets) { - const rects = [propToRects(item.prop), propToRects(item.insets)]; - outerCR = Object.keys(rects[0]).reduce( - (smallest, key) => Math.min(smallest, Math.abs(rects[0][key] - rects[1][key])), + const [outer, inner] = [propToRects(item.prop), propToRects(item.insets)]; + outerCR = sides.reduce( + (smallest, key) => Math.min(smallest, Math.abs(outer[key] - inner[key])), Number.MAX_SAFE_INTEGER ); outerCR = innerCR + Math.min(outerCR, 100); @@ -490,7 +504,8 @@ export class DiagramService { let unitId = undefined; if (item.schemaClass === SchemaClasses.POLYMER) { const polymerGraphNode = dbIdToGraphNode.get(item.reactomeId)!; - const unitGraph = dbIdToGraphNode.get(polymerGraphNode.children[0])!; + // A unit the graph does not list reads as one it does not know, as before. + const unitGraph = dbIdToGraphNode.get(polymerGraphNode.children[0] ?? -1)!; const unitClass = this.nodeTypeMap.get(unitGraph.schemaClass) || this.nodeTypeMap.get( this.schemaClassToNodeTypeMap.get( @@ -756,11 +771,15 @@ export class DiagramService { const linkEdges: cytoscape.EdgeDefinition[] = diagram.links ?.filter( (link) => - !link.renderableClass.includes('EntitySet') || link.inputs[0].id !== link.outputs[0].id + !link.renderableClass.includes('EntitySet') || link.inputs[0]?.id !== link.outputs[0]?.id ) ?.map((link) => { - const source = idToNodes.get(link.inputs[0].id)!; - const target = idToNodes.get(link.outputs[0].id)!; + const [input] = link.inputs; + const [output] = link.outputs; + // As before, a link with no end cannot be drawn; now it says which. + if (!input || !output) throw new Error(`Link ${link.id} has no input or no output.`); + const source = idToNodes.get(input.id)!; + const target = idToNodes.get(output.id)!; const sourceP = scale(source.position); const targetP = scale(target.position); @@ -781,14 +800,13 @@ export class DiagramService { if (link.isDisease) classes.push('disease'); const isBackground = link.isFadeOut || - (idToNodes.get(link.inputs[0].id)?.isBackground && - idToNodes.get(link.outputs[0].id)?.isBackground); + (idToNodes.get(input.id)?.isBackground && idToNodes.get(output.id)?.isBackground); return { data: { id: link.id + '', - source: link.inputs[0].id + '', - target: link.outputs[0].id + '', + source: input.id + '', + target: output.id + '', ...relativeSegments, ...this.getEndpointData(sourceP, targetP, from, to), isFadeOut: link.isFadeOut, @@ -1056,10 +1074,10 @@ export class DiagramService { classes: [this.pick(reactionTypes), 'reaction'], })); - const nodes: cytoscape.NodeDefinition[] = physicalEntities.flatMap((node, i) => [ - node, - reactions[i], - ]); + const nodes: cytoscape.NodeDefinition[] = physicalEntities.flatMap((node, i) => { + const reaction = reactions[i]; + return reaction ? [node, reaction] : [node]; + }); const inOut: cytoscape.EdgeDefinition[] = physicalEntities.flatMap((_node, i) => [ { diff --git a/projects/reactome-diagram-element/tsconfig.app.json b/projects/reactome-diagram-element/tsconfig.app.json index 28bf5940..88902e4e 100644 --- a/projects/reactome-diagram-element/tsconfig.app.json +++ b/projects/reactome-diagram-element/tsconfig.app.json @@ -2,7 +2,10 @@ "extends": "../../tsconfig.json", "compilerOptions": { "outDir": "../../out-tsc/reactome-diagram-element", - "types": [] + "types": [], + // Every index read checked (#345). Not yet in tsconfig.json: the workspace + // libraries have findings left, held at a count by scripts/check-indexed-access.mjs. + "noUncheckedIndexedAccess": true }, "files": ["src/main.ts"], "include": ["../pathway-browser/src/**/*.d.ts", "../../src/**/*.d.ts"] diff --git a/scripts/check-indexed-access.mjs b/scripts/check-indexed-access.mjs index dd23adb6..00d8e0d2 100644 --- a/scripts/check-indexed-access.mjs +++ b/scripts/check-indexed-access.mjs @@ -1,7 +1,12 @@ #!/usr/bin/env node /** * Hold the `noUncheckedIndexedAccess` findings at today's count, file by file, - * until there are none and the flag can go into tsconfig.json (#345). + * in the projects that do not have the flag yet (#345). + * + * The app and the embeddable diagram reached zero and have it in their own + * tsconfig.app.json, so their builds enforce it. What is left is the workspace + * libraries, listed in PROJECTS; when they reach zero too, the flag goes into + * tsconfig.json for everything and this check goes. * * With the flag, `arr[i]` and `record[key]` are typed `T | undefined`, which is * the honest type: an index out of range or a missing key really is undefined, @@ -14,9 +19,10 @@ * Moving or renaming a file with findings shows as an increase under its new * path. Check the total is unchanged, then run with --update. * - * It runs the project the build uses (tsconfig.app.json, as check-types does) - * with the flag added. A run that fails without naming a single finding is a - * crash, not a clean result, and is refused. + * It runs each project's own build config with the flag added. A run that + * fails without naming a single finding is a crash, not a clean result, and is + * refused. (tsconfig.spec.json is left out: it cannot resolve vitest/globals, + * as scripts/check-types.mjs says.) * * node scripts/check-indexed-access.mjs check against the baseline * node scripts/check-indexed-access.mjs --update record today's counts @@ -30,7 +36,10 @@ import { isEntry } from './is-entry.mjs'; const ROOT = join(dirname(fileURLToPath(import.meta.url)), '..'); const BASELINE = join(ROOT, 'indexed-access-baseline.json'); -const PROJECT = 'tsconfig.app.json'; +const PROJECTS = [ + 'projects/reactome-table/tsconfig.lib.json', + 'projects/reactome-gsa-form/tsconfig.lib.json', +]; /** `file(line,col): error TS…` lines, counted per file. */ export function countErrors(/** @type {string} */ output) { @@ -64,25 +73,35 @@ export function outcome(status, output) { const total = (/** @type {Record} */ counts) => Object.values(counts).reduce((a, b) => a + b, 0); -function main() { +/** One project's findings per file, or exits when tsc did not get that far. */ +function countProject(/** @type {string} */ project) { const result = spawnSync( 'npx', - ['tsc', '--noEmit', '--pretty', 'false', '-p', PROJECT, '--noUncheckedIndexedAccess'], + ['tsc', '--noEmit', '--pretty', 'false', '-p', project, '--noUncheckedIndexedAccess'], { cwd: ROOT, encoding: 'utf8', maxBuffer: 64 * 1024 * 1024 } ); const output = `${result.stdout ?? ''}${result.stderr ?? ''}`; const run = outcome(result.status, output); if (!run.ok) { console.error( - `tsc did not get as far as the type check (exit ${result.status}), so it has nothing to count:` + `tsc did not get as far as the type check of ${project} (exit ${result.status}), so it has nothing to count:` ); console.error(output.trim()); process.exit(1); } + return run.counts; +} - const counts = Object.fromEntries( - Object.entries(run.counts).sort(([a], [b]) => a.localeCompare(b)) - ); +function main() { + /** @type {Record} */ + const merged = {}; + for (const project of PROJECTS) { + // A file two projects compile is counted once. + for (const [file, n] of Object.entries(countProject(project))) { + merged[file] = Math.max(merged[file] ?? 0, n); + } + } + const counts = Object.fromEntries(Object.entries(merged).sort(([a], [b]) => a.localeCompare(b))); if (process.argv.includes('--update')) { writeFileSync( BASELINE, @@ -107,7 +126,7 @@ function main() { for (const { rule: file, was, now } of increased) console.error(` ${file}: ${was} -> ${now}`); console.error( `\nGuard the new index reads in those files: an index out of range or a missing key is` + - ` undefined. See them with:\n\n npx tsc --noEmit -p ${PROJECT} --noUncheckedIndexedAccess\n\n` + + ` undefined. See them with:\n\n${PROJECTS.map((project) => ` npx tsc --noEmit -p ${project} --noUncheckedIndexedAccess`).join('\n')}\n\n` + `A file that was only moved or renamed shows here under its new path: if the total` + ` (${total(counts)} now, ${total(baseline)} before) has not gone up, run with --update.\n` ); @@ -122,7 +141,9 @@ function main() { } else { console.log(`Unchecked indexed access: ${now}, unchanged from the baseline.`); } - if (now === 0) console.log('None left: add noUncheckedIndexedAccess to tsconfig.json.'); + if (now === 0) { + console.log('None left: add noUncheckedIndexedAccess to tsconfig.json, and remove this check.'); + } } if (isEntry(import.meta.url)) main(); diff --git a/tsconfig.app.json b/tsconfig.app.json index 8886e903..a8c19fe1 100644 --- a/tsconfig.app.json +++ b/tsconfig.app.json @@ -4,7 +4,10 @@ "extends": "./tsconfig.json", "compilerOptions": { "outDir": "./out-tsc/app", - "types": [] + "types": [], + // Every index read checked (#345). Not yet in tsconfig.json: the workspace + // libraries have findings left, held at a count by scripts/check-indexed-access.mjs. + "noUncheckedIndexedAccess": true }, "files": ["src/main.ts"], "include": ["src/**/*.d.ts"] From eb43ad1cf8bc52ca07426cc7e80e2f6f4138aa97 Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Mon, 12 Oct 2026 07:00:23 +0000 Subject: [PATCH 2/2] Keep pick's undefined, refuse a link with no ends by name, test the merge From review: - pick threw whenever the value it drew was undefined; one caller picks undefined on purpose. It throws only for an empty list now. - An EntitySet link with no ends was silently dropped by the filter rather than reaching the named error; it is kept, and refused below as stated. - closestToAverage over no positions threw before, as the diagram could not be laid out; it throws again, saying why, instead of placing a shadow at NaN. - tsconfig.json's comment said the flag was off. - The per-project merge in the check is exported and has a test. Co-Authored-By: Claude Opus 5.5 --- .../src/app/services/diagram.service.ts | 22 ++++++++++-------- scripts/check-indexed-access.mjs | 23 ++++++++++++------- .../check-indexed-access.spec.mjs | 17 +++++++++++++- tsconfig.json | 8 ++++--- 4 files changed, 49 insertions(+), 21 deletions(-) diff --git a/projects/pathway-browser/src/app/services/diagram.service.ts b/projects/pathway-browser/src/app/services/diagram.service.ts index 6063b690..fc1d9da0 100644 --- a/projects/pathway-browser/src/app/services/diagram.service.ts +++ b/projects/pathway-browser/src/app/services/diagram.service.ts @@ -64,8 +64,9 @@ const isFinitePoint = (point: Position | undefined | null): point is Position => const closestToAverage = (positions: Position[]): Position => { const average = avg(positions); const [first, ...rest] = positions; - // No positions has no closest; the average (of nothing) stands in for it. - if (!first) return average; + // A shadow over a sub-pathway with no events: as before, the diagram cannot + // be laid out, now saying why rather than reading a property of undefined. + if (!first) throw new Error('There are no positions to choose a closest from.'); let closest = first; let min = squaredDist(closest, average); for (const pos of rest) { @@ -171,9 +172,9 @@ export class DiagramService { } pick(values: T[]): T { - const value = values[this.random(0, values.length - 1)]; - if (value === undefined) throw new Error('There is nothing to pick from.'); - return value; + if (values.length === 0) throw new Error('There is nothing to pick from.'); + // In range, and undefined is a value some callers pick on purpose. + return values[this.random(0, values.length - 1)] as T; } private readonly COMPARTMENT_SHIFT = 35; @@ -769,10 +770,13 @@ export class DiagramService { }); const linkEdges: cytoscape.EdgeDefinition[] = diagram.links - ?.filter( - (link) => - !link.renderableClass.includes('EntitySet') || link.inputs[0]?.id !== link.outputs[0]?.id - ) + ?.filter((link) => { + const [input] = link.inputs; + const [output] = link.outputs; + // A link without both ends is kept, to be refused below by name. + if (!input || !output) return true; + return !link.renderableClass.includes('EntitySet') || input.id !== output.id; + }) ?.map((link) => { const [input] = link.inputs; const [output] = link.outputs; diff --git a/scripts/check-indexed-access.mjs b/scripts/check-indexed-access.mjs index 00d8e0d2..26b0bda7 100644 --- a/scripts/check-indexed-access.mjs +++ b/scripts/check-indexed-access.mjs @@ -70,6 +70,20 @@ export function outcome(status, output) { return { ok: true, counts }; } +/** + * Several projects' counts as one: a file two projects compile is counted + * once, at the larger of its counts. + * @param {Record[]} perProject + */ +export function mergeCounts(perProject) { + /** @type {Record} */ + const merged = {}; + for (const counts of perProject) { + for (const [file, n] of Object.entries(counts)) merged[file] = Math.max(merged[file] ?? 0, n); + } + return merged; +} + const total = (/** @type {Record} */ counts) => Object.values(counts).reduce((a, b) => a + b, 0); @@ -93,14 +107,7 @@ function countProject(/** @type {string} */ project) { } function main() { - /** @type {Record} */ - const merged = {}; - for (const project of PROJECTS) { - // A file two projects compile is counted once. - for (const [file, n] of Object.entries(countProject(project))) { - merged[file] = Math.max(merged[file] ?? 0, n); - } - } + const merged = mergeCounts(PROJECTS.map(countProject)); const counts = Object.fromEntries(Object.entries(merged).sort(([a], [b]) => a.localeCompare(b))); if (process.argv.includes('--update')) { writeFileSync( diff --git a/tools/indexed-access-check/check-indexed-access.spec.mjs b/tools/indexed-access-check/check-indexed-access.spec.mjs index e82d1518..68c3f49a 100644 --- a/tools/indexed-access-check/check-indexed-access.spec.mjs +++ b/tools/indexed-access-check/check-indexed-access.spec.mjs @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { countErrors, outcome } from '../../scripts/check-indexed-access.mjs'; +import { countErrors, mergeCounts, outcome } from '../../scripts/check-indexed-access.mjs'; // tsc's own output, as `tsc --noEmit` prints it without --pretty. const output = [ @@ -36,4 +36,19 @@ describe('the indexed-access count', () => { expect(outcome(134, 'FATAL ERROR: JavaScript heap out of memory').ok).toBe(false); expect(outcome(2, 'error TS5058: The specified path does not exist').ok).toBe(false); }); + + // The projects still counted are separate libraries, but a file two of them + // compiled would be reported by both: once is the truth. + it('counts a file two projects compile once, at its larger count', () => { + expect( + mergeCounts([ + { 'a.ts': 2, 'b.ts': 1 }, + { 'b.ts': 3, 'c.ts': 1 }, + ]) + ).toEqual({ + 'a.ts': 2, + 'b.ts': 3, + 'c.ts': 1, + }); + }); }); diff --git a/tsconfig.json b/tsconfig.json index 571416a9..86ece533 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -51,9 +51,11 @@ // Strictness here is a deliberate trade: each of these was measured against // the codebase before being switched on. strictTemplates cost 83 fixes and // was worth it; typeCheckHostBindings, strictStandalone and the checks below - // all cost nothing. noUncheckedIndexedAccess (185 errors) and - // exactOptionalPropertyTypes (72) are not on -- worth revisiting, but not - // free. noUncheckedIndexedAccess was trialled properly rather than dismissed: + // all cost nothing. exactOptionalPropertyTypes (72) is not on -- worth + // revisiting, but not free. noUncheckedIndexedAccess is on for the app and the + // embeddable diagram, in their own tsconfig.app.json (#345), and held at a + // count for the workspace libraries by scripts/check-indexed-access.mjs until + // it can go here. It was first trialled and set aside: // of its 185 findings, roughly 44% are places the code already checks .length // or iterates a bounded loop and TypeScript simply cannot connect the check to // the access, where the only "fix" is a non-null assertion that documents