Uh oh!
There was an error while loading. Please reload this page.
fix(react-router): prevent blank frame before initial pending UI - #7961
Conversation
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe change scopes router match publication transitions to the React component and adds an Issue 4759 E2E project. Tests verify default and route-specific pending states without blank paint states before route loading completes. ChangesPending frame fix
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant Matches
participant Transitioner
participant Router
Browser->>Matches: render router matches
Matches->>Transitioner: pass component transition setter
Transitioner->>Router: publish router state before startTransition
Router-->>Transitioner: update rendered matches
Transitioner-->>Browser: render pending or loaded state
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
View your CI Pipeline Execution ↗ for commit 282a402
☁️ Nx Cloud last updated this comment at |
🚀 Changeset Version Preview5 package(s) bumped directly, 18 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. |
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 `@e2e/react-router/issue-4759/package.json`:
- Around line 14-20: Update the internal `@tanstack/react-router` and
`@tanstack/router-e2e-utils` dependency ranges in package.json from workspace:^ to
workspace:*. Leave the external React and Playwright dependency ranges
unchanged.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 963190f4-dd33-47bf-b55a-3489018b2083
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
.changeset/bright-frames-settle.mde2e/react-router/issue-4759/index.htmle2e/react-router/issue-4759/package.jsone2e/react-router/issue-4759/playwright.config.tse2e/react-router/issue-4759/src/main.tsxe2e/react-router/issue-4759/tests/issue-4759.spec.tse2e/react-router/issue-4759/tsconfig.jsone2e/react-router/issue-4759/vite.config.tspackages/react-router/src/Matches.tsxpackages/react-router/src/Transitioner.tsxpackages/react-router/tests/issue-4759-pending-frame.test.tsx
💤 Files with no reviewable changes (1)
- packages/react-router/tests/issue-4759-pending-frame.test.tsx
Uh oh!
There was an error while loading. Please reload this page.
Merging this PR will degrade performance by 1.48%
|
| Mode | Benchmark | BASE | HEAD | Efficiency | |
|---|---|---|---|---|---|
| ⚡ | Memory | mem server error-paths redirect (vue) | 927.7 KB | 449.2 KB | ×2.1 |
| ⚡ | Simulation | ssr global-mw server-fn (solid) | 60.3 ms | 57.5 ms | +4.82% |
| 👁 | Memory | mem server error-paths redirect (solid) | 273.7 KB | 476.2 KB | -42.52% |
| 👁 | Simulation | client-mount loop (react) | 36.1 ms | 38.5 ms | -6.18% |
| 👁 | Memory | mem server error-paths not-found (react) | 267.5 KB | 286.6 KB | -6.68% |
| 👁 | Memory | mem server error-paths redirect (react) | 195.9 KB | 210.7 KB | -6.99% |
| 👁 | Memory | mem server server-fn-churn (react) | 265.2 KB | 275.8 KB | -3.84% |
| 👁 | Memory | mem client navigation-churn (solid) | 537.4 KB | 590.1 KB | -8.92% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing codex/fix-4759-pending-frame (282a402) with main (6aefb33)
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Important
At least one additional CI pipeline execution has run since the conclusion below was written and it may no longer be applicable.
Nx Cloud has identified a possible root cause for your failed CI:
We determined this failure is unrelated to the PR's changes. The tanstack-solid-start-example-basic-static:build task failed because the external API jsonplaceholder.typicode.com returned a 502 Bad Gateway error during static prerendering. This is a transient external service outage and the build should pass once the service recovers.
No code changes were suggested for this issue.
Trigger a rerun:
🎓 Learn more about Self-Healing CI on nx.dev
Uh oh!
There was an error while loading. Please reload this page.
Fixes#4759.
What was happening
On the first client render, a route whose pending delay resolves to zero—either through
defaultPendingMs: 0or a route-levelpendingMs: 0—could paint the application shell for one frame before showing its pending component.Router core was already selecting the correct pending boundary and offering the expected matches immediately. The gap was in the React adapter: match publication used the static
React.startTransitionAPI during mount. At that point, the match tree's store subscription may not have committed yet, and a static transition does not create a component-owned update that schedules the match tree itself. The publication was therefore observed on a later render, after the shell had already painted.Why this fixes it
Matchesnow creates a component-scoped transition withuseTransitionand passes its start function toTransitioner. When router core starts a match publication, that transition schedulesMatches—the owner of the rendered match tree—so the already-offered matches are observed before the browser paints. The existing Suspense boundary can then display the correct pending component immediately.The router store remains the sole source of truth. This does not add parallel presentation state or duplicate pending-option logic in the React adapter: router core still decides the pending timing, boundary, and exact matches to publish.
Regression coverage
The new browser reproduction uses only public APIs and samples the visible DOM state once per animation frame. It covers:
defaultPendingMs={0}pendingMs: 0while the provider default is1000Both cases assert that the pending component is painted and that neither the surrounding shell nor an empty root is ever observed before it.
Validation
Summary by CodeRabbit
Bug Fixes
Tests