perf(rolldown): build all DTS entries in a single rolldown call#151
Open
vitbokisch wants to merge 3 commits into
Open
perf(rolldown): build all DTS entries in a single rolldown call#151vitbokisch wants to merge 3 commits into
vitbokisch wants to merge 3 commits into
Conversation
Each per-entry DTS call instantiates the slow rolldown-plugin-dts (and its embedded TS compiler) from scratch — that's the [PLUGIN_TIMINGS] warning rolldown itself surfaces. Doing it in one call amortizes that setup AND lets common imports emit as a single _chunks/*.d.ts instead of being inlined into every entry's stub. Same architecture as PR #139's JS multi-entry grouping: partition DTS configs by output dir, run groups of >=2 entries in one rolldown call, keep single-entry packages on the existing buildDtsIsolated path. The "find largest .d.ts" trick that handles the plugin's <name>.d.ts stub + <name>NUMBER.d.ts real pair is preserved, just batched. Measured on repo's own DTS fixtures: dts-pipeline 675ms -> 380ms (1.78x) multi-entry-share 551ms -> 348ms (1.58x) Scales further with entry count — per-call plugin instantiation is the dominant cost, now amortized. Regression-locked: integration test asserts the build output contains "single-pass" marker AND not the per-entry timing lines for grouped entries — silent fallback to per-entry would fail the test. e2e: 576 tests pass (+2 new regression locks), typecheck + lint clean, all 10 packages build, zero leaked __dts_tmp* dirs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The first version of the single-pass DTS path had a real bug — proven
by a strict-mode (skipLibCheck:false) typecheck on the produced lib.
When the plugin code-splits across entries, it emits:
- <stem>.d.ts (re-export stub)
- <stem>NUMBER.d.ts (real declarations)
The other entry files reference the real-decls path:
import { X } from "./<stem>NUMBER.js"
The post-process renames <stem>NUMBER.d.ts -> <stem>.d.ts but DID NOT
rewrite the import paths in sibling entry files. Result: index.d.ts
contained `import { ... } from "./component2.js"` after `component2.d.ts`
had been renamed away. Real consumers fail with:
TS2307: Cannot find module './component2.js'
(The first PR's tests asserted file existence + content patterns only —
they didn't typecheck the produced .d.ts strictly, so the break slipped
through. Adding that as a regression lock.)
Fix:
- Track <oldStem -> newStem> renames during the promote step.
- After promote, scan each entry file for `./oldStem.js` import
specifiers and rewrite to `./newStem.js`.
- Tighten the candidate regex (drop a loose startsWith fallback).
- Split buildDtsGrouped into focused helpers to drop complexity.
Regression lock (new test in build.integration.test.ts):
- Typecheck the produced lib/types/*.d.ts with skipLibCheck:false.
- Without the fix: fails with TS2307 (verified by temporarily
skipping repairStaleImports).
- With the fix: passes.
e2e: 579 tests pass (+3 new), typecheck + lint clean, all 10 packages
build, zero leaked __dts_tmp* dirs.
Co-Authored-By: Claude Opus 4.7 (1M context) <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
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.
Summary
You were right to push back on the earlier negative result. Trying the single multi-entry DTS pass approach instead of parallel-Promise.all turned out to be the real win.
The `[PLUGIN_TIMINGS] rolldown-plugin-dts:generate` warning is fired per call into the plugin. Each per-entry DTS call instantiates the plugin (and its embedded TS compiler) from scratch. Building all entries in one call amortizes that setup AND lets common imports (e.g. a shared `types.ts`) emit as one `_chunks/*.d.ts` instead of inlining into every entry's stub.
Measured (apples-to-apples on the repo's own DTS fixtures)
Win scales with entry count — per-call plugin instantiation is the dominant cost, now amortized to one per group.
What I tried first (negative — discarded)
Bounded-concurrency `Promise.all` over the per-entry loop. Measured:
Per-entry duration tripled under that approach (≈655ms each vs ≈120ms sequential) because `Promise.all` over CPU-bound JS work serializes on V8's single thread. Discarded, branch deleted.
What works (this PR)
Mirrors PR #139's JS multi-entry shared-chunk architecture:
Regression-locked
The integration test now asserts:
e2e verification
Versioning
`@vitus-labs/tools-rolldown`: minor — semantically a perf fix, but consumers will see a new `_chunks/` dir alongside the `.d.ts` files for multi-entry packages (the shared chunk file). Additive, no API change.
🤖 Generated with Claude Code