feat(FR-3214): exclude pnpm store from the dev type-check watcher to cut per-process watch FDs#8357
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has required the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
There was a problem hiding this comment.
Pull request overview
Reduces dev-server file descriptor usage by excluding the immutable pnpm store from TypeScript’s watch program.
Changes:
- Generates a machine-specific checker tsconfig during development.
- Falls back safely to the standard tsconfig.
- Ignores the generated config in Git.
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
react/vite.config.ts |
Generates and applies the pnpm-store watch exclusions. |
.gitignore |
Ignores the generated checker tsconfig. |
…cut per-process watch FDs Stacked on the process-group kill fix. The group kill stops orphaned watch trees from accumulating; this change additionally shrinks each live dev server's footprint by ~4,500 FDs: vite-plugin-checker's TS watch program installs a per-file kqueue watcher on every dependency .d.ts in the immutable pnpm store. dev.mjs-side vite config now derives a gitignored react/tsconfig.checker.json with watchOptions.excludeDirectories/excludeFiles pointing at the absolute store root and points the checker at it. Type-checking completeness, CI, build, and HMR are unaffected (see PR description for measurements). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ar6Rr2uEGLPBXcWNpASXDD
35067ae to
f4ce9bf
Compare
agatha197
left a comment
There was a problem hiding this comment.
Mechanism looks correct and I verified the injection locally (generated an equivalent tsconfig.checker.json; tsc --showConfig carries watchOptions with the absolute store glob, inherits the base paths workaround, and the program file set is unchanged — so type-checking stays complete and only the watchers are dropped). The FD reduction itself is macOS/kqueue-specific and I can't reproduce it on Linux, so that rests on your live lsof measurements, which look thorough.
Requesting one change (the stale-file cleanup) plus two optional nits inline. None are blocking correctness — happy to see it merge once the cleanup is addressed or consciously waived.
| // store-resolution / write failure → no-op, original behavior preserved). | ||
| function resolveCheckerTsconfigPath(storeRoot: string | undefined): string { | ||
| const baseTsconfig = resolve(__dirname, 'tsconfig.json'); | ||
| if (!storeRoot) { |
There was a problem hiding this comment.
Requested change — clean up a stale derived config on the degrade path. If an earlier serve wrote tsconfig.checker.json and a later run resolves no store root (pnpm store lookup fails, or a flat npm/yarn install), we return the base tsconfig but leave the previously-generated file on disk. It's gitignored and unreferenced so it's harmless, but the checker silently keeps reading a stale exclude config across runs. Suggest unlinking it here before returning:
if (!storeRoot) {
try { rmSync(resolve(__dirname, 'tsconfig.checker.json'), { force: true }); } catch {}
return baseTsconfig;
}| // TS watchOptions globs use forward slashes, even on Windows. | ||
| const storeGlob = `${normalizePath(storeRoot)}/**`; | ||
| try { | ||
| writeFileSync( |
There was a problem hiding this comment.
Nit (optional) — side-effecting file write during config evaluation. writeFileSync runs as a side effect of evaluating the defineConfig factory (the plugins array), so it fires on every config load/reload rather than as part of the dev-server lifecycle. It works, but writing a file while resolving config is a little surprising. Consider moving the generation into a small plugin configResolved/buildStart hook so the write is tied to an explicit lifecycle step. Non-blocking.
| extends: './tsconfig.json', | ||
| watchOptions: { | ||
| excludeDirectories: [storeGlob], | ||
| excludeFiles: [storeGlob], |
There was a problem hiding this comment.
Nit (optional). excludeFiles is matched against files, whereas <storeRoot>/** is a directory-subtree glob already covered by excludeDirectories above — so this line is redundant belt-and-suspenders. It's harmless (and your measurements confirm the exclude works), just noting it in case you'd rather keep excludeFiles for an actual file pattern or drop it.

Related to #8047 (FR-3214) — the issue itself is resolved by #8156 (process-group kill); this PR is the complementary footprint reduction, split out per review discussion.
Problem
Even with orphan leaks fixed in #8156, each live dev server still holds far more file descriptors than it needs. Measured on a live dev stack: a single
vitedev process holds ~5,894 open file descriptors, of which ~4,481 are regular files inside the pnpm store (immutable dependency.d.tstype declarations, ~3,840 of them).The holder is vite-plugin-checker's TypeScript watch program (
ts.createWatchProgram), which installs a per-file watcher on every file in the program — including every dependency.d.tsin the store. On macOS each per-file watch costs 1 open FD, so:maxfiles= 256 → instant EMFILE.Fix
Tell the checker's TS watch program not to watch the pnpm store via
watchOptions.excludeDirectories/excludeFiles.Two constraints shape the implementation:
**/-globs under the projectbasePath(combinePaths()), so they can never match the store realpath which lives outside the repo (~/Library/pnpm/store/v11/...). Measured:**/node_modules/**,**/store/**etc. all had zero effect. Since the absolute path is machine-specific, it cannot live in the committedtsconfig.json.ts.createWatchCompilerHostwithout awatchOptionsToExtendarg, so the only injection point is the tsconfig file it reads.Therefore
react/vite.config.tsnow generates a gitignoredreact/tsconfig.checker.jsonat dev startup —{ extends: './tsconfig.json', watchOptions: { excludeDirectories/excludeFiles: ['<absolute store root>/**'] } }— and pointschecker({ typescript: { tsconfigPath } })at it. The store root reuses the existingpnpmStorePath(frompnpm store path).Why excluding the store from the watch is safe
watchOptionsexcludes only drop the file watcher, not the read. TS still reads every dependency.d.tsto build the program — verified: error count identical before/after (real-stack boot: "Found 0 errors" both ways; in an error-injection harness, 296 → 296).scripts/verify.share untouched. Those run one-shottsc --noEmit(no--watch), which ignoreswatchOptionsentirely — and this PR does not modify the committedreact/tsconfig.jsonat all. The exclude lives only in the gitignored derived file that the dev-mode checker reads. Every gate that actually enforces type safety is unaffected.vite buildis untouched.pnpmStorePathis only resolved whencommand === 'serve'; on build the helper returns the plain base tsconfig without writing anything (a one-shot build installs no file watchers anyway).backend.ai-ui/backend.ai-clientresolve via tsconfigpathsto../packages/*/src— project-internal source outside the store — so they keep normal per-file watch and instant re-check.tsconfig.json— original behavior, no exclude.The only theoretical gap: a dependency
.d.tschanging while dev is running, with zero source edits won't auto-trigger a re-check. In practicepnpm install/add/updatetriggers Vite's dep re-optimization / dev restart (which re-reads the store), and using a new package requires a source edit — which is watched and re-triggers the checker. The store is content-addressed and immutable during normal dev, and CI re-checks everything regardless.Measurements (real stack,
lsofon the live vite pid)Also verified this is not a Vite regression by actually booting the last pure-craco commit (
00e8431a1) in a worktree and measuring the sametsc --watchprogram: craco-era = 6,870 FDs (6,055 in-store) vs current Vite = 5,894 — the Vite stack was already slightly lighter; the store-watch cost is inherent to running an in-process TS type-checker, and this PR removes it.Verification
pnpm run dev, wait for the checker's "Found 0 errors", thenlsofthe vite pid): parent branch without the exclude = 6,056 open REG files (4,566 under the pnpm store) vs this branch = 87 open REG files (2 under the store).tsconfig.checker.jsongenerated with the correct absolute store glob and confirmed gitignored; checker reports "Found 0 errors" in both runs.bash scripts/verify.sh—=== ALL PASS ===(Relay / Lint / Format / TypeScript), re-run on this stacked branch.tsconfig.checker.jsongenerated correctly, confirmed gitignored, checker reports "Found 0 errors", FD table above.react/src/index.tsxvia atomic save while dev was running → checker reported it within ~2s (source watch intact); reverting cleared it.🤖 Generated with Claude Code