-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
refactor(router-core): store updates are reserved for reactivity updates #6563
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
refactor(router-core): store updates are reserved for reactivity updates #6563
Conversation
📝 WalkthroughWalkthroughPublic RouterState.pendingMatches was removed and pending-match tracking was internalized in router-core using private maps, indices, queued match update maps, and a microtask flush. Documentation, framework adapters, HMR, devtools, SSR hydration, and tests were updated to stop referencing public pendingMatches. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller as Caller (load / beforeLoad / update)
participant Router as RouterCore
participant Queue as __queuedMatchUpdates
participant Flush as MicrotaskFlush
participant Store as State Commit / Subscribers
Caller->>Router: request match update (updateMatch / load)
Router->>Queue: queueMatchUpdate(id, match)
Note right of Queue: updates buffered in maps (__queuedMatchUpdates / __queuedCachedMatchUpdates)
Router->>Flush: schedule microtask flush (if not already scheduled)
Flush->>Router: run flushQueuedMatchUpdates()
Router->>Router: updateMatchInternal -> apply to __matchesById / __pendingMatches
Router->>Router: rebuildIndices / rebuildMatchesById
Router->>Store: commit new matches/state
Store->>Caller: notify subscribers, devtools, plugins
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
View your CI Pipeline Execution ↗ for commit a288124
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/vue-router/src/useMatch.tsx`:
- Around line 75-76: The select callback in useMatch's selector uses explicit
any for state and matches; fix by calling useRouterState with the TRouter
generic (useRouterState<TRouter>()) so the selector infers
RouterState<TRouter['routeTree']>, and update the select signature to use that
inferred type (or explicitly type state as RouterState<TRouter['routeTree']>) so
state.matches and the match variable are properly typed; reference the
useRouterState call and the select function that accesses state.matches (mirror
the pattern used in Matches.tsx where RouterState<TRouter['routeTree']> is
used).
| select: (state: any) => { | ||
| const match = state.matches.find((d: any) => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, locate and read the file to see the context around lines 75-76
fd -type f -name 'useMatch.tsx' packages/vue-router/src/ | head -1 | xargs wc -lRepository: TanStack/router
Length of output: 248
🏁 Script executed:
# Read the specific section of the file
cat -n packages/vue-router/src/useMatch.tsx | sed -n '70,85p'Repository: TanStack/router
Length of output: 748
🏁 Script executed:
# Check if useRouterState is defined and what types it exposes
fd -type f -name '*.tsx' -o -name '*.ts' packages/vue-router/src/ | xargs rg -l "useRouterState" | head -5Repository: TanStack/router
Length of output: 524
🏁 Script executed:
# Find useRouterState definition in Vue router
fd -type f -name '*.tsx' -o -name '*.ts' packages/vue-router/src/ | xargs rg -l "export.*useRouterState" | head -3Repository: TanStack/router
Length of output: 368
🏁 Script executed:
# Check the imports and type definitions at the top of useMatch.tsx
cat -n packages/vue-router/src/useMatch.tsx | head -40Repository: TanStack/router
Length of output: 1370
🏁 Script executed:
# Look at useRouterState test file to understand expected typing
cat -n packages/vue-router/tests/useRouterState.test-d.tsx | head -80Repository: TanStack/router
Length of output: 1947
🏁 Script executed:
# Read the useRouterState implementation
cat -n packages/vue-router/src/useRouterState.tsxRepository: TanStack/router
Length of output: 2212
🏁 Script executed:
# Check the RouterState type definition
rg -n "export.*type RouterState" packages/vue-router/src/Repository: TanStack/router
Length of output: 41
🏁 Script executed:
# Read the full function signature of useMatch to understand available generics
cat -n packages/vue-router/src/useMatch.tsx | sed -n '50,85p'Repository: TanStack/router
Length of output: 1427
🏁 Script executed:
# Also check if there are other usages of useRouterState in the codebase to see the pattern
rg -n "useRouterState\(" packages/vue-router/src/ -A2Repository: TanStack/router
Length of output: 6007
🏁 Script executed:
# Check if removing `any` would actually type correctly without the generic parameter
# by looking at how TypeScript would infer it
rg -n "useRouterState<" packages/vue-router/src/ -B1 -A2Repository: TanStack/router
Length of output: 346
🏁 Script executed:
# Verify if TRouter is indeed available in the useMatch function scope
cat -n packages/vue-router/src/useMatch.tsx | sed -n '54,75p'Repository: TanStack/router
Length of output: 865
🏁 Script executed:
# Check one working example from Matches.tsx that explicitly types
cat -n packages/vue-router/src/Matches.tsx | sed -n '310,320p'Repository: TanStack/router
Length of output: 514
Pass TRouter generic to useRouterState for proper type inference.
Line 75-76 uses explicit any types. The root cause is that useRouterState is called without the <TRouter> generic parameter. Either pass <TRouter> to useRouterState and rely on inference, or explicitly type the state parameter as RouterState<TRouter['routeTree']> to match the pattern in Matches.tsx:315.
♻️ Suggested change
- select: (state: any) => {
- const match = state.matches.find((d: any) =>
+ select: (state) => {
+ const match = state.matches.find((d) =>Should be combined with passing the generic:
- const matchState = useRouterState({
+ const matchState = useRouterState<TRouter>({
select: (state) => {🤖 Prompt for AI Agents
In `@packages/vue-router/src/useMatch.tsx` around lines 75 - 76, The select
callback in useMatch's selector uses explicit any for state and matches; fix by
calling useRouterState with the TRouter generic (useRouterState<TRouter>()) so
the selector infers RouterState<TRouter['routeTree']>, and update the select
signature to use that inferred type (or explicitly type state as
RouterState<TRouter['routeTree']>) so state.matches and the match variable are
properly typed; reference the useRouterState call and the select function that
accesses state.matches (mirror the pattern used in Matches.tsx where
RouterState<TRouter['routeTree']> is used).
This PR proposes that we don't expose the
pendingMatches. These are updated many times during the loading process, when in reality what we want is "nothing while it loads, and then mount the routes". Each store update triggers every router hook on the page every time, and some navigations can cause many store updates because of the presence ofpendingMatchesin the store.SSR benchmark (not the main target of this PR)
before
after
Summary by CodeRabbit
Breaking Changes
Documentation
New Features
Devtools
Tests
✏️ Tip: You can customize this high-level summary in your review settings.