Conversation
github-actions
bot
added
documentation
📝 WalkthroughWalkthroughThis PR removes the public Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Comment |
|
View your CI Pipeline Execution ↗ for commit 565a264
☁️ Nx Cloud last updated this comment at |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/vue-router/src/useMatch.tsx (1)
80-94:⚠️ Potential issue | 🟠 MajorImport
RouterStateand properly type the state parameter to maintain strict type safety.The
selectcallback inuseRouterStateexpects(state: RouterState<TRouter['routeTree']>) => TSelected, but the current code passes(state: any), breaking the type contract. Similarly,(d: any)in the pending match check should be typed via the inferred return type frommatchRoutes.Suggested refactor
import type { AnyRouter, MakeRouteMatch, MakeRouteMatchUnion, RegisteredRouter, + RouterState, StrictOrFrom, ThrowConstraint, ThrowOrOptional, } from '@tanstack/router-core'const matchSelection = useRouterState({ - select: (state: any) => { + select: (state: RouterState<TRouter['routeTree']>) => { const match = state.matches.find((d: any) => opts.from ? opts.from === d.routeId : d.id === nearestMatchId.value, ) if (match === undefined) { - const hasPendingMatch = + const pendingMatches = router.matchRoutes(router.latestLocation) + const hasPendingMatch = state.status === 'pending' && - router - .matchRoutes(router.latestLocation) - .some((d: any) => + pendingMatches.some((d) => opts.from ? opts.from === d.routeId : d.id === nearestMatchId.value, )As per coding guidelines:
**/*.{ts,tsx}: Use TypeScript strict mode with extensive type safety for all code.packages/solid-router/src/useMatch.tsx (1)
83-95:⚠️ Potential issue | 🟠 MajorAdd
RouterStatetype to the selector function to maintain strict type safety.The
selectfunction parameter and match element callbacks useanyinstead of the properly typedRouterState. ImportRouterStatefrom@tanstack/router-coreand type the selector asselect: (state: RouterState<TRouter['routeTree']>) => { ... }to align with the TypeScript strict mode guideline and existing patterns in the codebase (seeMatches.tsx).
🤖 Fix all issues with AI agents
In `@docs/router/api/router/RouterStateType.md`:
- Line 6: Update the documentation sentence for the RouterState type to include
the missing article for clarity: change the phrase so it reads "The
`RouterState` type represents the shape of the internal state of the router."
(refer to the `RouterState` type description) and similarly ensure the following
sentence reads "The Router's internal state..." where appropriate for consistent
readability.
In `@packages/router-devtools-core/src/BaseTanStackRouterDevtoolsPanel.tsx`:
- Around line 282-289: The createMemo callbacks for pendingMatches and
displayedMatches are using implicit/any types which breaks strict TS guarantees;
update the createMemo calls for pendingMatches and displayedMatches to use
explicit generic type parameters (e.g., createMemo<AnyRouteMatch[]>) so the
returned arrays are strongly typed, remove any explicit any annotations in the
callbacks and downstream map/find usages, and ensure the expressions using
routerState(), router().matchRoutes(router().latestLocation) and
routerState().matches preserve the new typed signature.
🧹 Nitpick comments (3)
packages/solid-router/tests/useMatch.test.tsx (1)
184-243: Good test coverage for the complementary case.The test correctly validates that
useMatchstill throws when the target route (/posts) is not in the pending matches, even when the router is in a pending state for another route (/other).Consider adding a final assertion after navigation completes to verify the router reaches the expected state:
🔧 Optional: Add assertion for final state
resolveOtherLoader() await navigation + expect(await screen.findByText('OtherTitle')).toBeInTheDocument() })packages/vue-router/tests/useMatch.test.tsx (1)
185-244: Add a final assertion to confirm navigation completes.
This prevents the test from passing if the transition aborts after the invariant error.✅ Suggested assertion
resolveOtherLoader() await navigation + expect(await screen.findByText('OtherTitle')).toBeInTheDocument()packages/router-devtools-core/src/BaseTanStackRouterDevtoolsPanel.tsx (1)
143-148: Avoid recomputingmatchRoutesperRouteCompwhile pending.This memo calls
router().matchRoutes(router().latestLocation)for every route row when pending, which can multiply matcher cost on large trees. Consider computing pending matches once in the panel and passing an accessor down toRouteComp.
| --- | ||
|
|
||
| The `RouterState` type represents shape of the internal state of the router. The Router's internal state is useful, if you need to access certain internals of the router, such as any pending matches, is the router in its loading state, etc. | ||
| The `RouterState` type represents shape of the internal state of the router. The Router's internal state is useful if you need to access certain internals of the router, such as whether it is currently loading. |
There was a problem hiding this comment.
Grammar: add “the” for readability.
✏️ Suggested edit
-The `RouterState` type represents shape of the internal state of the router.
+The `RouterState` type represents the shape of the internal state of the router.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| The `RouterState` type represents shape of the internal state of the router. The Router's internal state is useful if you need to access certain internals of the router, such as whether it is currently loading. | |
| The `RouterState` type represents the shape of the internal state of the router. |
🤖 Prompt for AI Agents
In `@docs/router/api/router/RouterStateType.md` at line 6, Update the
documentation sentence for the RouterState type to include the missing article
for clarity: change the phrase so it reads "The `RouterState` type represents
the shape of the internal state of the router." (refer to the `RouterState` type
description) and similarly ensure the following sentence reads "The Router's
internal state..." where appropriate for consistent readability.
| const pendingMatches = createMemo(() => | ||
| routerState().status === 'pending' | ||
| ? router().matchRoutes(router().latestLocation) | ||
| : [], | ||
| ) | ||
| const displayedMatches = createMemo(() => | ||
| pendingMatches().length ? pendingMatches() : routerState().matches, | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate the file
find . -type f -name "BaseTanStackRouterDevtoolsPanel.tsx" | head -5Repository: TanStack/router
Length of output: 130
🏁 Script executed:
# Check file size to determine how to read it
wc -l packages/router-devtools-core/src/BaseTanStackRouterDevtoolsPanel.tsxRepository: TanStack/router
Length of output: 132
🏁 Script executed:
# Read the relevant sections mentioned in the review
sed -n '280,295p' packages/router-devtools-core/src/BaseTanStackRouterDevtoolsPanel.tsxRepository: TanStack/router
Length of output: 665
🏁 Script executed:
# Also check lines 532 and 687-694
sed -n '530,535p' packages/router-devtools-core/src/BaseTanStackRouterDevtoolsPanel.tsxRepository: TanStack/router
Length of output: 311
🏁 Script executed:
sed -n '685,700p' packages/router-devtools-core/src/BaseTanStackRouterDevtoolsPanel.tsxRepository: TanStack/router
Length of output: 743
🏁 Script executed:
# Search for AnyRouteMatch type definition
rg "AnyRouteMatch" --type ts --type tsxRepository: TanStack/router
Length of output: 86
🏁 Script executed:
# Check the imports in the file to understand available types
head -50 packages/router-devtools-core/src/BaseTanStackRouterDevtoolsPanel.tsxRepository: TanStack/router
Length of output: 1457
Tighten types for pending/displayed matches to keep strict TS guarantees.
Explicit any in the createMemo callbacks and map/find operations undermines strict typing. Adding explicit type parameters <AnyRouteMatch[]> to both createMemo calls removes the need for any type annotations and keeps usages typed throughout.
♻️ Proposed fix
- const pendingMatches = createMemo(() =>
+ const pendingMatches = createMemo<AnyRouteMatch[]>(() =>
routerState().status === 'pending'
? router().matchRoutes(router().latestLocation)
: [],
)
- const displayedMatches = createMemo(() =>
+ const displayedMatches = createMemo<AnyRouteMatch[]>(() =>
pendingMatches().length ? pendingMatches() : routerState().matches,
)- {displayedMatches().map((match: any, _i: any) => {
+ {displayedMatches().map((match) => {- {pendingMatches().find(
- (d: any) => d.id === activeMatch()?.id,
- )
+ {pendingMatches().find(
+ (d) => d.id === activeMatch()?.id,
+ )This addresses the coding guideline: **/*.{ts,tsx}: Use TypeScript strict mode with extensive type safety for all code. Similar issues exist at lines 532 and 687–694.
🤖 Prompt for AI Agents
In `@packages/router-devtools-core/src/BaseTanStackRouterDevtoolsPanel.tsx` around
lines 282 - 289, The createMemo callbacks for pendingMatches and
displayedMatches are using implicit/any types which breaks strict TS guarantees;
update the createMemo calls for pendingMatches and displayedMatches to use
explicit generic type parameters (e.g., createMemo<AnyRouteMatch[]>) so the
returned arrays are strongly typed, remove any explicit any annotations in the
callbacks and downstream map/find usages, and ensure the expressions using
routerState(), router().matchRoutes(router().latestLocation) and
routerState().matches preserve the new typed signature.
| walkReplaceSegmentTree(newRoute, router.processedTree.segmentTree); | ||
| const filter = m => m.routeId === oldRoute.id; | ||
| if (router.state.matches.find(filter) || router.state.pendingMatches?.find(filter)) { | ||
| const hasPendingRouteMatch = router.state.status === "pending" && router.matchRoutes(router.latestLocation).some(filter); |
There was a problem hiding this comment.
Perhaps, defer having to ever having to run this IF condition which if successful checks for pending matches and keep it to only do so when the left-hand of the OR fails?
| const hasPendingRouteMatch = router.state.status === "pending" && router.matchRoutes(router.latestLocation).some(filter); | |
| const checkHasPendingRouteMatch = () => router.state.status === "pending" && router.matchRoutes(router.latestLocation).some(filter); | |
| if (router.state.matches(filter) || checkHasPendingRouteMatch()) { |
2 participants
Summary by CodeRabbit
Breaking Changes
Documentation
Tests
Devtools/UX