fix(react-router): useMatchRoute w/ React Compiler - #8015
Conversation
|
View your CI Pipeline Execution ↗ for commit f1fffd1
☁️ Nx Cloud last updated this comment at |
🚀 Changeset Version Preview2 package(s) bumped directly, 21 bumped as dependents. 🟩 Patch bumps
|
|
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)
📝 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: Recreate client callback
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 2 benchmarks
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Memory | mem server error-paths redirect (solid) |
278.2 KB | 640.2 KB | -56.55% |
| ❌ | Memory | mem server server-fn-churn (vue) |
263.2 KB | 274.8 KB | -4.21% |
| ⚡ | Memory | mem server error-paths not-found (solid) |
728.6 KB | 411.7 KB | +76.97% |
| ⚡ | Memory | mem server error-paths redirect (vue) |
433.1 KB | 298.4 KB | +45.14% |
| ⚡ | Memory | mem client preload-churn (vue) |
805.5 KB | 738.8 KB | +9.03% |
| ⚡ | Memory | mem client unique-location-churn (solid) |
352.4 KB | 341.9 KB | +3.07% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing fix-react-router-use-match-route-react-compiler (f1fffd1) with main (44a8c3e)
![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
Summary by CodeRabbit
Bug Fixes
useMatchRouteupdates during route navigation.Tests