chore(typecheck): compile the build scripts and root configs too - #184
Merged
Conversation
`yarn typecheck` ran two projects, `tsconfig.backend.json` and
`tsconfig.renderer.json`, and both are scoped to `src`. Everything outside it
— `scripts/`, `forge.config.ts`, `vite.*.config.ts`, `vitest.config.ts`,
`drizzle.config.ts` — was compiled by nothing. Fifteen files, several of them
run in CI and in the release pipeline, where a type error surfaces as a
release that half-happened rather than as a red check.
`tsconfig.tooling.json` covers them: node types, no DOM lib, `noEmit`, strict
like the other two. It is referenced from `tsconfig.json` and named on the
`typecheck` command line.
Turning it on was the failing gate, and it found two real ones:
scripts/generate-icons.ts(35,9): error TS2868: Cannot find name 'Bun'.
scripts/mysql-seed.test.ts(192,21): error TS2339: Property 'cause' does
not exist on type 'void | Error'.
The first is `await Bun.write(icoPath, icoBuffer)` in a script that already
imports `fs` and uses it three times. The global was reachable only because
nothing typechecked the file; the script still runs under `bun`, which
implements `node:fs`, so `writeFileSync` is the same write without a type
dependency on the runtime.
The second is a `.catch` that widens the awaited value to `void | Error`,
because the call it guards resolves with `void` on the path where it does not
reject. Reading `.cause` off it was a `TypeError` waiting for the day the pipe
stopped rejecting — and the case would then have failed for the wrong reason,
reporting a missing property rather than a command that stopped failing. An
`invariant` names what the case is actually assuming.
Both are the sort of thing a third project catches once and then goes on
catching, which is the point. What it cannot catch is a fourth file, or a
fourth project, quietly falling outside it — so `scripts/tsconfig-coverage.ts`
and its test hold that down:
- Every `*.ts`/`*.tsx` file `git ls-files` names is resolved by one of the
projects `tsconfig.json` references. The projects are read from that block
rather than listed in the test, and their file lists come from
`tsc --showConfig`, which resolves the include globs and prints the answer
without compiling — the compiler's own answer, not a reimplementation of
its glob rules. The failure names the files and says which project to put
them in.
- The projects `tsconfig.json` references are the projects the `typecheck`
script runs. Being referenced is not being checked: the references block
is what the editor reads, and CI runs a separate command line that has to
name each project again. A fourth project added to one and not the other
reads as covered while nothing compiles it.
- The tooling project still checks the way this message says it does. Each
of its three overrides is invisible to everything else in the repository:
without `strict` a build script can dereference an undefined value,
without the narrowed `lib` it can reference `document` and compile against
a DOM that is not there at runtime, and without `noEmit` a bare
`tsc -p tsconfig.tooling.json` writes into `dist`. All three pass every
typecheck and every other case here, so they are read back out of the
config the compiler resolved. Only the tooling project, deliberately:
`tsconfig.backend.json` inherits the base `dom` too and has the same
problem, but changing what the backend compiles against is a separate
change with its own failures to work through.
An empty "uncovered" list is what a healthy repository looks like and also
what a guard that has stopped reading anything looks like, so a third case
asserts each project really resolved the file it exists for
(`src/server/runtime.ts`, `src/app/App.tsx`, `scripts/seed.ts`) and a fourth
that the repository list is non-empty.
Both spellings of a project path mean the same project.
`tsc -p ./tsconfig.backend.json` typechecks exactly what
`tsc -p tsconfig.backend.json` does, and the references block writes the `./`
form, so a script written that way is correct — but read literally it made the
guard red and say the script and the references "disagree about which projects
exist", which would be a false accusation and a bad half hour. `normalize` already existed one function up
for precisely this, on file paths; it is exported now and both comparisons go
through it.
TDD. Red was `tsc --noEmit -p tsconfig.tooling.json` on the two errors above,
watched before either fix. Red for the coverage guard was 17 files, reached by
writing the test against a `tsconfig.json` that did not yet reference the
tooling project — the 15 above plus `scripts/tsconfig-coverage.ts` and its own
test, which are themselves uncovered until the project exists. Red for the
script-versus-references case was `typecheckedProjects is not a function`, and
for the project-path spelling `expected ['./tsconfig.backend.json'] to deeply
equal ['tsconfig.backend.json']`.
The three tooling-project overrides have no red of that kind — the config was
already right, and the case exists because deleting an override was a mutant
that survived. Each was watched failing by deleting the option it names.
Mutation: 14 defects, all killed.
reference to tsconfig.tooling.json removed covers every file
typecheck script stops naming it are the projects it runs
normalize keeps the ./ prefix reads a resolved path
normalize keeps the backslash reads a windows separator
typecheckedProjects keeps the ./ prefix reads a project named with a prefix
tooling project stops including scripts/ covers every file
uncoveredFiles answers unsorted names what it found in one order
uncoveredFiles walks the projects instead names a file once
typecheckedProjects reads only -p reads the long spelling
typecheckedProjects requires whitespace reads the joined spelling
the guard reads no files from any project reads the files each resolved
tsconfig.tooling.json drops strict checks the tooling project
tsconfig.tooling.json drops its lib checks the tooling project
tsconfig.tooling.json drops noEmit checks the tooling project
Two of those came out of writing the list rather than running it. A `new Set`
around the result could not dedupe anything — the function makes one pass over
the repository, so it cannot emit a file twice — and it was hiding the
per-project mutant, which now dies. And nothing exercised the backslash in
`normalize`: neither producer emits one today, but a guard that answered "add
every file in the repository to a project" over a separator would be a bad
failure to debug, so the case says a separator is a separator.
Restoring `Bun.write`, or dropping the `invariant`, is caught by
`tsc -p tsconfig.tooling.json` and by nothing in vitest — which is the arrangement
this commit exists to create, not a gap in it.
Closes #161
Contributor
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
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.
Closes #161.
yarn typecheckrantsconfig.backend.jsonandtsconfig.renderer.json, and both are scoped tosrc. Fifteen files sat outside every project —scripts/(nine files),forge.config.ts,forge.env.d.ts,vite.main.config.ts,vite.preload.config.ts,vitest.config.ts,drizzle.config.ts— several of which run in CI and in the release pipeline, where a type error shows up as a release that half-happened rather than as a red check.tsconfig.tooling.jsonis the third project. Node types, no DOM lib,noEmit, strict like the other two; referenced fromtsconfig.jsonand named on thetypecheckcommand line.What turning it on found
await Bun.write(...)in a script that already importsfsand uses it three times — the global was reachable only because nothing typechecked the file. It still runs underbun, which implementsnode:fs, sowriteFileSyncis the same write without a type dependency on the runtime.The second is a
.catchwidening the awaited value tovoid | Error. Reading.causeoff it was aTypeErrorwaiting for the day the pipe stopped rejecting — and the case would then have failed for the wrong reason, reporting a missing property rather than a command that stopped failing. Aninvariantnames what the case actually assumes.The guard
A third project catches those two and goes on catching them. What it cannot catch is a fourth file, or a fourth project, quietly falling outside it.
scripts/tsconfig-coverage.tsand its test hold that down on three properties:*.ts/*.tsxfilegit ls-filesnames is resolved by some referenced project. File lists come fromtsc --showConfig, which resolves the include globs and prints the answer without compiling — the compiler's own answer rather than a reimplementation of its glob rules.yarn typecheckruns. Being referenced is not being checked; CI runs a separate command line that has to name each project again.stricta script can dereference an undefined value, without the narrowedlibit can referencedocumentand compile, and withoutnoEmita baretsc -p tsconfig.tooling.jsonwrites intodist.An empty "uncovered" list is what a healthy repository looks like and also what a guard that has stopped reading anything looks like, so separate cases assert each project really resolved the file it exists for and that the repository list is non-empty.
Review
A fresh-context review verified every claim in the commit message independently — reproduced both type errors at the same line and column, ran the real
generateIcobody underbunto confirm thewriteFileSyncswap is byte-identical, and re-ran all ten mutants. It raised two should-fixes, both taken:typecheckedProjectsdid not normalize the./prefix. Writing the script astsc -p ./tsconfig.backend.json— the same spelling the references block uses, and a command line that typechecks fine — turned the guard red with an assertion message that was simply wrong.normalizealready existed one function up for exactly this on file paths; it is exported now and both comparisons go through it.tsconfig.tooling.json's compiler options were guarded, and three are load-bearing. Deletingstrict,lib, ornoEmitsurvivedtscand all fifteen tests. They are now read back from the resolved config, and all three mutants die.Mutation testing: 14 defects, no survivors.
Follow-up, deliberately not here
tsconfig.backend.jsoninheritsdomfrom the base config too, so backend code can referencedocumentand compile. Same shape of problem, but changing what the backend compiles against is a separate change with its own failures to work through.Gates
tsc -p tsconfig.backend.jsontsc -p tsconfig.renderer.jsontsc -p tsconfig.tooling.json--checkDatabaseExplorer.tsx:19)sqlite-adapter.test.tsWindows path, pre-existing onmain