Type-check main.d.ts in CI - #45
Merged
Merged
Conversation
`main.d.ts` is the published `types` entry, but nothing verified it. `npm test` only runs `node --test` over the JS, so the declarations could drift from `main.js` without anything failing. `src/main.test-d.ts` exercises the declared API at compile time and never runs — `tsc` failing to compile it is the failure. The `@ts-expect-error` directives are load-bearing in both directions, since a declaration that gets looser stops erroring and then fails on the unused directive. Verified by mutating `main.d.ts` six ways: widening the return type to `boolean`, making `body` optional, retyping `leewayMinutes`, adding an index signature, widening the error `name`, and dropping the error class from the namespace. All six fail the check. The type test is excluded from the published package. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Only `body`'s requiredness and `leewayMinutes`'s type were negatively tested, so `url`, `secret`, and `header` could turn optional — and all four required options could widen to `unknown` or `any` — with zero diagnostics. Each option now gets its own omission case and its own wrong-type case, one directive per property, so a single option drifting can't hide behind another. The wrong-type cases are what catch a widening: the call stops erroring and `tsc` fails on the unused directive. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
main.d.tsis the publishedtypesentry, but nothing verified it —npm testonly runsnode --testover the JS, so the declarations could drift frommain.jswithout anything failing. AGENTS.md asks for the two to be kept in sync, which until now was a manual promise.src/main.test-d.tsexercises the declared API at compile time and never runs;tscfailing to compile it is the failure. Each option's omission and each option's type get their own@ts-expect-error, one directive per property, so a single option drifting can't hide behind another. Those directives are load-bearing in both directions — if a declaration ever gets looser, the line stops erroring andtscfails on the unused directive. That's what catches a widening tounknownorany.To check the gate actually bites rather than just passing green, I mutated
main.d.ts18 ways: each of the four required options made optional, widened tounknown, and widened toany;leewayMinutesretyped and made required; the return type widened toboolean; an index signature admitting arbitrary keys; the errornamewidened off its literal; and the error class dropped from the namespace. All 18 fail the check.An earlier revision of this PR claimed a six-mutation check passed clean. That was true but misleading — those six happened to be mutations the file already covered. Review correctly caught that
url,secret, andheadercould turn optional, and that all four required options could widen, with zero diagnostics. Now fixed, and the mutation matrix covers the declared surface rather than a sample of it.typescriptis the only new dependency and it's dev-only, so the package stays zero-dependency at runtime. The type test is excluded from the tarball —npm pack --dry-runstill lists justpackage.json,error.js,main.d.ts, andmain.js.On the filename:
.test-d.tsrather thanmain.d.test.tsormain.test.d.ts, because TypeScript treats any.d.<something>.tsas a declaration file, which makes the helper body and the@ts-expect-errorassertions illegal in an ambient context.🤖 Generated with Claude Code