Repository navigation
chore(types): add a typecheck script and @types/node - #164
Merged
Merged
Conversation
Run the compiler directly and enforce the check in CI; it surfaced two latent errors. • Add `@types/node` and a `typecheck` script (`tsc --noEmit`), run from CI and `test:prod`. • Add `"types": ["node"]` to `tsconfig.json` so Node globals resolve under TypeScript 7. • Fix the two `never`-narrowing errors in `TreeInterpreter`'s exhaustive operator switches; the runtime behaviour and diagnostic text are unchanged.
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.
Add a real typecheck gate.
npx tsc --noEmitwas the only way to typecheck, and it failed for two reasons: no@types/node, and two latentnever-narrowing errors inTreeInterpreter.• Add
@types/nodeand atypecheckscript (tsc --noEmit), and run it in CI after lint and intest:prod.• Add
"types": ["node"]totsconfig.json. Without it, TypeScript 7 does not pick up the Node globals (process,console,ReadableStream,TextDecoder,node:fs, …) even with@types/nodeinstalled.• Fix the two exhaustive-switch
defaultbranches inTreeInterpreter. TypeScript narrows the node toneveronce everyTokencase is handled, so readingnode.operatorfor the error message is a type error. The branch is unreachable; the assertion keeps the diagnostic text and changes no behaviour.These were real errors that the build never reported:
tsdownonly emits declarations for exported APIs, andcli.ts/Stream.tsare not typechecked unlesstscruns over the project.Verification
On Node.js 26.13.0:
npm run typecheck— passes.npm run lint:ci,npm run build,npm test— 1,380 tests passed; build artifacts test 21/21.npm run coverage— unchanged: statements 95.66%, branches 91.93%, functions 95.83%, lines 95.54%.npm audit— 0 vulnerabilities.Note for a follow-up
The depth-limit test (
enforces maxDepth option and defaults to DEFAULT_MAX_DEPTH) depends onvisit()'s stack-frame size. Hoistingnode.operatorinto a local made the 1,200-deep query overflow the JS stack before the 1,000 depth guard, which is why the fix keeps the access inline. That test would be more robust if it asserted the guard with a small explicitmaxDepthinstead of relying on native stack headroom.