refactor(build): align Rsbuild integration with official plugin patterns - #510
Conversation
|
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. |
9f60224 to
fc1d717
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a3cb01fcf
ℹ️ 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".
| 'agent-bundle': patch | ||
| --- | ||
|
|
||
| Register the package build's `__filename`/`__dirname` ESM shim through Rsbuild's `processAssets` plugin hook instead of a hand-rolled Rspack plugin class inside `tools.rspack`, matching how Rsbuild's own asset plugins hang post-build rewrites. The emitted `dist/**` is byte-identical; no artifact, config key, or CLI output changes. (#PR) |
There was a problem hiding this comment.
Replace the implementation-only changeset summary
When the next Version Packages release consumes this changeset, this paragraph becomes user-facing release prose, but it only documents the internal processAssets/Rspack refactor and explicitly says behavior is unchanged; it also names no public command, export, or config key. Replace it with the required user-facing imperative summary rather than publishing internal build mechanics.
AGENTS.md reference: AGENTS.md:L104-L105
Useful? React with 👍 / 👎.
commit: |
Source-reading pass over the official/community plugins evaluated in #509 (
rsbuild-plugin-publint,rsbuild-plugin-arethetypeswrong,@rsbuild/plugin-type-check,@rsbuild/plugin-check-syntax,rsbuild-plugin-source-build,@rsbuild/plugin-react,rsbuild-plugin-virtual-module) plus Rsbuild core's built-in plugins, looking for patterns that make an existing path inpackages/agent-bundle/src/build/**or the two packagerslib.config.tsfiles shorter or clearer with identical output. Scope kept to that; no new abstraction, plugin layer, or option. Each change below is one commit-sized item the maintainer can judge separately.Changes
packages/agent-bundle/rslib.config.ts— the__filename/__dirnameESM shimRspack/rspacktype imports and the engine parameterapi.processAssets({ stage: 'additions' }, ({ assets, compilation, sources }) => …)— how Rsbuild's built-inrsbuild:inline-chunkandrsbuild:appIconplugins hang asset rewrites, instead of a hand-written Rspack plugin class (apply → thisCompilation.tap → processAssets.tap({ stage: PROCESS_ASSETS_STAGE_ADDITIONS })) appended viatools.rspack. Rsbuild's implementation of the hook is the samethisCompilation+processAssets.tapPromise({ stage })tap.dist/**byte-identical (warm and cold cache):diff -rqbefore/after clean;dist/1178.jsstill carries the shim.That is the only change that met the bar.
pluginPublintwas already registered the official way, and bothcomposeEntryLibConfigandcomposeMcpAppsRsbuildConfigalready use@rsbuild/plugin-react.Considered and left alone (with the reason)
onAfterBuild/afterEmit(assertNoResidualReservedImports,assertSelfContainedViews; publint/attw useapi.onAfterBuild, check-syntax usescompiler.hooks.afterEmitovercompilation.getAssets()): the framework's checks deliberately read the published on-disk bytes afterrslib.build()resolves and afterclose(), and wrapping them in a plugin would be a new plugin layer, not a shorter path.@rsbuild/core'slogger(publint/attw/type-check pattern):src/build/**prints nothing itself — every failure is a thrownError/DiagnosticErrorthat the CLI's diagnostics layer renders, and builds run atlogLevel: 'silent'/'error'. Nothing to move.toolsshape checks (AB4720–4723) live inconfig/validate.ts, outside this scope, and are the diagnostics the docs promise.pluginReact({ fastRefresh: false })inrslib.ts: the plugin only enables refresh in dev mode, so the option is redundant forbuild(), but the file is owned by perf(build): one Rslib instance per target for every agent-host surface (xref row 12) #503 right now (see below) and the explicitfalsedocuments intent.composeMcpAppsRsbuildConfigalready uses one Rsbuild instance with one environment per view, the shape type-check'scheckedTsconfig-per-environment logic assumes.For the
xref-build-orchestrationworker (#503 touchesrslib.ts; not edited here)virtualModulesPluginConstructoris duplicated verbatim insrc/build/rslib.ts(Rslib's engine) andsrc/build/mcp-apps.ts(workspace@rsbuild/core), differing only in the message text. One helper taking(rspack, engineName, purpose)would drop ~10 lines; both call sites keep their own engine object, so the dual-engine rule holds.enforceInvariantsinrslib.tsgrows aDefinePluginand aVirtualModulesPluginthroughconfig.plugins = [...(config.plugins ?? []), …]twice; a single append at the end of the mutator is one statement.Verification
pnpm typecheck,pnpm lint: clean.packages/agent-bundle/distbefore vs after:diff -rqreports no differences, with the default warm Rspack cache and with a freshAGENT_BUNDLE_RSLIB_CACHE_DIRECTORY.artifact/**forexamples/audiobook-curatorandexamples/host-test: unchanged by construction — nosrc/build/**file changes in this PR and the packagedistthey build with is byte-identical.Review status
Codex reviewed
5a3cb01(PR opened). One thread, addressed in-branch rather than by reply (this PR posts no comments):.changeset/build-official-plugin-patterns.md— "implementation-only summary": agreed; there is no user-facing change to describe (byte-identicaldist/**, no command, export, or config key). The changeset is removed and the PR carries theskip-changesetlabel, the AGENTS.md escape hatch for genuinely no-op changes.Last Codex-reviewed head:
5a3cb01. Unreviewed heads (no re-review arrived after the pushes; this PR posts no comments): the changeset-number commit, the rebase over #507, and the changeset removal — current head76877346d. Merged on green CI per the AGENTS.md fallback; any thread opened afterwards is answered in a follow-up PR.