chore(ts): TypeScript hardening — strict flags, Go-native tsgo, noUncheckedIndexedAccess#207
Merged
Conversation
Pragmatic TS hardening (deps untouched): - tsconfig: enable noImplicitReturns + noUnusedParameters (both strict, near-zero churn). Skipped the high-churn/low-value flags for now (noPropertyAccessFromIndexSignature ~261 errs); noUncheckedIndexedAccess (~234) and exactOptionalPropertyTypes (~89) are left for follow-up PRs. - Broaden `include` to cover plugins' lib/ and components/ (+ top-level plugin files) — previously only backend.ts/app.tsx/panel.tsx entry files were checked, so lib code was only validated transitively. Add an explicit `exclude` for test/spec, build, and .cache so this stays production-focused. - Adopt the Go-native TypeScript preview (@typescript/native-preview / tsgo), pinned exactly, as a second CI typecheck step. It's ~9x faster than tsc and slightly stricter — it caught a real generic-inference hole tsc missed. tsc stays the authoritative gate; tsgo adds fast coverage. New `typecheck:go` script for local use. Fixes surfaced by the above: - lsfg-vk InstallCard: `Select` inferred its generic as `string`, so the layer-version value flowed into a `LayerVersion` handler unchecked. Pin the generic explicitly (`Select<LayerVersion>`) — genuinely type-safe, no cast. - recomp/lib/state.ts: 6 vestigial `state` params (the helpers now read fresh state via mutateState) flagged by noUnusedParameters — `_`-prefixed. Audit note: the production `any`/suspect-cast surface is small (~51 across ~24 files) and almost entirely legitimate runtime/FFI boundaries (Electrobun CEF imports resolved at runtime, Steam-webhelper patching). The 8 production @ts-ignore are all on runtime-resolved electrobun/* imports. tsc: 0 · tsgo: 0 · eslint: 0 errors · 364 tests pass. Co-Authored-By: Claude Opus 4.8 <[email protected]>
…review Per follow-up: promote the Go-native compiler from a CI-only extra to the main typecheck everywhere, and address PR-review findings. - `typecheck` script now runs tsgo (primary); `tsc` moves to `typecheck:tsc` and is kept as a CI/release safety-net step since tsgo is still a -dev preview (can have false negatives) — belt-and-braces until it GAs. - ci.yml + release.yml: tsgo is the primary typecheck step, tsc the backstop. Documented that tsgo must be invoked via `bun run` (its bin shim is `#!/usr/bin/env node`; bun runs it with no node installed). Review findings folded in: - Coverage gap (Medium): the shipped recomp install recipes under plugins/*/games/**/*.ts were type-checked by nothing (loaded via a runtime-computed dynamic import, so not even transitively). Added plugins/*/games/** and plugins/*/scripts/** to `include` — both compilers pass with zero new errors. - Symmetry (Low): added **/*.spec.ts to `exclude` (only *.spec.tsx was listed). tsc: 0 · tsgo: 0 · frozen-lockfile clean. Co-Authored-By: Claude Opus 4.8 <[email protected]>
Turn on `noUncheckedIndexedAccess` — indexed access (arr[i], obj[key], regex groups, .split() parts, array destructuring) now yields `T | undefined`, catching a large class of latent "possibly undefined" bugs. Fixed all 323 resulting errors across ~90 files, properly (no suppressions): - Real guards / early-returns / `continue` where a value can genuinely be missing (parser rows, device lookups, empty collections). - `?? fallback` where a sensible default exists. - Non-null `!` ONLY where provably in-bounds — after a `.length` check, inside `0..len-1` loops, on required regex capture groups, or on `findIndex` results — each annotated with a short justifying comment. No `any` / `as any` / `as unknown as` / `@ts-ignore` introduced; the one pre-existing React-fiber `as unknown as` in qam.ts was preserved as-is. Runtime behavior unchanged (guards only fire on inputs that would previously have thrown or produced undefined downstream); parser test suites still pass. tsgo: 0 · tsc: 0 · eslint: 0 errors · 364 tests pass. Co-authored-by: Claude Opus 4.8 <[email protected]>
srsholmes
changed the title
…allbacks Review feedback on the noUncheckedIndexedAccess pass: ~159 of the fixes were bare `!` assertions — compile-time-only claims that insert no runtime check, so a wrong (or later-invalidated) assumption still flows `undefined` silently downstream. Rework them into runtime-safe patterns, preserving exact behavior for all real inputs: - Bitwise/boolean numeric reads → `?? 0` (behavior-identical: `undefined & mask` already coerces to 0; HID report bytes, evdev bitmaps, capability bits, gamepad axes). - Required regex capture groups → `?? ""` (group is always defined on a match; the fallback is dead code that removes the assertion). - "Must exist" elements (length-checked [0], findIndex/splice results, Promise.all index-aligned rows) → hoisted guarded locals that log and degrade with the SAME action the surrounding code already used (continue/return/throw-the-same-error), so an impossible-today miss becomes a diagnosable warning instead of a silent undefined. - Loop-bounded indexing → restructured to for...of / .entries() / paired iteration where it reads cleanly (removes the assertion entirely). - injector.ts: `this.cdp!` sites → a `requireCdp()` accessor; definite- assignment `let tab!` → explicit `| undefined` with a post-loop guard. Bare `!` reads remaining in the diff vs main: 2 (DAY_NAMES[date.getDay()]!, provably 0-6 into a 7-element const, commented). Definite-assignment declarations (`let x!: T`) kept where they're the standard sync-Promise- executor pattern. tsgo: 0 · tsc: 0 · eslint: 0 errors · 364 tests pass. Co-Authored-By: Claude Fable 5 <[email protected]>
Review finding on the noUncheckedIndexedAccess pass: parseVdf used to throw on brace-underflowed (corrupt/truncated) VDF, and callers like playtime used that throw as their malformed-file signal (skip the user). The added guards made it silently return a partial parse instead — indistinguishable from a complete one. Keep the lenient parse (more robust than crashing), but log a one-shot warning when underflow is detected so a partial result is diagnosable. Co-Authored-By: Claude Fable 5 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
End-to-end TypeScript hardening. Dependencies untouched (deferred by request). Everything below applies equally to
tscand the Go-nativetsgo; both are green, as are eslint (0 errors) and the full test suite (364 pass).1. Stricter compiler flags
Enabled, with near-zero churn:
noImplicitReturnsnoUnusedParameters(surfaced 6 vestigialstateparams inrecomp/lib/state.ts→_-prefixed)noUncheckedIndexedAccess— the big one: indexed access (arr[i],obj[key], regex groups,.split()parts, array destructuring) now yieldsT | undefined.Skipped
noPropertyAccessFromIndexSignature(~261 errors, low value).exactOptionalPropertyTypes(~89) is deferred to a follow-up.2. Broadened typecheck coverage
includepreviously only covered plugin entry files (backend.ts/app.tsx/panel.tsx), so pluginlib/,components/,games/, andscripts/code was only checked transitively — and the shipped recomp install recipes (plugins/recomp/games/**/setup.ts, loaded via a runtime-computed dynamic import) weren't checked by anything. Now all of it is, with an explicitexcludefor tests/build/.cache.3. Go-native
tsgoas the primary typecheckAdopted
@typescript/native-preview(tsgo), pinned exactly. ~9× faster thantsc(0.9s vs 8.6s) and slightly stricter — it caught a real generic-inference holetscmissed (lsfg-vk/InstallCard; fixed with an explicitSelect<LayerVersion>).typecheckrunstsgoeverywhere (local, CI, release gate);tscis retained as atypecheck:tscsafety-net step whiletsgois a-devpreview. Must be invoked viabun run(its bin shim needs node otherwise) — documented in the workflows.4. The 323
noUncheckedIndexedAccesserrors — fixed twice overFirst pass fixed all 323 across ~90 files. Review feedback (correctly) flagged that ~159 of those were bare non-null
!assertions — compile-time-only claims that insert no runtime check. A second pass reworked them into runtime-safe patterns, preserving exact behavior:?? 0(behavior-identical:undefined & maskalready coerces to 0 — HID report bytes, evdev bitmaps, gamepad axes).?? ""(group always defined on a match; fallback is dead code).[0],findIndex/spliceresults,Promise.allindex-aligned rows) → hoisted guarded locals that log and degrade with the same action the surrounding code already used, so an impossible-today miss becomes a diagnosable warning instead of a silentundefined.for...of/.entries()/ paired iteration.injector.ts:this.cdp!sites → arequireCdp()accessor; definite-assignmentlet tab!→ explicit| undefined+ post-loop guard.Bare
!reads remaining vs main: 2 (DAY_NAMES[date.getDay()]!, provably 0–6 into a 7-element const, commented). Noany/as any/@ts-ignoreintroduced anywhere.Correctness review — two adversarial rounds
!provably safe, every guard fires only where old code crashed.requireCdp()rework (no error-type/message branching anywhere). One real finding:parseVdfhad gone from throw to silently partial-parsing corrupt (brace-underflowed) VDF, which could have understated playtime instead of skipping a corrupt user file. Fixed: the lenient parse stays, but underflow now logs a one-shot warning so a partial result is diagnosable.On-device verification (OneXPlayer APEX)
Built + installed: all 29 plugins load with zero errors; TDP boot-restore and the InputPlumber wake-button reload work; manual pass over the highest-churn areas (fan-curve editing, Steam CEF injection/badges, launch options, playtime, storage cleaner, flatpak, steamgriddb art, RGB) — all working. Known pre-existing issue #119 (right-stick scroll) unchanged by this PR.
Deferred (follow-ups)
exactOptionalPropertyTypes(~89 errors)Checks
tsgo0 ·tsc0 · eslint 0 errors · 364 tests pass ·bun install --frozen-lockfileclean · CI green.🤖 Generated with Claude Code