Fix @xnetjs/plugins/node losing its type declarations to a build race - #688
Fix @xnetjs/plugins/node losing its type declarations to a build race#688crs48 wants to merge 3 commits into
Conversation
…dle's types
`packages/plugins/tsup.config.ts` exported an array of two configs. tsup runs
array configs CONCURRENTLY, and the first config's `clean` covers all of
`dist/` — a strict superset of the second config's `dist/services/` output.
tsup cleans twice per config: `**/*` before the ESM phase, then
`**/*.d.{ts,mts,cts}` RECURSIVELY before the DTS phase (`cleanDtsFiles`).
Whichever of those landed after the node bundle had already written a file
silently deleted it, and the build still exited 0. The narrow window where the
first clean lands before the node bundle's JS write but the DTS clean lands
after its `.d.ts` write leaves exactly `dist/services/node.js` with no
`node.d.ts` beside it — a green, cacheable, truncated build.
Consumers then resolved `@xnetjs/plugins/node` to bare JavaScript and degraded
it to `any` (TS7016), which surfaced downstream as ~25 unrelated-looking TS7006
"implicitly has an 'any' type" errors on callback parameters across
`packages/cli/src/utils/{agent-remote,agent-local,vector-tier}.ts`. That made
`turbo typecheck --affected` fail in the pre-commit hook for reasons pointing at
innocent files, which pushes people toward `--no-verify`.
Split the node bundle into `tsup.node.config.ts` and run the two builds
sequentially, so a clean and a write can never interleave. Bundle contents are
unchanged: identical entry, format, externals and output sizes.
Verified: 5/5 consecutive builds emit both `.d.ts` files;
`pnpm --filter @xnetjs/cli typecheck` passes; `pnpm check:packaging` passes
(publint flags the missing declaration, so the poisoned state was reachable in
CI too); 833 plugins tests pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: xNet Test <test@xnet.dev>
|
✓ Changelog fragment found — thanks! |
|
Warning Review limit reached
Next review available in: 40 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Preview: https://xnet.fyi/pr/688/app/ |
1 participant
The symptom
pnpm --filter @xnetjs/cli typecheckfails with ~25TS7006/TS7016errors, pointing at callback parameters inpackages/cli/src/utils/{agent-remote,agent-local,vector-tier}.ts. Only one of those errors names the real fault:dist/services/node.jsexists;dist/services/node.d.tsdoes not.The cause
Not a missing
dts: true— that was already set on both entries, and theexportsmap was already correct. It is a race between the two tsup configs.packages/plugins/tsup.config.tsexported an array of two configs. tsup runs array configs concurrently, and config 1'scleancovers all ofdist/— a strict superset of config 2'sdist/services/output.tsup cleans twice per config:
**/*underoutDirbefore the ESM phase**/*.d.{ts,mts,cts}recursively underoutDirbefore the DTS phase (cleanDtsFiles,tsup/dist/index.js:1364)Whichever of those landed after the node bundle had written a file silently deleted it — and the build still exited 0. The window where clean #1 lands before config 2's JS write but clean #2 lands after its
.d.tswrite yields exactlynode.jswith nonode.d.ts: a green, cacheable, truncated build that Turbo then stores as a success.Concurrency is visible in the build log — config 2's build starts before config 1 cleans:
Proof that config 1's clean reaches config 2's output — running config 1 alone against a fully populated
dist:Downstream, TypeScript does not report "module not found" — it degrades the subpath to
any, so the failure surfaces as a pile of unrelated-looking errors in innocent files.turbo typecheck --affectedthen fails in the pre-commit hook for reasons that point nowhere near the actual problem, which pushes people toward--no-verify(forbidden byAGENTS.md). It is also the exact shapeAGENTS.mdnames under Errors: a truncated run that is indistinguishable from a completed one.The fix
Split the node bundle into
packages/plugins/tsup.node.config.tsand run the two builds sequentially (tsup && tsup --config tsup.node.config.ts), so a clean and a write can never interleave. Each config'scleannow only reaches its own output.Bundle contents are unchanged — identical entry, format, externals,
splitting, and output sizes (index.js521.53 KB,index.d.ts299.36 KB,node.js327.31 KB,node.d.ts85.94 KB).packages/pluginswas the only package with a nested-outDirtsup config (scanned all ofpackages/andapps/).Verification
pnpm --filter @xnetjs/plugins buildruns emit both.d.tsfilespnpm --filter @xnetjs/cli typecheckpassespnpm turbo run build typecheck— 112/112 tasks passpnpm check:packagingpasses; publint flags the missing declaration at--level error, confirming the poisoned state was reachable in CI tooOut of scope
pnpm check:api-reportfails onpackages/data/etc/data.api.mdandpackages/react/etc/react.api.md. Verified pre-existing on a cleanmainby stashing this branch's changes, rebuilding both packages, and re-running. Not touched here; tracked separately.🤖 Generated with Claude Code