regression tests#7889
Open
schiller-manuel wants to merge 3 commits into
Open
Conversation
Contributor
|
View your CI Pipeline Execution ↗ for commit aba4ddc
☁️ Nx Cloud last updated this comment at |
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR adds Issue 6221 route and head-data coverage, updates Issue 4614 selective-SSR context tracking, and expands router tests for transition ordering, preload contracts, invalidation, and hydration cleanup timing. ChangesIssue 6221 fresh loader data
Issue 4614 context propagation
Router runtime contract coverage
Estimated code review effort: 4 (Complex) | ~50 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant ArticleRoute
participant LocalStorage
participant DashboardRoute
Browser->>LocalStorage: set issue-6221-auth
Browser->>ArticleRoute: navigate to article
ArticleRoute->>LocalStorage: check authentication
ArticleRoute-->>Browser: render article and head title
Browser->>DashboardRoute: navigate to dashboard
Browser->>ArticleRoute: go back
ArticleRoute-->>Browser: refresh loader data and head title
sequenceDiagram
participant Browser
participant RootRoute
participant Issue4614Route
Browser->>RootRoute: hover issue-4614-link
RootRoute->>RootRoute: run beforeLoad and record context
RootRoute->>Issue4614Route: propagate route context during preload
Issue4614Route->>Issue4614Route: record beforeLoad context
Browser->>Browser: assert root and target payloads
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
🚀 Changeset Version Preview4 package(s) bumped directly, 19 bumped as dependents. 🟩 Patch bumps
|
schiller-manuel
changed the base branch from
main
to
fix-router-core-lane-match-loader
July 23, 2026 20:48
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
e2e/react-start/selective-ssr/tests/app.spec.ts (1)
46-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winType the selective-ssr globals instead of casting
globalThistoanyDeclare__issue4614RootBeforeLoadsand__issue4614TargetBeforeLoadon the shared browser global type, then use them directly in both the route code and the Playwright test so the contract stays type-checked.🤖 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 `@e2e/react-start/selective-ssr/tests/app.spec.ts` at line 46, Replace the any-cast access to __issue4614RootBeforeLoads in the Playwright test with a typed shared browser-global declaration, also declaring __issue4614TargetBeforeLoad. Update the selective-SSR route code and test to access both globals directly so their names and values remain type-checked.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@e2e/react-start/selective-ssr/tests/app.spec.ts`:
- Line 46: Replace the any-cast access to __issue4614RootBeforeLoads in the
Playwright test with a typed shared browser-global declaration, also declaring
__issue4614TargetBeforeLoad. Update the selective-SSR route code and test to
access both globals directly so their names and values remain type-checked.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b762bf4d-8cf3-44a3-99ec-787fbafbae82
📒 Files selected for processing (2)
e2e/react-start/selective-ssr/src/routes/__root.tsxe2e/react-start/selective-ssr/tests/app.spec.ts
Collaborator
Author
|
@copilot resolve the merge conflicts in this pull request |
Contributor
Resolved the merge conflicts. The only conflict was in
The |
schiller-manuel
force-pushed
the
regression-4614
branch
from
July 23, 2026 21:16
f17721e to
33b9472
Compare
schiller-manuel
changed the base branch from
fix-router-core-lane-match-loader
to
main
July 23, 2026 22:06
schiller-manuel
force-pushed
the
regression-4614
branch
from
July 23, 2026 22:06
0f94cda to
279db75
Compare
Contributor
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. |
Contributor
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-start/selective-ssr/src/routes/__root.tsx`:
- Around line 62-68: Define a shared TypeScript global declaration for the Issue
4614 instrumentation payloads, then remove the globalThis as any casts. In
e2e/react-start/selective-ssr/src/routes/__root.tsx lines 62-68, type
__issue4614RootBeforeLoads; in
e2e/react-start/selective-ssr/src/routes/issue-4614.tsx lines 6-10, type
__issue4614TargetBeforeLoad; and in
e2e/react-start/selective-ssr/tests/app.spec.ts line 19, read the typed root
payload directly. Ensure all sites reuse the same shared contract.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7cf4ba4a-6ce8-494e-a9bf-1b029b33397d
📒 Files selected for processing (8)
e2e/react-start/basic/src/routeTree.gen.tse2e/react-start/basic/src/routes/issue-6221.article.$id.tsxe2e/react-start/basic/src/routes/issue-6221.dashboard.tsxe2e/react-start/basic/tests/issue-6221-head.spec.tse2e/react-start/selective-ssr/src/routes/__root.tsxe2e/react-start/selective-ssr/src/routes/issue-4614.tsxe2e/react-start/selective-ssr/tests/app.spec.tspackages/router-core/tests/issue-6221-head-waits-for-loader.test.ts
💤 Files with no reviewable changes (1)
- packages/router-core/tests/issue-6221-head-waits-for-loader.test.ts
Comment on lines
62
to
68
| const calls = ((globalThis as any).__issue4614RootBeforeLoads ??= []) | ||
| calls.push({ | ||
| cause, | ||
| preload, | ||
| root, | ||
| issue4614Context, | ||
| isClient, | ||
| isServer, | ||
| }) |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Define one typed global contract for Issue 4614 instrumentation.
The new globalThis as any casts erase the payload shape across the route and its regression test. Add a shared TypeScript declaration for these globals and reuse it everywhere.
e2e/react-start/selective-ssr/src/routes/__root.tsx#L62-L68: type__issue4614RootBeforeLoads.e2e/react-start/selective-ssr/src/routes/issue-4614.tsx#L6-L10: type__issue4614TargetBeforeLoad.e2e/react-start/selective-ssr/tests/app.spec.ts#L19-L19: read the typed root payload instead of casting toany.
📍 Affects 3 files
e2e/react-start/selective-ssr/src/routes/__root.tsx#L62-L68(this comment)e2e/react-start/selective-ssr/src/routes/issue-4614.tsx#L6-L10e2e/react-start/selective-ssr/tests/app.spec.ts#L19-L19
🤖 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 `@e2e/react-start/selective-ssr/src/routes/__root.tsx` around lines 62 - 68,
Define a shared TypeScript global declaration for the Issue 4614 instrumentation
payloads, then remove the globalThis as any casts. In
e2e/react-start/selective-ssr/src/routes/__root.tsx lines 62-68, type
__issue4614RootBeforeLoads; in
e2e/react-start/selective-ssr/src/routes/issue-4614.tsx lines 6-10, type
__issue4614TargetBeforeLoad; and in
e2e/react-start/selective-ssr/tests/app.spec.ts line 19, read the typed root
payload directly. Ensure all sites reuse the same shared contract.
Source: Coding guidelines
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/react-start-client/src/tests/hydrateStart.test.ts (1)
23-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPreserve type safety in both hydration fixtures.
Both tests bypass the
window.$_TSRcontract withas any; replace it with the declared shape or a narrow test-specific type.
packages/react-start-client/src/tests/hydrateStart.test.ts#L23-L24: remove theas anycast from the React fixture.packages/solid-start-client/src/tests/hydrateStart.test.ts#L23-L24: remove theas anycast from the Solid fixture.🤖 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 `@packages/react-start-client/src/tests/hydrateStart.test.ts` around lines 23 - 24, Remove the as any cast from the window.$_TSR hydration fixture in packages/react-start-client/src/tests/hydrateStart.test.ts lines 23-24, using the declared contract or a narrow test-specific type; make the same change to the corresponding window.$_TSR fixture in packages/solid-start-client/src/tests/hydrateStart.test.ts lines 23-24.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@packages/react-start-client/src/tests/hydrateStart.test.ts`:
- Around line 23-24: Remove the as any cast from the window.$_TSR hydration
fixture in packages/react-start-client/src/tests/hydrateStart.test.ts lines
23-24, using the declared contract or a narrow test-specific type; make the same
change to the corresponding window.$_TSR fixture in
packages/solid-start-client/src/tests/hydrateStart.test.ts lines 23-24.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 932b396a-fc47-4514-a900-eb6b1f43b6b8
📒 Files selected for processing (4)
packages/react-router/tests/transitioner-render-ack.test.tsxpackages/react-start-client/src/tests/hydrateStart.test.tspackages/router-core/tests/public-preload-lane-contract.test.tspackages/solid-start-client/src/tests/hydrateStart.test.ts
schiller-manuel
force-pushed
the
regression-4614
branch
from
July 23, 2026 22:22
0c5da78 to
5e4bb96
Compare
Merging this PR will degrade performance by 11.8%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Memory | mem server error-paths redirect (vue) |
306 KB | 803.4 KB | -61.91% |
| ❌ | Memory | mem client unique-location-churn (solid) |
341.1 KB | 552.9 KB | -38.3% |
| ❌ | Simulation | ssr server-fn not-found (solid) |
61.8 ms | 66.8 ms | -7.57% |
| ❌ | Simulation | ssr server-fn POST (solid) |
64.6 ms | 69.5 ms | -7% |
| ❌ | Simulation | ssr server-fn during document ssr (react) |
63.5 ms | 67.3 ms | -5.65% |
| ❌ | Simulation | ssr server-fn send-context (solid) |
65.8 ms | 69.6 ms | -5.47% |
| ❌ | Memory | mem server error-paths not-found (react) |
242.6 KB | 253.7 KB | -4.36% |
| ❌ | Memory | mem server error-paths not-found (vue) |
332 KB | 346.4 KB | -4.17% |
| ❌ | Simulation | ssr server-fn GET (solid) |
71.4 ms | 74.3 ms | -3.95% |
| ❌ | Simulation | ssr selective (solid) |
70.3 ms | 73.1 ms | -3.9% |
| ❌ | Memory | mem client navigation-churn (solid) |
544.9 KB | 563.5 KB | -3.29% |
| ⚡ | Memory | mem server error-paths not-found (solid) |
751.3 KB | 692.7 KB | +8.46% |
| ⚡ | Memory | mem server peak-large-page (solid) |
819.4 KB | 787.8 KB | +4.02% |
| ⚡ | Memory | mem server error-paths redirect (solid) |
283.4 KB | 273.6 KB | +3.62% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing regression-4614 (aba4ddc) with fix-router-core-lane-match-loader (2d639e1)
schiller-manuel
changed the base branch from
main
to
fix-router-core-lane-match-loader
July 23, 2026 22:23
schiller-manuel
force-pushed
the
regression-4614
branch
from
July 23, 2026 22:33
5e4bb96 to
aba4ddc
Compare
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
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 by CodeRabbit