Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 45 additions & 2 deletions packages/vue-router/src/useMatch.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,49 @@ import type {
ThrowOrOptional,
} from '@tanstack/router-core'

const functionalMatchStoreRefs = new WeakMap<
object,
WeakMap<object, Readonly<Vue.Ref<any>>>
>()
Comment on lines +17 to +20

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== file existence and relevant sections =="
if [ -f packages/vue-router/src/useMatch.tsx ]; then
  wc -l packages/vue-router/src/useMatch.tsx
  sed -n '1,90p' packages/vue-router/src/useMatch.tsx
  sed -n '140,185p' packages/vue-router/src/useMatch.tsx
else
  echo "packages/vue-router/src/useMatch.tsx not found"
  fd -a 'useMatch\.(tsx|ts)$' . || true
fi

echo
echo "== search for WeakMap functionalMatchStoreRefs usage =="
rg -n "functionalMatchStoreRefs|Readonly<Vue\.Ref<unknown>|Readonly<Vue\.Ref<any>>" packages/vue-router/src/useMatch.tsx . --glob '*.tsx' --glob '*.ts' || true

echo
echo "== tsconfig strict-related snippets =="
fd -a 'tsconfig.*json|package.json' . | while read -r f; do
  echo "--- $f"
  rg -n '"strict"|"moduleResolution"|"types"|"vue"' "$f" || true
done

Repository: TanStack/router

Length of output: 50372


Remove any from the cached ref type.

Vue.Ref<any> disables type checking for every cached store value. Use unknown for the cache boundary, then keep the precise cast at the helper return boundary.

