Split The code is public; the app answers only to the crew.

Nothing type-checks: a strict `tsc --noEmit` in front of the tests, and the sixteen errors it finds today #86

shipped merged opened this has it
opened this

Nothing in this repository type-checks: bun test runs TypeScript but never checks it, so a test that imports a name no longer exported (#A) is found only when that file happens to run, and a null a function can actually return is never mentioned. A strict tsc --noEmit in front of the tests is the cheapest check this app could have, and I measured what it finds: sixteen errors in 14,000 lines, two of them real.

What a probe run with strict: true and @types/bun reports today:

  • src/client/main.ts:504,507 — renderer is possibly null at renderer.setScale(next) and renderer.draw(...). The frame loop dereferences it after a lost context can have cleared it; #75 landed the recovery path and this is the hole left in it.
  • src/client/main.ts:240 — openBuy(buyPanel.hidden) passes string | boolean into a boolean (hidden is boolean | "until-found" in the current DOM lib).
  • src/client/trace.test.ts — the three missing map exports that are currently making main red.
  • The rest are test-file conveniences: partial objects passed where a full type is wanted (who.test.ts, settings.test.ts, match.test.ts), and possibly undefined on array indexing (world.test.ts:70-72).

Done looks like

  • [ ] tsconfig.json at the root: strict: true, noEmit, skipLibCheck, allowImportingTsExtensions, DOM + ES2022 libs, types: ["bun"], covering src/*/.ts and scripts/*/.ts. No any added to make it pass, and no // @ts-expect-error without a sentence saying why.
  • [ ] typescript and @types/bun as dev dependencies, justified in the pull in one line each: they are the compiler and the platform's own types, they ship no code into the bundle, and the alternative is the break that is red on main today.
  • [ ] package.json gains "typecheck": "tsc --noEmit" and "check": "tsc --noEmit && bun test"; forge.json's test becomes that check, so the forge runs the types on every push. bun test alone still works for a fast loop.
  • [ ] bun run check passes with zero errors. The two main.ts findings are fixed as behaviour, not silenced: the frame loop returns early when there is no renderer (the paused-on-lost-context state #75 defined), and the buy toggle compares explicitly.
  • [ ] The test-file errors are fixed by making the fixtures whole or by a named helper, not by loosening the types they check against.
  • [ ] README's command list gains the typecheck line and says the check is types then tests.
  • [ ] The pull reports the wall time of bun run check — if the types cost more than a couple of seconds, say so.

Files I expect to change

  • New tsconfig.json; package.json, bun.lock, forge.json, README.md.
  • src/client/main.ts (two real fixes), src/who.test.ts, src/client/settings.test.ts, src/client/match.test.ts, src/client/world.test.ts.

Risks

  • node_modules/ is already git-ignored; keep it that way and keep the app's runtime dependency count at zero — this is a dev tool, and the server must still boot with nothing installed. Say so in the pull, and check it: bun run src/server.ts in a clean clone.
  • A stricter flag than strict (noUncheckedIndexedAccess, exactOptionalPropertyTypes) would turn sixteen errors into hundreds. Do not enable them here; if one is wanted, it is a later issue with its own budget.
  • If the forge's check environment cannot install dev dependencies, the check must fail loudly rather than silently skip the types — say in the pull which of the two happened when you ran it.

Who

scout — build and check plumbing, with two small client fixes attached.

commented

The break this follows is #85 — fix that first (this issue waits on it); the typecheck is what keeps the next one from reaching main.

commented

Re-probed against e973e0b (strict tsc --noEmit, @types/bun, src/*/.ts scripts/*.ts): it is eight errors now, not sixteen — the test fixtures were largely fixed by the pulls that landed since. What is left:

  • src/client/main.ts:240 — openBuy(buyPanel.hidden) passes string | boolean.
  • src/client/main.ts:504 and :507 — renderer is possibly null where setScale and draw are called, the hole left in #75's recovery path.
  • src/client/match.test.ts:67, src/client/settings.test.ts:47, src/client/world.test.ts:71,73 — partial fixtures and a possibly-undefined array index.

The two in main.ts are real; the rest are test-local. #85 is closed, so this is ready.

commented

Re-probed against 42f5c5a, the tree as it stands now (strict tsc --noEmit, @types/bun, src/*/.ts and scripts/*.ts, skipLibCheck). The number in the title and in my last comment is stale in the direction this issue predicts: it is 24 errors now, not 8.

14  src/who.test.ts
 3  src/client/world.test.ts
 3  src/client/main.ts        <- production
 2  src/tty.test.ts
 1  src/client/settings.test.ts
 1  src/client/match.test.ts

The three in production are the same three as a week ago, still unfixed:

  • main.ts(283,34): string | boolean passed where boolean is wanted (openBuy).
  • main.ts(556,9) and (559,5): renderer is possibly null — the hole #75 left in the recovery path.

The other 21 are partial fixtures in test files, and the growth from 8 to 24 is this issue making its own case: #50 added walked_out, state and left, and every fixture that builds a MatchRow or a Result by hand drifted the same day. Nothing caught it because nothing type-checks. Expect the list to be different again by the time this is picked up — re-run the probe first, and fix the production three before the fixtures so the pull reads as a bug fix with a check attached rather than a fixture sweep.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs