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
180 changes: 97 additions & 83 deletions packages/router-core/src/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1248,11 +1248,11 @@ export class RouterCore<
}

emit: EmitFn = (routerEvent) => {
this.subscribers.forEach((listener) => {
for (const listener of this.subscribers) {
if (listener.eventType === routerEvent.type) {
listener.fn(routerEvent)
}
})
}
}

/**
Expand Down Expand Up @@ -1765,24 +1765,24 @@ export class RouterCore<
}

cancelMatches = () => {
this.stores.pendingMatchesId.state.forEach((matchId) => {
for (const matchId of this.stores.pendingMatchesId.state) {
this.cancelMatch(matchId)
})
}

this.stores.matchesId.state.forEach((matchId) => {
for (const matchId of this.stores.matchesId.state) {
if (this.stores.pendingMatchStoresById.has(matchId)) {
return
continue
}

const match = this.stores.activeMatchStoresById.get(matchId)?.state
if (!match) {
return
continue
}

if (match.status === 'pending' || match.isFetching === 'loader') {
this.cancelMatch(matchId)
}
})
}
}

/**
Expand Down Expand Up @@ -1915,7 +1915,7 @@ export class RouterCore<
let nextSearch = fromSearch
if (opts._includeValidateSearch && this.options.search?.strict) {
const validatedSearch = {}
destRoutes.forEach((route) => {
for (const route of destRoutes) {
if (route.options.validateSearch) {
try {
Object.assign(
Expand All @@ -1929,7 +1929,7 @@ export class RouterCore<
// ignore errors here because they are already handled in matchRoutes
}
}
})
}
nextSearch = validatedSearch
}

Expand Down Expand Up @@ -2093,13 +2093,13 @@ export class RouterCore<
'__TSR_index',
'__hashScrollIntoViewOptions',
] as const
ignoredProps.forEach((prop) => {
for (const prop of ignoredProps) {
;(next.state as any)[prop] = this.latestLocation.state[prop]
})
}
const isEqual = deepEqual(next.state, this.latestLocation.state)
ignoredProps.forEach((prop) => {
for (const prop of ignoredProps) {
delete next.state[prop]
})
}
return isEqual
}

Expand Down Expand Up @@ -2418,54 +2418,42 @@ export class RouterCore<
// exitingMatches uses match.id (routeId + params + loaderDeps) so
// navigating /foo?page=1 → /foo?page=2 correctly caches the page=1 entry.
let exitingMatches: Array<AnyRouteMatch> | null = null

// Lifecycle-hook identity uses routeId only so that navigating between
// different params/deps of the same route fires onStay (not onLeave+onEnter).
let hookExitingMatches: Array<AnyRouteMatch> | null = null
let hookEnteringMatches: Array<AnyRouteMatch> | null = null
let hookStayingMatches: Array<AnyRouteMatch> | null = null
let pendingMatches: Array<AnyRouteMatch> = []
let currentMatches: Array<AnyRouteMatch> = []
let pendingRouteIds: Set<string> | null = null
let activeRouteIds: Set<string> | null = null

this.batch(() => {
const pendingMatches =
this.stores.pendingMatchesSnapshot.state
const mountPending = pendingMatches.length
const currentMatches =
this.stores.activeMatchesSnapshot.state

exitingMatches = mountPending
? currentMatches.filter(
(match) =>
!this.stores.pendingMatchStoresById.has(match.id),
)
: null

// Lifecycle-hook identity: routeId only (route presence in tree)
// Build routeId sets from pools to avoid derived stores.
const pendingRouteIds = new Set<string>()
for (const s of this.stores.pendingMatchStoresById.values()) {
if (s.routeId) pendingRouteIds.add(s.routeId)
}
const activeRouteIds = new Set<string>()
for (const s of this.stores.activeMatchStoresById.values()) {
if (s.routeId) activeRouteIds.add(s.routeId)
pendingMatches = this.stores.pendingMatchesSnapshot.state
currentMatches = this.stores.activeMatchesSnapshot.state

if (pendingMatches.length) {
activeRouteIds = new Set<string>()
for (const match of currentMatches) {
activeRouteIds.add(match.routeId)
}

pendingRouteIds = new Set<string>()
exitingMatches = []

for (const match of pendingMatches) {
pendingRouteIds.add(match.routeId)
}

for (const match of currentMatches) {
if (
!this.stores.pendingMatchStoresById.has(match.id) &&
match.status !== 'error' &&
match.status !== 'notFound' &&
match.status !== 'redirected'
) {
exitingMatches.push(match)
}
}
} else {
exitingMatches = null
}
Comment on lines +2453 to 2455
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.

else not necessary, exitingMatches is already null

Suggested change
} else {
exitingMatches = null
}
}


hookExitingMatches = mountPending
? currentMatches.filter(
(match) => !pendingRouteIds.has(match.routeId),
)
: null
hookEnteringMatches = mountPending
? pendingMatches.filter(
(match) => !activeRouteIds.has(match.routeId),
)
: null
hookStayingMatches = mountPending
? pendingMatches.filter((match) =>
activeRouteIds.has(match.routeId),
)
: currentMatches

this.stores.isLoading.setState(() => false)
this.stores.loadedAt.setState(() => Date.now())
/**
Expand All @@ -2474,31 +2462,47 @@ export class RouterCore<
* deliberately excluded from `cachedMatches` so that subsequent invalidations
* or reloads re-run their loaders instead of reusing the failed/not-found data.
*/
if (mountPending) {
if (pendingMatches.length) {
this.stores.setActiveMatches(pendingMatches)
this.stores.setPendingMatches([])
this.stores.setCachedMatches([
...this.stores.cachedMatchesSnapshot.state,
...exitingMatches!.filter(
(d) =>
d.status !== 'error' &&
d.status !== 'notFound' &&
d.status !== 'redirected',
),
...exitingMatches!,
])
this.clearExpiredCache()
}
})

