Repository navigation
types(strict): strictNullChecks on for the six programs that can hold it - #530
Merged
Merged
Conversation
First tier of the null-safety work. Six of the ten programs are now clean under
strictNullChecks and declare it permanently: the three auxiliary Workers, cal
(src plus its Vitest suite), and the two node-runtime test programs.
`strict` STAYS OFF, and the split is the whole point rather than timidity.
Measured across all ten programs, deduped by file:line: noImplicitAny is 7,197
diagnostics and strictNullChecks is 1,544. They answer different questions.
noImplicitAny finds missing annotations on code that was written without types,
which is labour; strictNullChecks finds places a null can actually arrive.
Raising them together buries the second under the first, so this raises the
second alone.
NOTHING IS SILENCED. No `!`, no `as any`, no suppression: a non-null assertion
would tell the compiler something nobody verified, which is worse than leaving
the flag off. Two of the fixes are behavioural and both are improvements:
- cf-garage's DO seed read `parseInt(url.searchParams.get("seed"), 10) || 0`,
which reached 0 only because `parseInt(null)` coerces to NaN. `?? ""` gets
the same 0 by a route that is written down.
- cal's snapshot age computed `Number.isFinite(snap.ts) ? now - snap.ts : ...`,
where `isFinite` is not a type guard and cannot narrow `number | undefined`.
Reading it into a local and testing `!= null` keeps the meaning exactly.
Three tests gained real guards where a null would previously have thrown a bare
TypeError blaming the assertion rather than the fixture: Readability's `parse()`
answers null, and its `title` and `content` are independently nullable.
`assert.ok` does NOT narrow through a namespace import, so those are plain
throws that say which field was missing.
THE JSDOC TRAP AGAIN, in cal this time. `booking.ts` carried
`/** @type {Slot[]} */` from #521, inert since the file became .ts, so the array
inferred `never[]` the moment the flag came on. That is the third module to hit
it. Everything else in that class here is the same shape: an empty array literal
only evolves from its pushes while strictNullChecks is off, so eight
accumulators across cal, lwe-ask, lens-reader and crawl.ts needed their element
type stated. `cal/src/availability.ts` now exports the `Interval` type the
module actually traffics in rather than repeating the shape inline.
THE RATCHET is a contract test, and it only ever tightens. The six programs are
declared; removing the flag from one, or editing it out of the list, fails by
name. It is the mirror of config/ts-migration.json's "may only shrink". Control
run: dropping strictNullChecks from lwe-ask's config fails the test.
Deliberately text-based. Proving a program clean means RUNNING tsc, which
`bun run typecheck` already does for every one of these; a second compile per
program would put eight of them on a suite that answers in a second.
Remaining, for the tiers after this: 1,516 diagnostics in src/worker (506),
src/client (466), tools (369) and serendipity (96). 55% sit in ten files, with
src/client/lens.js alone carrying 325, so it stays tractable file by file.
typecheck (10 programs), lint, 354 contract tests, build, routes:check (142),
all three project suites (cal 59, cf-garage 8, lens-reader 15), and dry-runs for
all three auxiliary Workers. Bundles unmoved: cf-garage 3.07 KiB gzip, lwe-ask
27.29, site Worker unchanged.
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
aadhar-sh | 95fcd38 | Aug 24 2026, 01:26 AM |
Contributor
Wire-size diff vs merge base
Brotli q11, the quality the edge serves. Every number here is deterministic: Client assetsNo change. 22 files, 77.74 KiB Brotli total. PagesNo change. 50 files, 531.29 KiB Brotli total. Worker bundle237.85 KiB → 237.86 KiB gzip (+0.01 KiB, +0.0%) Largest module deltas (raw bytes in bundle):
Dictionary deltas (
|
oddharsh
added a commit
that referenced
this pull request
Aug 24, 2026
Second tier, and the seventh program to declare the flag. #530's ratchet test asserts every program setting strictNullChecks appears in its declared list; this adds tools/ to it. RATCHETED RATHER THAN CLEAN, which is the difference from tier 1. tools/ carried 395 diagnostics under strictNullChecks; 101 are fixed here and the remaining 292 are seeded into config/ts-tools-baseline.json, where check-tool-types.mjs has held a per-file count since it was written. That ratchet fails on a new file, on a file that got worse, and on a file that got BETTER without the baseline being updated, so the number is monotone. The alternative was leaving the flag off until all 395 were done, which protects nothing in the meantime. The seeded number is 292 rather than 294 because #531 landed while this was in flight and removed two of them. The ratchet caught that on the rebase, by its third arm: a file that got BETTER without the baseline being updated fails too, which is what keeps the number honest in both directions. Three files cleared outright: tools/lib/html-to-md.ts 45 → 0 talks/…/build-deck.mjs 30 → 0 tools/deploy-promote.mjs 26 → 0 THE .mjs AND .ts RULES ARE EXACT INVERSES, and getting this backwards costs a compile. A .ts file ignores JSDoc types; a .mjs file rejects a TypeScript annotation outright with TS8010. tools/ is 88 .mjs files, so nearly everything in this tier is JSDoc, and the three .ts files under tools/lib take annotations. Worth knowing before the next tier, since the same directory holds both. TYPING `die` AS `never` CLEARED A CLUSTER, and it is the more general lesson. deploy-promote.mjs uses `die()` as control flow throughout ("bail unless X"), and without `@returns {never}` TypeScript keeps the impossible branch alive and reports the value the caller just refused as still nullable. One annotation took that file from 11 remaining to 9, and the same shape is waiting in every other script with a bail helper. html-to-md.ts gained real types rather than annotations of convenience: `Token` as a discriminated union whose raw-text arm carries `raw` and `owner` together (they are one fact, and pairing them is what lets the tree builder drop a <script> body without proving the field is there), and `TextNode | ElementNode` with the stack declared as elements only, which is the invariant that makes `top().children` total. VERIFIED, because this converter publishes every Markdown twin. Built the twins with this branch's converter and with origin/main's, hashing all 58: identical at 568b28e024de910a957c6bc382e0a3e241764fd91e068297b68a1b1ede0ecbc9. And `deploy:promote --dry-run` runs end to end, resolves a target, and prints its freshness guard, which is the release path exercised rather than assumed. One behavioural addition, in deploy-promote: the sampling loop's result is read after the loop, so a zero SAMPLE_ATTEMPTS would have ramped to the next step having verified nothing. That now dies by name. It is a guard on a constant rather than on the network, and it is the one outcome the script exists to prevent. typecheck (10 programs), lint, 354 contract tests, build, and the site bundle at 237.34 KiB gzip.
This was referenced Aug 24, 2026
oddharsh
added a commit
that referenced
this pull request
Aug 24, 2026
Second tier, and the seventh program to declare the flag. #530's ratchet test asserts every program setting strictNullChecks appears in its declared list; this adds tools/ to it. RATCHETED RATHER THAN CLEAN, which is the difference from tier 1. tools/ carried 395 diagnostics under strictNullChecks; 101 are fixed here and the rest are seeded into config/ts-tools-baseline.json, where check-tool-types.mjs has held a per-file count since it was written. That ratchet fails on a new file, on a file that got worse, and on a file that got BETTER without the baseline being updated, so the number is monotone. The alternative was leaving the flag off until all 395 were done, which protects nothing in the meantime. THE SEEDED NUMBER IS 600 RATHER THAN THE 294 THIS BRANCH FIRST MEASURED, and the gap is #532 rather than a regression here. That PR converted 39 more tools scripts from .mjs to .ts while this was in flight, and JS-mode leniency withdraws with the extension, so the same code answers for more under the flag. The ratchet is what surfaced it: rebasing tripped both its arms, one file at a time. IT ALSO MADE THREE OF MY OWN FIXES INERT, which is the trap this migration keeps hitting and the fifth time it has fired. deploy-promote had its bail helper typed as never, and two nullable holders typed, all three in JSDoc. Correct while the file was .mjs and ignored the moment it became .ts: that file went 0 back to 37. All three are real annotations now, plus four accumulators the conversion newly exposed, and it is clean again. Worth a follow-up rather than widening this: 16 JSDoc type annotations across 5 tools .ts files on main are inert for exactly the same reason. Three files cleared outright: tools/lib/html-to-md.ts 45 → 0 talks/…/build-deck.mjs 30 → 0 tools/deploy-promote.mjs 26 → 0 THE .mjs AND .ts RULES ARE EXACT INVERSES, and getting this backwards costs a compile. A .ts file ignores JSDoc types; a .mjs file rejects a TypeScript annotation outright with TS8010. tools/ is 88 .mjs files, so nearly everything in this tier is JSDoc, and the three .ts files under tools/lib take annotations. Worth knowing before the next tier, since the same directory holds both. TYPING `die` AS `never` CLEARED A CLUSTER, and it is the more general lesson. deploy-promote.mjs uses `die()` as control flow throughout ("bail unless X"), and without `@returns {never}` TypeScript keeps the impossible branch alive and reports the value the caller just refused as still nullable. One annotation took that file from 11 remaining to 9, and the same shape is waiting in every other script with a bail helper. html-to-md.ts gained real types rather than annotations of convenience: `Token` as a discriminated union whose raw-text arm carries `raw` and `owner` together (they are one fact, and pairing them is what lets the tree builder drop a <script> body without proving the field is there), and `TextNode | ElementNode` with the stack declared as elements only, which is the invariant that makes `top().children` total. VERIFIED, because this converter publishes every Markdown twin. Built the twins with this branch's converter and with origin/main's, hashing all 58: identical at 568b28e024de910a957c6bc382e0a3e241764fd91e068297b68a1b1ede0ecbc9. And `deploy:promote --dry-run` runs end to end, resolves a target, and prints its freshness guard, which is the release path exercised rather than assumed. One behavioural addition, in deploy-promote: the sampling loop's result is read after the loop, so a zero SAMPLE_ATTEMPTS would have ramped to the next step having verified nothing. That now dies by name. It is a guard on a constant rather than on the network, and it is the one outcome the script exists to prevent. typecheck (10 programs), lint, 354 contract tests, build, and the site bundle at 237.34 KiB gzip.
oddharsh
added a commit
that referenced
this pull request
Aug 24, 2026
Second tier, and the seventh program to declare the flag. #530's ratchet test asserts every program setting strictNullChecks appears in its declared list; this adds tools/ to it. RATCHETED RATHER THAN CLEAN, which is the difference from tier 1. tools/ carried 395 diagnostics under strictNullChecks; 101 are fixed here and the rest are seeded into config/ts-tools-baseline.json, where check-tool-types.mjs has held a per-file count since it was written. That ratchet fails on a new file, on a file that got worse, and on a file that got BETTER without the baseline being updated, so the number is monotone. The alternative was leaving the flag off until all 395 were done, which protects nothing in the meantime. THE SEEDED NUMBER IS 600 RATHER THAN THE 294 THIS BRANCH FIRST MEASURED, and the gap is #532 rather than a regression here. That PR converted 39 more tools scripts from .mjs to .ts while this was in flight, and JS-mode leniency withdraws with the extension, so the same code answers for more under the flag. The ratchet is what surfaced it: rebasing tripped both its arms, one file at a time. IT ALSO MADE THREE OF MY OWN FIXES INERT, which is the trap this migration keeps hitting and the fifth time it has fired. deploy-promote had its bail helper typed as never, and two nullable holders typed, all three in JSDoc. Correct while the file was .mjs and ignored the moment it became .ts: that file went 0 back to 37. All three are real annotations now, plus four accumulators the conversion newly exposed, and it is clean again. Worth a follow-up rather than widening this: 16 JSDoc type annotations across 5 tools .ts files on main are inert for exactly the same reason. Three files cleared outright: tools/lib/html-to-md.ts 45 → 0 talks/…/build-deck.mjs 30 → 0 tools/deploy-promote.mjs 26 → 0 THE .mjs AND .ts RULES ARE EXACT INVERSES, and getting this backwards costs a compile. A .ts file ignores JSDoc types; a .mjs file rejects a TypeScript annotation outright with TS8010. tools/ is 88 .mjs files, so nearly everything in this tier is JSDoc, and the three .ts files under tools/lib take annotations. Worth knowing before the next tier, since the same directory holds both. TYPING `die` AS `never` CLEARED A CLUSTER, and it is the more general lesson. deploy-promote.mjs uses `die()` as control flow throughout ("bail unless X"), and without `@returns {never}` TypeScript keeps the impossible branch alive and reports the value the caller just refused as still nullable. One annotation took that file from 11 remaining to 9, and the same shape is waiting in every other script with a bail helper. html-to-md.ts gained real types rather than annotations of convenience: `Token` as a discriminated union whose raw-text arm carries `raw` and `owner` together (they are one fact, and pairing them is what lets the tree builder drop a <script> body without proving the field is there), and `TextNode | ElementNode` with the stack declared as elements only, which is the invariant that makes `top().children` total. VERIFIED, because this converter publishes every Markdown twin. Built the twins with this branch's converter and with origin/main's, hashing all 58: identical at 568b28e024de910a957c6bc382e0a3e241764fd91e068297b68a1b1ede0ecbc9. And `deploy:promote --dry-run` runs end to end, resolves a target, and prints its freshness guard, which is the release path exercised rather than assumed. One behavioural addition, in deploy-promote: the sampling loop's result is read after the loop, so a zero SAMPLE_ATTEMPTS would have ramped to the next step having verified nothing. That now dies by name. It is a guard on a constant rather than on the network, and it is the one outcome the script exists to prevent. typecheck (10 programs), lint, 354 contract tests, build, and the site bundle at 237.34 KiB gzip.
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
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.
First tier of the null-safety work. Six of the ten programs are now clean under
strictNullChecksand declare it permanently: the three auxiliary Workers, cal (src plus its Vitest suite), and the two node-runtime test programs.strictstays off, and that split is the pointMeasured across all ten programs, deduped by
file:line:noImplicitAnystrictNullChecksstrict(all eight)Raising them together buries the second under the first. This raises the second alone.
Nothing is silenced
No
!, noas any, no suppressions. A non-null assertion tells the compiler something nobody verified, which is worse than leaving the flag off.Two fixes are behavioural, both improvements:
parseInt(url.searchParams.get("seed"), 10) || 0— it reached0only becauseparseInt(null)coerces toNaN.?? ""gets the same0by a route that's written down.Number.isFinite(snap.ts) ? now - snap.ts : Infinity.isFiniteis not a type guard and can't narrownumber | undefined; reading into a local and testing!= nullkeeps the meaning exactly.Three tests gained real guards.
Readability.parse()answers null, and itstitleandcontentare independently nullable — previously a null would have thrown a bareTypeErrorblaming the assertion rather than the fixture. Noteassert.okdoes not narrow when reached through a namespace import, so these are plain throws naming which field was missing.The JSDoc trap, third module running
cal/src/booking.tscarried/** @type {Slot[]} */from #521 — inert since that file became.ts, so the array inferrednever[]the moment the flag came on.Everything else in this class is one shape: an empty array literal only evolves from its pushes while
strictNullChecksis off. Eight accumulators across cal, lwe-ask, lens-reader andcrawl.tsneeded their element type stated.availability.tsnow exports theIntervaltype the module actually traffics in, rather than repeating{start, end}inline six times.The ratchet
A contract test, and it only ever tightens: removing the flag from a declared program — or editing it out of the list — fails by name. Mirror of
ts-migration.json's "may only shrink". Control run: droppingstrictNullChecksfrom lwe-ask's config fails the test.Text-based deliberately. Proving a program clean means running tsc, which
bun run typecheckalready does for all of these; a second compile per program would put eight of them on a suite that answers in a second.What's left
1,516 diagnostics, in
src/worker(506),src/client(466),tools(369),serendipity(96). 55% sit in ten files, withsrc/client/lens.jsalone carrying 325, so the remaining tiers stay tractable file by file.Green
typecheck(10 programs),lint, 354 contract tests,build,routes:check(142), all three project suites (cal 59, cf-garage 8, lens-reader 15), and dry-runs for all three auxiliary Workers. Bundles unmoved: cf-garage 3.07 KiB gzip, lwe-ask 27.29, site Worker unchanged.