perf: reuse prepared SSR request locations - #8441
schiller-manuel wants to merge 1 commit into
Conversation
|
Important Review skippedWe couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (19)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughSSR request handling now reuses the parsed request location. Server-route matching uses the prepared pathname, while handlers and middleware retain encoded pathnames. Build manifests now expose whether server routes exist. ChangesSSR and server-route handling
Priority: ⬇️ Low Estimated code review effort: 4 (Complex) | ~45 minutes Change: Refactor Sequence Diagram(s)sequenceDiagram
participant Request
participant createStartHandler
participant Router
participant ServerMiddleware
Request->>createStartHandler: submit SSR request
createStartHandler->>Router: prepare or reuse latestLocation
createStartHandler->>ServerMiddleware: run configured middleware
ServerMiddleware->>Router: getMatchedRoutes(location.pathname)
createStartHandler->>Router: load(_skipLocationUpdate: true)
Router-->>createStartHandler: return loaded route data
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The SSR request-location and server-route manifest changes retain their documented routing, rewrite, and fallback behavior. No concrete merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 15.63% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 32 functions across 21 files. (2 skipped: 2 unsupported.) ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
|
View your CI Pipeline Execution ↗ for commit 832a8a0
☁️ Nx Cloud last updated this comment at |
🚀 Changeset Version Preview2 package(s) bumped directly, 21 bumped as dependents. 🟩 Patch bumps
|
Bundle Size Benchmarks
The following scenarios have bundle-size changes compared with the baseline:
Current gzip tracks all emitted client JS chunks. Initial gzip tracks only the entry/import graph. Trend sparkline is historical current gzip ending with this PR measurement; lower is better. |
Merging this PR will regress 1 benchmark
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Memory | mem server request-churn (react) |
856.5 KB | 1,112.6 KB | -23.02% |
| ⚡ | Memory | mem server streaming-peak chunked (react) |
643.4 KB | 351.8 KB | +82.89% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing codex/reuse-ssr-request-location (832a8a0) with main (de023a7)
Footnotes
-
3 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
3df2769 to
b7600ee
Compare
b7600ee to
5727d34
Compare
5727d34 to
832a8a0
Compare
🎯 Changes
Reuse the request location already prepared by the router during SSR. Server-route handling matches that parsed pathname instead of running the input rewrite again, while preserving encoded pathnames for handlers and middleware. SSR loading also skips the redundant location update; ordinary router loads retain their existing behavior.
Validation: affected unit, type, and lint checks passed, along with the server-routes Chromium tests. Regression coverage includes rewrites, encoded parameters, basepaths, mutable history, and route middleware.
The build-time shortcut for apps without server routes is a separate follow-up in #8444.
✅ Checklist
🚀 Release Impact