Cut typecheck wall time with TypeScript project references [DO NOT MERGE] - #503
Conversation
|
Merge-order hazard between this PR and the other one touching PR #503 (CL-7226) recomputes the content hash for all 14 PR #508 (CL-7242) adds a new migration and a schema change under Each PR computed its hash from a tree that does not contain the other's changes. They will conflict textually on the Whichever of these merges second must recompute rather than resolve by hand: then verify with Recording this rather than leaving it to be discovered as a red check after merge. |
ca1e586 to
b06a5fe
Compare
Covers reference derivation from real workspace deps (dropping devDependencies), the src/test tsconfig split, dependency-cycle exclusion, shared-root-script exclusion, and --check drift detection.
Every package tsconfig now sets composite/emitDeclarationOnly/outDir and declares references generated from its real workspace dependencies, so tsc --build consumes dependents' declarations instead of re-checking their full transitive source on every package's own tsc invocation. scripts/generate-tsconfig-references.ts derives those references from package.json (not devDependencies), excludes real dependency cycles and their transitive consumers from the composite graph via Tarjan's algorithm plus cascading exclusion (a composite project cannot put a non-composite dependency in references, and half-excluding just the cycle members left their consumers pulling that dependency's raw source into their own program), and writes a sibling tsconfig.test.json per package so test files -- which routinely reach a shared test harness that itself imports production code -- can't put a package in a reference cycle with itself. tsc --build runs against a single generated solution file (tsconfig.build.json) rather than every composite tsconfig.json as separate command-line roots: the latter was tried first and produces wrong results, because tsc --build shares source-file/diagnostic state across sibling root arguments and can misattribute a rootDir violation from one project's graph to an unrelated one in the same invocation. A single root project does not have this problem and still builds the whole graph in dependency order, skipping whatever is already up to date. tsconfig.test.json also needs an explicit rootDir at the workspace root: with outDir inherited from the composite src config but no explicit rootDir, TypeScript infers rootDir from include alone and TS6059s on any file a test reaches outside src/test -- which a shared test helper living outside every package's own directory always is. Cold full typecheck: ~118s, down from over 10 minutes (exceeded the 600s command cap). check:tsconfig-references (wired into check:structural) fails if a tsconfig drifts from the generator's output.
…dant scripts/affected.ts guessed which packages a change could break from package.json edges, with a GLOBAL_PATHS fallback for the cases it couldn't reason about. It existed only to make bun run check affordable by skipping most of the workspace, because every package's tsc invocation re-checked the full transitive source of its dependencies. tsc --build now reads the real dependency graph and does its own incremental up-to-date checks, so the guess is no longer needed and was already the weaker of the two: it narrows from package.json edges, tsc --build narrows from the files that actually changed. WORKBENCH_CHECK_SINCE and its CI wiring go with it. Measured before removing it from the test job: a full bun run scripts/run-all.ts test across every workspace package takes about 118s, comfortably under the 600s command-cap this repo works under -- so losing that narrowing does not reintroduce the problem it was working around. WORKBENCH_CHECK_CONCURRENCY stays: it sizes the worker pool that fans package scripts out across cores, which has nothing to do with guessing affected packages, and scripts/run-all.test.ts already exercises it directly. The duplicated parsing/validation between run-all.ts and typecheck.ts moved to scripts/concurrency.ts in the previous commit.
The project-references cutover edited every vendor/intx/* tsconfig.json (composite, references, a sibling tsconfig.test.json), the same shape every other workspace package got in this change. Vendored trees carry a content hash of their own; update it and note the delta in VENDORED.md so a future diff against upstream isn't mistaken for drift nobody explained.
check:killdates hashes each vendored directory's content to catch a silent edit, and already excluded node_modules and *.tsbuildinfo as install/build artifacts. It missed dist/: once a composite package's tsc --build run writes declaration output there, the hash changes even though nothing was actually edited, and the very next check:killdates run reports every vendored package as drifted. CI caught this on CL-7226's project-references PR because it was the first change to ever make these packages emit dist/ output.
…tion
ESLint's projectService (like an editor's language server, or a bare
tsc invocation in a package directory) discovers a project by looking
for a file literally named tsconfig.json -- it has no way to also
check a same-purpose file under a different name. The composite
src-only project used to be tsconfig.json itself, with the combined
src+test project at tsconfig.test.json; every composite package's
test files were therefore invisible to project discovery ("was not
found by the project service"), and ESLint had no built-in project to
fall back to for them either (allowDefaultProject disallows the ** glob
that would be needed for arbitrarily-nested test files).
Swapping the names fixes this with no ESLint-specific configuration at
all: tsconfig.json is now the combined project (extends
tsconfig.src.json, adds test) that every tool finds on its own, and
tsconfig.src.json is the composite project that only tsc --build and a
composite dependent's own references need to know exists (by an
explicit path, e.g. "../dep/tsconfig.src.json" -- a project reference
target is never a bare directory once the composite file isn't named
tsconfig.json).
This also fixes the crash CI reported on the previous version of this
branch (SIGABRT, heap OOM): the underlying cause was ESLint's type
checker resolving cross-package imports through raw source instead of
compact .d.ts files, because no build had run yet in the standalone
lint job and every composite package's test files (unable to resolve a
project) still needed full type information somehow. Two changes fix
it together: `bun run lint` now builds the composite graph
(tsc --build tsconfig.build.json) before invoking eslint, so cross-
package types resolve through declarations; and this rename means
every file -- test included -- resolves to the project the language
service actually expects, instead of falling back to loading whichever
package's raw source it can find. Measured peak eslint memory: ~7GB
before either fix, ~4.2GB after both (a genuinely cold run, no
.eslintcache). NODE_OPTIONS=--max-old-space-size=6144 gives the lint
step headroom above that measured peak -- Node's default old-space
ceiling sits close enough to 4.2GB that a busier CI runner could still
tip over it.
Two more fixes needed along the way:
- withSrcFields hardcoded "../../tsconfig.base.json" as the composite
project's extends path. vendor/intx/* sits one directory deeper than
every other workspace root, so the fixed string resolved to a path
that doesn't exist there; the extends silently failed, dropping
skipLibCheck (among everything else tsconfig.base.json sets) and
surfacing hundreds of node_modules library type errors that had
nothing to do with this change. Computed from the package's own
directory depth instead.
- combinedConfigFor rebuilt every package's `include` from a fixed
["src", "test"] list, silently dropping packages/e2b-sandbox-sidecar's
`template/` directory (sandbox assets read at runtime, not src or
test). Carries forward any include entry beyond src/test from the
package's existing tsconfig.json now, the same way the composite
side already preserved package-specific compilerOptions.
vendor/intx/*'s tsconfig content changed again with the tsconfig.src.json rename (and the extends-path fix it needed); scripts/checks/kill-dates.txt's recorded hash has to move with it or check:killdates reports every vendored package as edited-without- recording. VENDORED.md's delta note updated to describe the current shape.
…CONCURRENCY instead of re-reading process.env
b06a5fe to
9a970c4
Compare
Summary
CL-7226: typecheck across 115 workspace packages took over 10 minutes
(exceeds the 600s command cap), because none of the 86 package tsconfigs
used TypeScript project references — every package's
tscre-checked thefull transitive source of every dependency. Two mandatory parts, per the
ticket's scope-expansion comment:
tsc --build, withreferencesgenerated from real workspace dependencies by a script.
slowness —
scripts/affected.ts,WORKBENCH_CHECK_SINCE— cuttingover cleanly, no fallback path left beside the new one.
DO NOT MERGE — pushed for CI and review per instruction.
What changed
composite: true,emitDeclarationOnly: true,outDir: dist,tsBuildInfoFile: dist/tsconfig.tsbuildinfo, andreferencesgeneratedfrom its own
package.jsondependencies(neverdevDependencies) byscripts/generate-tsconfig-references.ts.bun run check:tsconfig-references(wired into
check:structural) fails on drift.excludes its members from the composite graph, and — this is load-bearing,
not cosmetic — that exclusion cascades to every package that depends on
an excluded package, even transitively. Half-excluding just the cycle
members leaves their consumers pulling the excluded dependency's raw
source directly into their own composite program, which TypeScript flags
as a
rootDirviolation once it walks into whatever that raw sourceitself imports. 27 of 115 packages fall back to the pre-change
tsc -p tsconfig.json --noEmitpath as a result (6 real cycle members +1 direct importer of a shared test harness + 20 cascaded transitive
consumers, including
apps/hubandapps/web). Breaking that cycle isout of scope here (mechanical tsconfig cutover only, no product-code
refactors) but is the natural next ticket to capture the rest of the win.
tsconfig.test.json(non-composite,src+test, same references) because test files routinely reach ashared, non-package test harness (
scripts/e2e/harness.ts) that itselfimports production code — joining the composite graph directly would put
a package in a reference cycle with itself.
tsc --buildruns against a single generated solution file(
tsconfig.build.json,{files: [], references: [...]}to everycomposite project), not against every composite tsconfig.json as
separate CLI roots. The latter was tried first and produces wrong
results: passing ~88 unrelated root projects to one
tsc --buildinvocation shares source-file/diagnostic state across them, and
misattributes a
rootDirviolation from one project's dependency graphto a completely unrelated project in the same invocation (verified:
isolated single/pair-project builds were clean, the full 88-root
invocation was not, the single-solution-file invocation is clean with
zero errors).
tsconfig.test.jsonneeded an explicitrootDir(computed as therelative path to the repo root) — with
outDirinherited from thecomposite src config but no explicit
rootDir, TypeScript infersrootDirfromincludealone and TS6059s on any file a test reachesoutside
src/test, even undernoEmit.scripts/affected.ts,scripts/affected.test.ts,WORKBENCH_CHECK_SINCE(including its.github/workflows/ci.ymlwiring on both the
typecheckandtestjobs).WORKBENCH_CHECK_CONCURRENCYstays — it sizes the worker pool that fans package scripts across cores,
unrelated to guessing affected packages — extracted into a shared
scripts/concurrency.tsused by bothrun-all.tsandtypecheck.ts.VENDORED.mdandscripts/checks/kill-dates.txtupdated for thevendor/intx/*tsconfig edits (content hashes recomputed;check:killdateshashes the vendored tree and would otherwise flag drift). No upstream
Interchange repo touched.
CI-cost tradeoff, stated plainly
Removing
WORKBENCH_CHECK_SINCEfrom thetestjob means every PR nowruns the full test suite instead of only the affected slice. Measured
before landing this:
bun run scripts/run-all.ts testacross everyworkspace package takes ~118s, comfortably under the 600s cap — so this
is not a regression in practice. Accepted deliberately, matching the
ticket's explicit "nothing anywhere still needs to know about 'affected
packages' as a concept."
Benchmarks (this machine, other lanes concurrently active — load average noted per run)
tsc --noEmitWORKBENCH_CHECK_SINCEnarrowed typecheck (packages/chat change, 34/115)bun run typecheck, coldbun run typecheck, warm no-opbun run typecheck, single widely-depended package edit (packages/error-sink)bun run scripts/run-all.ts test"Before" full-typecheck/narrowed figures are the ticket's own pre-existing
measurements (pinned at
00bab807); "after" figures are freshly measured onthis branch just now. Load was never idle during these runs (other lanes
active) — numbers are reported honestly rather than cherry-picked from a
quiet window.
Type errors surfaced and how they were resolved
tsc --buildinitially surfaced 408TS6059(rootDir) errors, all oneclass, none a real type-strictness finding:
references,but their transitive consumers weren't, so those consumers pulled the
excluded package's raw source (and whatever that imports) straight
into their own program. Fixed by propagating exclusion to a fixpoint
over the dependency graph (down to 253 errors).
tsc --buildmisattribution — passing 88 compositetsconfigs as separate CLI roots in one invocation. Fixed by building a
single generated solution file instead (down to 253... same 253, this
was a different bug than Notifications: approvals, failures, and mentions as durable mail #1, both needed fixing).
tsconfig.test.jsonmissing an explicitrootDir— the last 253,all under the separate
tsc -p tsconfig.test.json --noEmitfallbackchecks, not
tsc --builditself. Fixed with a per-package computedrootDirpointing at the repo root.Zero errors remain.
exactOptionalPropertyTypesand every othertsconfig.base.jsonbase option are unchanged — nothing was loosened tomake errors disappear.
A drift-detection bug was also found and fixed along the way:
testFieldsMatchin the generator never comparedcompilerOptions, so achange to
tsconfig.test.json's generated compiler options would silentlynever get written to existing files or flagged by
--check.Unmet criteria / follow-ups
including
apps/hubandapps/web) still run the pre-changenon-composite
tsc --noEmitfallback. Breaking the cycle(
hub-client/connections/inference-settings/webhook-triggers/
workflow-catalog/folded-runs) would lettsc --buildcover themtoo — worth a follow-up ticket; out of scope here (product-code refactor).
cl-7188-add-local-pre-push-gates...) addsscripts/git-hooks/pre-push, which setsWORKBENCH_CHECK_SINCE=origin/mainbefore running lint/typecheck/test locally. Not touched here (different
branch, not yet merged) — once it rebases past this change, that env var
is inert (nothing reads it anymore) but should be removed from the hook
script in that PR to avoid a dangling reference.
Interchange defects
None found.
vendor/intx/*tsconfig edits are in-repo build tooling only;no upstream Interchange source was touched.
Verification
bun run check:structural— green (includes the newcheck:tsconfig-references)bun run scripts/typecheck.ts— 0 errors, ~110–118s coldscripts/generate-tsconfig-references.test.ts— 12/12 pass (includesregression coverage for the transitive-cascade exclusion)
scripts/run-all.test.ts— 13/13 passcheck:killdates,check:licenses— greenUpdate: CI failures fixed (lint SIGABRT + check:killdates)
CI reported two failures on the first push, both real consequences of
the project-references change, neither visible in local verification
because I'd correctly avoided the slow root-level runs:
1.
bun run lint— SIGABRT (heap OOM), exit 134.Root cause, found by reproducing locally: the composite
tsconfig.jsonexcludes a package's test files (required for
tsc --build's DAGconstraint — see the commit for why), but ESLint's
projectServiceonly ever discovers a project by looking for a file literally named
tsconfig.json; it has no way to also checktsconfig.test.json.Every composite package's test files were invisible to project
discovery ("was not found by the project service"), and with no build
having run yet (lint is a standalone CI job), the type checker fell
back to resolving every cross-package import through raw source
instead of compact
.d.tsfiles — for every file in the repo, in onelong-lived process. Measured peak: ~7GB, comfortably past what a
default Node heap (or a typical CI runner) can give it.
Fix, not a memory bump alone:
tsconfig.src.jsonand madetsconfig.jsonthe combined (src+test) project again, so every toolthat discovers a project by convention — ESLint, an editor, a bare
tscin the package directory — finds it without any ESLint-specificconfiguration.
tsc --buildand a composite dependent'sreferencesnow point at the renamed file by explicit path.
bun run lintnow runstsc --build tsconfig.build.jsonbeforeprettier/eslint, so cross-package type resolution goes throughdeclarations instead of raw source.
Together these bring a genuinely cold peak (no
.eslintcache) down to~4.2GB. That's the real, measured, bounded cost of type-aware
linting across 115 packages with declarations prebuilt — architecturally
correct, not a workaround. It's still close enough to typical default
Node heap ceilings that a busier CI runner could tip over it, so
NODE_OPTIONS=--max-old-space-size=6144gives the lint step headroomabove the measured peak. This is the "if you say why and give the
number" case: the number is 4.2GB measured, 6144MB is the number
raising it to.
Two more bugs surfaced and got fixed doing this:
withSrcFieldshardcoded a"../../tsconfig.base.json"extends paththat doesn't resolve for
vendor/intx/*(one directory deeper thanevery other workspace root) — the silently-broken extends dropped
skipLibCheckand surfaced ~280 unrelated node_modules library typeerrors. Computed from the package's actual directory depth now.
includewas rebuilt from a fixed["src", "test"]list, silently droppingpackages/e2b-sandbox-sidecar'stemplate/directory (sandboxassets, not src or test). Now carries forward any extra
includeentry from the package's existing tsconfig, with a regression test.
2.
check:killdates— 14 violations.Not a blanket regenerate: each violation was individually real —
hashDirectoryhashes a vendored directory's full tree but onlyexcluded
node_modulesand*.tsbuildinfo, notdist/. The hash I'drecorded was computed while
dist/(declaration output from a priortsc --buildrun) was sitting on disk locally; a fresh CI checkout hasno
dist/(gitignored), so its hash differed — for exactly the 14vendor/intx/*rows, matching the 14 violations precisely. FixedhashDirectoryto also excludedist/, added a regression testmirroring the existing
.tsbuildinfoone, verified the hash is nowstable both with and without
dist/present, and recomputed thecorrect hash (twice more, honestly, as the tsconfig rename above
changed vendored content again in the same session).
Verification after both fixes, all in the foreground:
bun run scripts/typecheck.ts: 0 errors, ~130s coldbun run lint: exit 0, 8 warnings (all pre-existing, confirmedagainst
00bab807, unrelated to this change), ~51sbun run check:killdates: ok, stable with and withoutdist/builtbun run check:structural: greenPR #472 (open, unmerged, sets
WORKBENCH_CHECK_SINCEin a newpre-push hook) is untouched, per the coordinator's note that it's
already tracked there.