perf(build): one Rslib instance per target for every agent-host surface (xref row 12) - #503
Conversation
🦋 Changeset detectedLatest commit: 49a56cd The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
e127307 to
424283b
Compare
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
424283b to
b939104
Compare
commit: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e12730734f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
b939104 to
62b0331
Compare
Plan each target's outputs as at most two stages (src/build/target-stages.ts): the optional browser MCP Apps stage first, only for targets with App routes, then every agent-host surface — routed CLI bin, bundled scripts, hook wrappers, MCP entries and their react-server Flight workers — lowered together through one Rslib instance (one Rsbuild environment per output, one Rspack multi-compiler) instead of one sequential instance per surface. Surfaces are plan/finish pairs that keep their own authored-source evidence exclusions and results. Bundled-output evidence now asks Rspack stats only for what it reads (assets, chunk ids, the complete module list with nameForCondition/identifier/moduleType and concatenated modules) and switches off per-module reasons, export usage, optimization bailouts, depth, module traces and errors, which dominated the post-compile cost once every surface reports through one stats object. Artifact and package-build trees of examples/audiobook-curator and examples/host-test are byte-identical to the baseline (modulo the pre-existing staged-directory token comment); manifest sourceInputs match.
…y names Surfaces sharing one run may legitimately reuse an entry name (a script authored as hooks-flight emits scripts/hooks-flight.mjs beside the hook surface's standalone worker hooks/hooks-flight.mjs); keying the lib id on the destination the planner already asserts unique keeps them apart. Ids are visible only in inspect --bundler and stats; emitted bytes are unchanged (artifact trees re-diffed against main ec65738).
62b0331 to
49a56cd
Compare
Summary
Cross-reference learning B (TanStack Start / React Router rsbuild-plugin matrix row 12, suggestion 5): one bundler instance with multiple environments and explicit dependency ordering, instead of one sequential instance per target × surface.
Builds on #495 (learning A, shared
compose-layers.ts, merged ase5428404). agent-bundle stays Rspack/Rsbuild/Rslib-only.Design
Per the maintainer's framing (agent-host surfaces are the primary outputs; browser MCP Apps are a secondary, optional environment), each target now compiles in at most two stages, planned by
src/build/target-stages.ts(planTargetStages) and driven by the orchestrator insrc/build/build.ts:mcp-apps— the browser environment through the workspace@rsbuild/core. Only present when the target declares App routes, and always first: the MCP entries embed its emitted HTML (this is the only build-time dependency in the graph), and its pass asserts the target root holds nothing but that HTML.node-surfaces— routed CLI bin, bundled scripts, hook wrappers, MCP stdio entries, and every surface's react-server Flight worker, lowered through one Rslib instance per target (buildRslibSurfaces/compileRslibSurfacesinsrc/build/rslib.ts): one Rsbuild environment (Rsliblibentry) per output, one Rspack multi-compiler. Each surface is a plan/finish pair (planCliBinsSurface,planScriptsSurface,planHooksSurface,planMcpEntriesSurface) that keeps its own authored-source evidence exclusions and result, so nothing about provenance changed.Why no intra-stage ordering: a host surface spawns its Flight worker by file name at run time (
new Worker(new URL('./x-flight.mjs', import.meta.url))), never via a build-time manifest — unlike TanStack/RR's client↔server manifest handoff. Rspack 2.1.10'sMultiCompilerdoes honor per-configdependencies(compiler names are the lib ids) if such an edge ever appears; it isn't needed today, so it isn't wired.Why not one instance total: MCP Apps compile under the workspace
@rsbuild/core2.2.1 forweb, the node surfaces under Rslib's nested Rsbuild 2.1.13 / Rspack 2.1.10; folding them into one engine would change emitted bytes. Targets don't share compilations (each target's outputs are per-target virtual entries into its own staged root), so per-target is the natural unit.Also in this PR, because it fell directly out of the measurement:
collectBundledOutputEvidence(src/build/provenance.ts) now asksstats.toJsononly for what it reads (assets, chunk ids, the complete module list incl. concatenated modules) and switches off per-modulereasons, export usage, optimization bailouts, depth, module traces, and errors. With every surface reporting through one stats object that walk had become the dominant post-compile cost (2.35 s → 0.54 s for host-test's 25-environment target).Before / after (rebased
mainc6db0ffe1, local,time pnpm exec agent-bundle build --output artifact, 3 runs each)examples/audiobook-curator(4 targets, MCP App)examples/host-test(4 targets)Rsbuild (MCP Apps) instances are unchanged: one per target that declares apps (audiobook-curator: 4; host-test: 0). Compilation counts are unchanged by construction — one Rspack compilation per emitted executable — the win is fewer instance setups, one
inspectConfig/stats pass per target, and the trimmed stats JSON. The artifact compile stage itself went from ~9.7 s → ~6.1 s (host-test) and ~2.4 s → ~1.5 s (audiobook-curator); the remaining wall time is dominated by artifact validation (~28 s on host-test:acorn+es-module-lexerparses of every multi-MB bundle), which is out of scope here and worth its own issue.Behavior preservation
artifact/**anddist/**of both examples are byte-identical to the baseline (manifestsourceInputs, file kinds, and digests match), modulo the pre-existing per-build staged-directory token (.artifact.stage-XXXXXX) that Rspack writes into a// NAMESPACE OBJECTcomment of MCP entries and that already makes two consecutivemainbuilds differ in those two files'sha256. Not introduced here; noted for follow-up.tools.rsbuild/tools.rspacksemantics (via refactor(build): one shared tools-hatch layering for every synthesized bundler config (xref row 13) #495's shared layering), and the per-surface emit order (CLI bin → scripts → hooks → MCP entries) are unchanged.inspect --bundlerdiffers in one debug-only field: Rslib lib ids (= Rsbuild environment / Rspack compiler names) now derive from the artifact destination (agent-bundle-scripts-tool,agent-bundle-hooks-hooks-flight,agent-bundle-lib-index) instead of the bare entry name — see the review response below. They never reach emitted bytes; the artifact diff above was re-run after that change againstmainec6573850. One observable difference in failure output only: the merged Rslib run logs at the most verbose level any surface asked for (errorwhenever a CLI bin or MCP entry is present), so a script or hook compile failure in such a target now also reaches the terminal as Rslib's own error log instead of only the thrown diagnostic.tests/target-stages.test.tspins the stage order (browser stage first and only with App routes; every host surface and its Flight worker in one node stage; empty node stage for nothing-to-compile), that four surfaces reach a singlecreateRslibcall with one lib per entry and per-surface evidence/exclusions, that no instance is created without entries, and the duplicate-lib-id guard.hooks.test.tsmoved fromcompileHookstocompileRslibSurfaces([planHooksSurface(...)]).build,hooks,mcp,cli,plugin-bundle,cli-routes-build,layout-build,inspect-bundler,package-build,dev-package-build(125/125);dev-artifact-service,dev-coordinator,dev-server,dev-watcher,dev-contract-adoption,prepack,dev-workbench,artifact-inspection-service(56/56);pnpm docs:site:buildgreen.Docs
docs/framework-mode.md(new "How a target compiles" under Distribution),website/docs/{en,zh}/guide/distribution/index.mdx(same section).Review status
Per the maintainer's instruction this PR carries no comments; review threads are answered here.
rslib.ts(e127307, "Namespace lib IDs across surfaces") — accepted and fixed in62b0331(now49a56cdafter rebase); thread resolved.entryLibIdnow derives fromoutputRelativePath(extension dropped,/→-), so a script authored ashooks-flight(scripts/hooks-flight.mjs) and the hook surface's standalone worker (hooks/hooks-flight.mjs) get distinct ids and share one run;assertDistinctLibIdsremains as an internal invariant behind the planner's unique-destination check.tests/target-stages.test.tspins the two ids and the guard;hooks.test.ts/inspect-bundler.test.tsstubs and expectations updated. Emitted artifacts unchanged (re-diffed).e127307(PR opened; one P2, addressed above). Unreviewed head49a56cd= rebase ontomainbf1d1eef4+ the lib-id fix; no re-review requested because this PR carries no comments.49a56cd(the earlier Docs site failure wasmain's duplicatedetailheader key from fix(build,validate,doctor): run the Claude host validator and --plugin-dir load check; doctor reads enabled (AB7327) (#475, #476) #504, fixed upstream in docs(site): drop the duplicate lineage Detail header key (#502 + #504 collision) #507 and picked up by the rebase; the pkg.pr.new failure was a transient Cloudflare 500).