//
for (const [matches, hook] of [
[hookExitingMatches, 'onLeave'],
[hookEnteringMatches, 'onEnter'],
[hookStayingMatches, 'onStay'],
] as const) {
if (!matches) continue
for (const match of matches as Array<AnyRouteMatch>) {
this.looseRoutesById[match.routeId]!.options[hook]?.(
// Lifecycle-hook identity uses routeId only so that navigating between
// different params/deps of the same route fires onStay (not onLeave+onEnter).
const nextPendingRouteIds =
pendingRouteIds as Set<string> | null
const nextActiveRouteIds =
activeRouteIds as Set<string> | null
Comment on lines +2478 to +2481
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.

This doesn't seem necessary, as casts to the same type those variables already have


if (nextPendingRouteIds && nextActiveRouteIds) {
for (const match of currentMatches) {
if (!nextPendingRouteIds.has(match.routeId)) {
this.looseRoutesById[match.routeId]!.options.onLeave?.(
match,
)
}
}

for (const match of pendingMatches) {
if (nextActiveRouteIds.has(match.routeId)) {
this.looseRoutesById[match.routeId]!.options.onStay?.(
match,
)
} else {
this.looseRoutesById[match.routeId]!.options.onEnter?.(
match,
)
}
}
} else {
for (const match of currentMatches) {
this.looseRoutesById[match.routeId]!.options.onStay?.(
match,
)
}
Expand All @@ -2525,9 +2529,7 @@ export class RouterCore<
? redirect.status
: notFound
? 404
: this.stores.activeMatchesSnapshot.state.some(
(d) => d.status === 'error',
)
: this.hasErrorMatch()
? 500
: 200

Expand Down Expand Up @@ -2561,9 +2563,7 @@ export class RouterCore<
let newStatusCode: number | undefined = undefined
if (this.hasNotFoundMatch()) {
newStatusCode = 404
} else if (
this.stores.activeMatchesSnapshot.state.some((d) => d.status === 'error')
) {
} else if (this.hasErrorMatch()) {
newStatusCode = 500
}
if (newStatusCode !== undefined) {
Expand Down Expand Up @@ -2930,10 +2930,24 @@ export class RouterCore<

serverSsr?: ServerSsr

hasErrorMatch = () => {
for (const match of this.stores.activeMatchesSnapshot.state) {
if (match.status === 'error') {
return true
}
}

return false
}

hasNotFoundMatch = () => {
return this.stores.activeMatchesSnapshot.state.some(
(d) => d.status === 'notFound' || d.globalNotFound,
)
for (const match of this.stores.activeMatchesSnapshot.state) {
if (match.status === 'notFound' || match.globalNotFound) {
return true
}
}

return false
}
}

Expand Down
101 changes: 100 additions & 1 deletion packages/router-core/tests/callbacks.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { describe, expect, it, test, vi } from 'vitest'
import { createMemoryHistory } from '@tanstack/history'
import { BaseRootRoute, BaseRoute } from '../src'
import { BaseRootRoute, BaseRoute, createControlledPromise } from '../src'
import { createTestRouter } from './routerTestUtils'

describe('callbacks', () => {
Expand Down Expand Up @@ -157,6 +157,105 @@ describe('callbacks', () => {
expect(onLeave).toHaveBeenCalledTimes(0)
expect(onStay).toHaveBeenCalledTimes(2)
})

it('treats a navigation that changes the match instance and match id for the same routeId as stay, not leave plus enter', async () => {
const onEnter = vi.fn()
const onLeave = vi.fn()
const onStay = vi.fn()
const loaderPromises: Array<
ReturnType<typeof createControlledPromise<void>>
> = []

const rootRoute = new BaseRootRoute({})
const fooRoute = new BaseRoute({
getParentRoute: () => rootRoute,
path: '/foo',
loaderDeps: ({ search }: { search: Record<string, unknown> }) => ({
page: search['page'],
}),
onEnter,
onLeave,
onStay,
loader: () => {
const promise = createControlledPromise<void>()
loaderPromises.push(promise)
return promise
},
})

const router = createTestRouter({
routeTree: rootRoute.addChildren([fooRoute]),
history: createMemoryHistory(),
})

const initialNavigation = router.navigate({
to: '/foo',
search: { page: '1' },
})

expect(loaderPromises).toHaveLength(1)
loaderPromises[0]!.resolve()
await initialNavigation

onEnter.mockClear()
onLeave.mockClear()
onStay.mockClear()

const currentMatches = router.stores.activeMatchesSnapshot.state
const currentFooMatch = currentMatches.find(
(match) => match.routeId === fooRoute.id,
)

expect(currentFooMatch).toBeDefined()
expect(router.looseRoutesById[fooRoute.id]!.options.onStay).toBe(onStay)
expect(router.looseRoutesById[fooRoute.id]!.options.onLeave).toBe(onLeave)
expect(router.looseRoutesById[fooRoute.id]!.options.onEnter).toBe(onEnter)

const secondNavigation = router.navigate({
to: '/foo',
search: { page: '2' },
})

expect(loaderPromises).toHaveLength(2)

let pendingMatches = router.stores.pendingMatchesSnapshot.state
for (let index = 0; index < 10 && pendingMatches.length === 0; index++) {
await Promise.resolve()
pendingMatches = router.stores.pendingMatchesSnapshot.state
}

const pendingFooMatch = pendingMatches.find(
(match) => match.routeId === fooRoute.id,
)

expect(pendingFooMatch).toBeDefined()
expect(pendingFooMatch).not.toBe(currentFooMatch)
expect(pendingFooMatch!.id).not.toBe(currentFooMatch!.id)
expect(pendingFooMatch!.routeId).toBe(currentFooMatch!.routeId)

const pendingRouteIds = new Set(
pendingMatches.map((match) => match.routeId),
)
const activeRouteIds = new Set(
currentMatches.map((match) => match.routeId),
)

expect(pendingRouteIds.has(fooRoute.id)).toBe(true)
expect(activeRouteIds.has(fooRoute.id)).toBe(true)

loaderPromises[1]!.resolve()
await secondNavigation

expect(onStay).toHaveBeenCalledTimes(1)
expect(onStay).toHaveBeenCalledWith(
expect.objectContaining({
id: pendingFooMatch!.id,
routeId: fooRoute.id,
}),
)
expect(onLeave).toHaveBeenCalledTimes(0)
expect(onEnter).toHaveBeenCalledTimes(0)
})
})

// Regression tests: switching lifecycle hooks to use routeId must NOT break
Expand Down
Loading