Skip to content

Check every index read in the app and the embed: the flag is on - #416

Merged
adamjohnwright merged 2 commits into
mainfrom
fix/345-guard-diagram
Oct 12, 2026
Merged

adamjohnwright merged 2 commits into
mainfrom
fix/345-guard-diagram

Conversation

@adamjohnwright

@adamjohnwright adamjohnwright commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Part of #345. This is the last of the app's noUncheckedIndexedAccess findings. The flag is now on in tsconfig.app.json and projects/reactome-diagram-element/tsconfig.app.json, so the builds enforce it.

The two files

  • render.component.ts (62): the figure renderer's geometry. Most findings came from a few reads (a chord's ends, a polygon's previous vertex, a segment's ends, the arrow's tip). Each now has a guard that can't fire on real data, because every index is in range by construction. The four-edge clip loop gets a pair type. Exports take const [instance] = instances.
  • diagram.service.ts (23): closest-to-average, pick, the normal-pathway id, CHEBI ids, sub-pathway pairs, homomer children, compartment insets (now over a typed list of sides, in the same order as Object.keys gave), polymer units, link ends, and the legend's node/reaction pairing.
  • No non-null assertions were added (lint is at its baseline).

Templates

tsc doesn't check templates, but with the flag on, Angular's strict template checking does. It found five more:

  • Four passed a sample's expression value (exp[i], values[i]) to cr-expression-tag, which isn't built for a missing one (a GSA label lookup and the colour scale would break). Those cells now read the value with @let and draw the tag only when there is one. The index is in range in practice, so nothing changes on screen.
  • The fifth, the found table's resource column id, falls back to the same entities-{resource} id it's computed from.

Behaviour

  • One stated change: a diagram link with no input or output already threw on link.inputs[0].id. It now throws Link {id} has no input or no output.
  • CHEBI ids: an identifier without a chebi: prefix used to add undefined to the set of ids looked up. It's now left out, so the page no longer asks EBI for /compound/NaN/structure/.
  • After review: pick throws only for an empty list (one caller picks undefined on purpose); an EntitySet link with no ends reaches the named error instead of being filtered out; closestToAverage over no positions throws, as it did, with a message, instead of placing a shadow at NaN.

The check moves on

scripts/check-indexed-access.mjs now counts the projects that don't have the flag yet: the workspace libraries reactome-table (66) and reactome-gsa-form (18), 84 findings in 10 files. When they reach zero, the flag goes into tsconfig.json and the check goes. (tsconfig.spec.json stays out: it can't resolve vitest/globals, as check-types.mjs notes.)

Tests

  • App config proven: an added xs[0].length fails check:types with TS2532.
  • Builds: npm run build and npm run build:embed with the flag: both complete.
  • Full e2e suite locally: 388 passed, plus the 6 tests that skip themselves.
  • Unit tests: 538 passed.
  • Render tests: ran live against the render service in the pre-push hook: ok.
  • Gates: check:lint and check:indexed unchanged at the baseline.

🤖 Generated with Claude Code

adamjohnwright and others added 2 commits October 12, 2026 06:44
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 <noreply@anthropic.com>
…erge

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 <noreply@anthropic.com>
@adamjohnwright
adamjohnwright merged commit f6746c4 into main Oct 12, 2026
6 checks passed
@adamjohnwright
adamjohnwright deleted the fix/345-guard-diagram branch October 12, 2026 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant