feat: add safe lazy route compilation#97
Open
ScriptedAlchemy wants to merge 5 commits into
Open
Conversation
commit: |
This was referenced Jul 11, 2026
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e9180b2bb
ℹ️ 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".
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds opt-in lazy route-entry compilation with eager route exclusions, native resource matching, and export bridges for generated browser manifest assets. Route-chunk transforms now use the shared post-processing wrapper. Development compilation tracking distinguishes file-backed invalidations from explicit lazy activations and coordinates CSS ownership and route-manifest notifications. Tests and the default template cover the new matching, export-bridge, invalidation, and full-lazy route activation behavior. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/modify-browser-manifest.ts (1)
274-283: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider gating the bridge pass to dev only.
lazyCompilationis a dev-only config, so lazy-compilation-proxy assets never exist during a production build. As written, this runs forwebin both dev and build, reading and regex-scanning every route-entry asset just tocontinueduring builds — wasted work, and it needlessly exposes the fail-loud throw paths (Lines 133-137, 143-166) to production builds.routeChunkOptions?.isBuildis already available in this scope.♻️ Proposed gate
- if (pluginOptions.unstableLazyCompilationRouteEntries) { + if ( + pluginOptions.unstableLazyCompilationRouteEntries && + !routeChunkOptions?.isBuild + ) { appendLazyRouteEntryExportBridges({ assets, compilation, moduleExportsByRouteId, routes, sources, stats, }); }Please confirm lazy route-entry bridges are never expected during a production build (i.e.,
__lazy-compilation-proxy__shapes only appear under devlazyCompilation).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/modify-browser-manifest.ts` around lines 274 - 283, Gate the appendLazyRouteEntryExportBridges call in the unstableLazyCompilationRouteEntries block to development only by also checking that routeChunkOptions?.isBuild is false. Use the existing routeChunkOptions scope and preserve the current behavior for dev builds; confirm production builds never process lazy-compilation-proxy route-entry assets.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/dev-runtime-controller.ts`:
- Around line 73-78: Keep the lazy-compilation marker logic in
isExplicitLazyCompilation version-specific: avoid relying unconditionally on
Symbol.for('rspack.lazyCompilationCurrent'), and gate or isolate it based on the
supported `@rspack/core` version so incompatible changes disable or bypass the
hook safely.
---
Nitpick comments:
In `@src/modify-browser-manifest.ts`:
- Around line 274-283: Gate the appendLazyRouteEntryExportBridges call in the
unstableLazyCompilationRouteEntries block to development only by also checking
that routeChunkOptions?.isBuild is false. Use the existing routeChunkOptions
scope and preserve the current behavior for dev builds; confirm production
builds never process lazy-compilation-proxy route-entry assets.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f3981b74-3f1a-4b05-aa03-3ef9a8c10081
📒 Files selected for processing (16)
.changeset/full-lazy-compilation-safety.mdsrc/build-output-transforms.tssrc/dev-generation.tssrc/dev-runtime-artifacts.tssrc/dev-runtime-controller.tssrc/index.tssrc/lazy-compilation.tssrc/modify-browser-manifest.tssrc/types.tstests/build-output-transforms.test.tstests/dev-generation-css.test.tstests/dev-generation.test.tstests/dev-runtime-controller.test.tstests/index.test.tstests/lazy-compilation.test.tstests/modify-browser-manifest.test.ts
Contributor
Benchmark ResultsCompared PR head Reading benchmark confidenceRaw deltas are always shown. The signal label only indicates whether the observed median delta is larger than a robust run-to-run noise band; it does not erase or replace the measurement. The noise band is the larger of 2% or two combined robust standard deviations estimated from each side's relative median absolute deviation (rMAD). Fewer than three finite samples is reported as insufficient data. An inconclusive result should be rerun or investigated from the uploaded raw samples before drawing a performance conclusion.
Dev Rollup
Production Build BenchmarksRendered 7 production build benchmarks.
ci-small+ci-large Dev Fixture SummaryRendered 7 dev benchmark fixtures from the
large-355-ssr-esm Plugin Operations
synthetic-1024-ssr-esm Plugin Operations
synthetic-1024-ssr-esm-split Plugin Operations
synthetic-256-sourcemaps Plugin Operations
synthetic-256-ssr-esm Plugin Operations
synthetic-256-ssr-esm-split Plugin Operations
synthetic-48-ssr-esm Plugin Operations
Synthetic Rsbuild AppRendered 2 production build benchmarks.
Rendered 1 dev benchmark fixture from the embedded complex app.
Profile: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@package.json`:
- Around line 136-137: Remove the temporary pkg.pr.new override for
`@rsbuild/core` from pnpm.overrides in package.json, replacing it with a stable
published release/tag or removing the override so installs and CI no longer
depend on the preview artifact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 97679c40-53d2-41af-a134-e4c39888842a
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
package.json
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
Adds opt-in lazy browser route entries with explicit eager-route exclusions, a native-safe selector, and a fail-loud client-export bridge. The bridge identifies the final lazy route execution even when React Refresh and HMR runtime executions precede it. Explicit lazy provenance suppresses transient route/CSS ownership reloads while file-backed or unknown changes remain strict.
Related links
Tests
@rspack-canary/core@2.1.4-canary-a938a49e-20260711045630.