🤖 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/vue-router/src/useMatch.tsx` around lines 17 - 20, Update the
functionalMatchStoreRefs WeakMap value type to use Readonly<Vue.Ref<unknown>>
instead of Readonly<Vue.Ref<any>>, while preserving the precise type cast at the
helper’s return boundary.

Source: Coding guidelines


type ComponentEffectScope = {
run: <T>(fn: () => T) => T | undefined
}

function useMatchStore<TStore extends Parameters<typeof useStore>[0]>(
matchStore: TStore,
): Readonly<Vue.Ref<ReturnType<TStore['get']>>> {
const instance = Vue.getCurrentInstance()

if (
!instance ||
typeof instance.type !== 'function' ||
Vue.getCurrentScope()
) {
return useStore(matchStore)
}

let refsByStore = functionalMatchStoreRefs.get(instance)
if (!refsByStore) {
refsByStore = new WeakMap()
functionalMatchStoreRefs.set(instance, refsByStore)
}

let match = refsByStore.get(matchStore)
if (!match) {
// Vue runs plain functional components outside their effect scope. Re-enter
// that scope so Vue owns the watcher, then reuse it on later renders of the
// same component instead of subscribing again on every render.
const componentScope = (
instance as unknown as { scope: ComponentEffectScope }
).scope
match = componentScope.run(() => useStore(matchStore))!
refsByStore.set(matchStore, match)
}

return match as Readonly<Vue.Ref<ReturnType<TStore['get']>>>
}

export interface UseMatchBaseOptions<
TRouter extends AnyRouter,
TFrom,
Expand Down Expand Up @@ -117,13 +160,13 @@ export function useMatch<
if (opts.from) {
// routeId case: subscribe to the stable per-route presentation atom.
const matchStore = router.stores.getMatchStore(opts.from)
match = useStore(matchStore)
match = useMatchStore(matchStore)
} else {
// Nearest-match case: use the routeId from context for stable lookup.
// The routeId is provided by the nearest Match component and doesn't
// change for the component's lifetime, so the store is stable.
if (nearestRouteId) {
match = useStore(router.stores.getMatchStore(nearestRouteId))
match = useMatchStore(router.stores.getMatchStore(nearestRouteId))
} else {
// No route context — will fall through to error handling below
match = Vue.ref(undefined) as Readonly<Vue.Ref<undefined>>
Expand Down
127 changes: 127 additions & 0 deletions packages/vue-router/tests/match-subscription-cleanup.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
import { afterEach, expect, test } from 'vitest'
import { cleanup, fireEvent, render, screen } from '@testing-library/vue'
import {
Outlet,
RouterProvider,
createMemoryHistory,
createRootRoute,
createRoute,
createRouter,
} from '../src'

afterEach(() => {
cleanup()
})

test('releases match-store subscriptions when route params replace a match', async () => {
const observedIds: Array<string> = []
const rootRoute = createRootRoute({
validateSearch: (search: Record<string, unknown>) => ({
q: typeof search.q === 'string' ? search.q : '',
}),
loaderDeps: ({ search }) => ({ q: search.q }),
loader: ({ deps }) => `root:${deps.q}`,
component: RootComponent,
})
function RootComponent() {
const rootData = rootRoute.useLoaderData()
return (
<section data-testid="root-data">
{rootData.value}
<Outlet />
</section>
)
}
const itemRoute = createRoute({
getParentRoute: () => rootRoute,
path: '/items/$id',
validateSearch: (search: Record<string, unknown>) => ({
q: typeof search.q === 'string' ? search.q : '',
}),
loaderDeps: ({ search }) => ({ q: search.q }),
loader: ({ params, deps }) => `${params.id}:${deps.q}`,
component: ItemComponent,
})
function ItemComponent() {
const id = itemRoute.useLoaderData()
const rootData = rootRoute.useLoaderData()
return (
<button
data-testid="item-id"
onClick={() => observedIds.push(`${id.value}|${rootData.value}`)}
>
{`${id.value}|${rootData.value}`}
</button>
)
}
const router = createRouter({
routeTree: rootRoute.addChildren([itemRoute]),
history: createMemoryHistory({ initialEntries: ['/items/initial'] }),
defaultGcTime: 0,
})
const matchStore = router.stores.getMatchStore('/items/$id')
const originalSubscribe = matchStore.subscribe.bind(matchStore)
let activeSubscriptions = 0
let subscriptions = 0
let unsubscriptions = 0

matchStore.subscribe = (observer) => {
subscriptions++
activeSubscriptions++
const subscription = Reflect.apply(originalSubscribe, matchStore, [
observer,
]) as ReturnType<typeof matchStore.subscribe>
let active = true

return {
unsubscribe() {
if (active) {
active = false
activeSubscriptions--
unsubscriptions++
}
subscription.unsubscribe()
},
}
}

render(<RouterProvider router={router} />)
expect(await screen.findByTestId('item-id')).toHaveTextContent(
'initial:|root:',
)
const initialSubscriptions = activeSubscriptions

for (let index = 0; index < 50; index++) {
await router.navigate({
to: '/items/$id',
params: { id: `item-${index}` },
search: { q: `query-${index}` },
replace: true,
})
}
expect(screen.getByTestId('item-id')).toHaveTextContent(
'item-49:query-49|root:query-49',
)
expect(activeSubscriptions).toBe(initialSubscriptions)

const subscriptionsAfterParamChanges = subscriptions
for (let index = 0; index < 50; index++) {
await router.navigate({
to: '/items/$id',
params: { id: 'item-49' },
search: { q: `same-param-query-${index}` },
replace: true,
})
}
expect(screen.getByTestId('item-id')).toHaveTextContent(
'item-49:same-param-query-49|root:same-param-query-49',
)
await fireEvent.click(screen.getByTestId('item-id'))
expect(observedIds).toEqual([
'item-49:same-param-query-49|root:same-param-query-49',
])

expect(activeSubscriptions).toBe(initialSubscriptions)
expect(subscriptions - unsubscriptions).toBe(initialSubscriptions)
expect(subscriptions).toBe(subscriptionsAfterParamChanges)
})
Loading