fix(react-router): useMatchRoute w/ React Compiler - #8015
Conversation
|
View your CI Pipeline Execution ↗ for commit 3abdab7
☁️ Nx Cloud last updated this comment at |
🚀 Changeset Version PreviewNo changeset entries found. Merging this PR will not cause a version bump for any packages. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough
ChangesReact Compiler match route updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant RouterProvider
participant useMatchRoute
participant MatchState
Browser->>RouterProvider: Navigate from Home to About
RouterProvider->>MatchState: Update location and route state
MatchState->>useMatchRoute: Update callback dependencies
useMatchRoute->>Browser: Render matched route name
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 |
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 0 benchmarks
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | Memory | mem client unique-location-churn (solid) |
436.9 KB | 341.5 KB | +27.94% |
| ⚡ | Simulation | ssr server-fn not-found (solid) |
70.4 ms | 66.1 ms | +6.51% |
| ⚡ | Simulation | ssr server-fn POST (solid) |
73 ms | 68.6 ms | +6.43% |
| ⚡ | Simulation | ssr server-fn send-context (solid) |
70.7 ms | 67.1 ms | +5.36% |
| ⚡ | Simulation | ssr server-fn GET (solid) |
82.6 ms | 79.3 ms | +4.26% |
| ⚡ | Simulation | ssr server-fn redirect (solid) |
55.9 ms | 54.1 ms | +3.25% |
| ⚡ | Memory | mem server error-paths not-found (vue) |
329.9 KB | 320.1 KB | +3.05% |
| 👁 | Memory | mem server error-paths redirect (vue) |
293 KB | 381.4 KB | -23.18% |
| 👁 | Memory | mem server error-paths not-found (solid) |
661.7 KB | 742.6 KB | -10.88% |
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 fix-react-router-use-match-route-react-compiler (3abdab7) with main (af8dcb8)
![nx-cloud[bot]](https://avatars.githubusercontent.com/in/80458?s=60&v=4){kind=small}
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 is proposing a fix for your failed CI:
We fixed the react-hooks/exhaustive-deps ESLint errors in useMatchRoute by moving the useStore calls to the top level of the hook unconditionally, then referencing their return values in a plain array literal passed to useCallback. This ensures both React's rules of hooks and the React Compiler's requirement for unconditional hook invocations are satisfied, while still correctly tracking location, resolvedLocation, and status changes as useCallback dependencies.
Warning
❌ We could not verify this fix.
Suggested Fix changes
diff --git a/packages/react-router/src/Matches.tsx b/packages/react-router/src/Matches.tsx
index 9d7708fc..a8df3ba0 100644
--- a/packages/react-router/src/Matches.tsx
+++ b/packages/react-router/src/Matches.tsx
@@ -154,6 +154,16 @@ export type UseMatchRouteOptions<
export function useMatchRoute<TRouter extends AnyRouter = RegisteredRouter>() {
const router = useRouter()
+ const locationHref = useStore(
+ router.stores.location,
+ (location) => location.href,
+ )
+ const resolvedLocationHref = useStore(
+ router.stores.resolvedLocation,
+ (location) => location?.href,
+ )
+ const status = useStore(router.stores.status, (status) => status)
+
return React.useCallback(
<
const TFrom extends string = string,
@@ -174,20 +184,7 @@ export function useMatchRoute<TRouter extends AnyRouter = RegisteredRouter>() {
includeSearch,
})
},
- (isServer ?? router.isServer)
- ? [router]
- : [
- router,
- // eslint-disable-next-line react-hooks/rules-of-hooks
- useStore(router.stores.location, (location) => location.href),
- // eslint-disable-next-line react-hooks/rules-of-hooks
- useStore(
- router.stores.resolvedLocation,
- (location) => location?.href,
- ),
- // eslint-disable-next-line react-hooks/rules-of-hooks
- useStore(router.stores.status, (status) => status),
- ],
+ [router, locationHref, resolvedLocationHref, status],
)
}
Or Apply changes locally with:
npx nx-cloud apply-locally c0UJ-Wu8o
Apply fix locally with your editor ↗ View interactive diff ↗
🎓 Learn more about Self-Healing CI on nx.dev
Co-authored-by: Kamal Bennani <kamal.bennani@aircall.io>
Co-authored-by: Sarah Gerrard <98355961+LadyBluenotes@users.noreply.github.com>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/react-compiler/package.json`:
- Line 14: Update the internal dependency entries in package.json, including the
`@tanstack/react-router` entry and the other referenced workspace dependency, to
use the workspace:* protocol instead of workspace:^.
In `@e2e/react-router/react-compiler/src/main.tsx`:
- Line 12: Remove the useRouterState import and subscription from the
reproduction, including its useMemo dependency in the relevant component. Keep
the label computation dependent only on matchRoute from useMatchRoute so the E2E
fixture verifies navigation updates through useMatchRoute alone.
🪄 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: cdcec0a0-73fa-45ae-b6ac-dffb666f6766
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
e2e/react-router/react-compiler/index.htmle2e/react-router/react-compiler/package.jsone2e/react-router/react-compiler/playwright.config.tse2e/react-router/react-compiler/src/main.tsxe2e/react-router/react-compiler/tests/use-match-route.spec.tse2e/react-router/react-compiler/tsconfig.jsone2e/react-router/react-compiler/vite.config.jspackages/react-router/src/Matches.tsxpackages/react-router/tests/Matches.test.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/react-router/src/Matches.tsx
1 participant
fixes #4499
Summary by CodeRabbit
Bug Fixes
useMatchRoutebehavior during navigation and server-side rendering.Tests