Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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
6 changes: 6 additions & 0 deletions .changeset/fancy-camels-rhyme.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@tanstack/react-router': patch
'@tanstack/router-core': patch
---

fix throw undefined on immediate redirect during route load
52 changes: 46 additions & 6 deletions packages/react-router/src/Match.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { ScrollRestoration } from './scroll-restoration'
import { ClientOnly } from './ClientOnly'
import type {
AnyRoute,
ControlledPromise,
ParsedLocation,
RootRouteOptions,
} from '@tanstack/router-core'
Expand Down Expand Up @@ -339,6 +340,16 @@ export const MatchInner = React.memo(function MatchInnerImpl({
return out
}

return <MatchInnerClient matchId={matchId} router={router} />
})

function MatchInnerClient({
matchId,
router,
}: {
matchId: string
router: ReturnType<typeof useRouter>
}) {
const matchStore = router.stores.activeMatchStoresById.get(matchId)
if (!matchStore) {
if (process.env.NODE_ENV !== 'production') {
Expand All @@ -349,11 +360,9 @@ export const MatchInner = React.memo(function MatchInnerImpl({

invariant()
}
// eslint-disable-next-line react-hooks/rules-of-hooks
const match = useStore(matchStore, (value) => value)
const routeId = match.routeId as string
const route = router.routesById[routeId] as AnyRoute
// eslint-disable-next-line react-hooks/rules-of-hooks
const key = React.useMemo(() => {
const remountFn =
(router.routesById[routeId] as AnyRoute).options.remountDeps ??
Expand All @@ -374,7 +383,6 @@ export const MatchInner = React.memo(function MatchInnerImpl({
router.routesById,
])

// eslint-disable-next-line react-hooks/rules-of-hooks
const out = React.useMemo(() => {
const Comp = route.options.component ?? router.options.defaultComponent
if (Comp) {
Expand All @@ -383,6 +391,38 @@ export const MatchInner = React.memo(function MatchInnerImpl({
return <Outlet />
}, [key, route.options.component, router.options.defaultComponent])

const suspensePromiseRef = React.useRef<ControlledPromise<void> | undefined>(
undefined,
)

React.useEffect(() => {
if (match.status !== 'pending' && match.status !== 'redirected') {
suspensePromiseRef.current?.resolve()
suspensePromiseRef.current = undefined
}
}, [match.status])

React.useEffect(() => {
return () => {
suspensePromiseRef.current?.resolve()
suspensePromiseRef.current = undefined
}
}, [match.id])

const getSuspensePromise = React.useCallback(() => {
const loadPromise = router.getMatch(match.id)?._nonReactive.loadPromise

if (loadPromise?.status === 'pending') {
return loadPromise
}

if (!suspensePromiseRef.current) {
suspensePromiseRef.current = createControlledPromise<void>()
}

return suspensePromiseRef.current
}, [router, match.id])

if (match._displayPending) {
throw router.getMatch(match.id)?._nonReactive.displayPendingPromise
}
Expand Down Expand Up @@ -413,7 +453,7 @@ export const MatchInner = React.memo(function MatchInnerImpl({
}
}
}
throw router.getMatch(match.id)?._nonReactive.loadPromise
throw getSuspensePromise()
}

if (match.status === 'notFound') {
Expand Down Expand Up @@ -442,7 +482,7 @@ export const MatchInner = React.memo(function MatchInnerImpl({
// false,
// 'Tried to render a redirected route match! This is a weird circumstance, please file an issue!',
// )
throw router.getMatch(match.id)?._nonReactive.loadPromise
throw getSuspensePromise()
}

if (match.status === 'error') {
Expand Down Expand Up @@ -471,7 +511,7 @@ export const MatchInner = React.memo(function MatchInnerImpl({
}

return out
})
}

/**
* Render the next child match in the route tree. Typically used inside
Expand Down
96 changes: 96 additions & 0 deletions packages/react-router/tests/loaders.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
fireEvent,
render,
screen,
waitFor,
} from '@testing-library/react'

import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'
Expand Down Expand Up @@ -940,3 +941,98 @@ test('reproducer for #6388 - rapid navigation between parameterized routes shoul
expect(paramPage).toHaveTextContent('Param Component 1 Done')
expect(loaderCompleteMock).toHaveBeenCalled()
})

test('keeps rendering the current pending route when a regular navigation aborts it', async () => {
const firstLoaderAborted = vi.fn()
const firstErrorComponentRenderCount = vi.fn()
let resolveSecondLoader: (() => void) | undefined

const rootRoute = createRootRoute({
component: () => <Outlet />,
})

const indexRoute = createRoute({
getParentRoute: () => rootRoute,
path: '/',
component: () => <div data-testid="home-page">Home page</div>,
})

const firstRoute = createRoute({
getParentRoute: () => rootRoute,
path: '/first',
pendingMs: 0,
loader: async ({ abortController }) => {
await new Promise<void>((_resolve, reject) => {
abortController.signal.addEventListener('abort', () => {
firstLoaderAborted()
reject(new DOMException('Aborted', 'AbortError'))
})
})

return 'first'
},
component: () => (
<div data-testid="first-page">{firstRoute.useLoaderData()}</div>
),
pendingComponent: () => (
<div data-testid="first-pending">Pending first route</div>
),
errorComponent: ({ error }) => {
firstErrorComponentRenderCount(error)
return <div data-testid="first-error">{String(error)}</div>
},
})

const secondRoute = createRoute({
getParentRoute: () => rootRoute,
path: '/second',
loader: async () => {
await new Promise<void>((resolve) => {
resolveSecondLoader = resolve
})

return 'second'
},
component: () => (
<div data-testid="second-page">{secondRoute.useLoaderData()}</div>
),
})

const routeTree = rootRoute.addChildren([indexRoute, firstRoute, secondRoute])
const router = createRouter({
routeTree,
history,
defaultPreload: false,
})

render(<RouterProvider router={router} />)
await act(() => router.latestLoadPromise)

expect(await screen.findByTestId('home-page')).toBeInTheDocument()

act(() => {
void router.navigate({ to: '/first' })
})
expect(await screen.findByTestId('first-pending')).toBeInTheDocument()

act(() => {
void router.navigate({ to: '/second' })
})
await act(() => sleep(0))

await waitFor(() => {
expect(resolveSecondLoader).toBeDefined()
})

expect(firstLoaderAborted).toHaveBeenCalled()
expect(screen.getByTestId('first-pending')).toBeInTheDocument()
expect(firstErrorComponentRenderCount).not.toHaveBeenCalled()
expect(screen.queryByTestId('first-error')).not.toBeInTheDocument()

act(() => {
resolveSecondLoader?.()
})

await act(() => router.latestLoadPromise)
expect(await screen.findByTestId('second-page')).toHaveTextContent('second')
})
73 changes: 73 additions & 0 deletions packages/react-router/tests/router.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1423,6 +1423,79 @@ describe('invalidate', () => {
})
})

it('keeps rendering a suspense fallback when invalidate({ forcePending: true }) reloads a route', async () => {
const history = createMemoryHistory({
initialEntries: ['/force-pending'],
})
const errorComponentRenderCount = vi.fn()
let shouldSuspendReload = false
let resolveReload: (() => void) | undefined

const rootRoute = createRootRoute({
component: () => <Outlet />,
})

const forcePendingRoute = createRoute({
getParentRoute: () => rootRoute,
path: '/force-pending',
pendingMs: 0,
pendingMinMs: 10,
loader: async () => {
if (shouldSuspendReload) {
await new Promise<void>((resolve) => {
resolveReload = resolve
})
}

return 'done'
},
component: () => (
<div data-testid="force-pending-route">
{forcePendingRoute.useLoaderData()}
</div>
),
pendingComponent: () => (
<div data-testid="force-pending-fallback">Pending...</div>
),
errorComponent: ({ error }) => {
errorComponentRenderCount(error)
return <div data-testid="force-pending-error">{String(error)}</div>
},
})

const router = createRouter({
routeTree: rootRoute.addChildren([forcePendingRoute]),
history,
})

render(<RouterProvider router={router} />)

await act(() => router.load())
expect(await screen.findByTestId('force-pending-route')).toHaveTextContent(
'done',
)

shouldSuspendReload = true
act(() => {
void router.invalidate({ forcePending: true })
})

expect(
await screen.findByTestId('force-pending-fallback'),
).toBeInTheDocument()
expect(errorComponentRenderCount).not.toHaveBeenCalled()
expect(screen.queryByTestId('force-pending-error')).not.toBeInTheDocument()

act(() => {
resolveReload?.()
})

await act(() => router.latestLoadPromise)
expect(await screen.findByTestId('force-pending-route')).toHaveTextContent(
'done',
)
})

/**
* Regression test:
* - When a route loader throws `notFound()`, the match enters a `'notFound'` status.
Expand Down
2 changes: 0 additions & 2 deletions packages/router-core/src/load-matches.ts
Original file line number Diff line number Diff line change
Expand Up @@ -841,7 +841,6 @@ const loadRouteMatch = async (
match._nonReactive.loaderPromise?.resolve()
match._nonReactive.loadPromise?.resolve()
match._nonReactive.loaderPromise = undefined
match._nonReactive.loadPromise = undefined
} catch (err) {
if (isRedirect(err)) {
await inner.router.navigate(err.options)
Expand Down Expand Up @@ -940,7 +939,6 @@ const loadRouteMatch = async (
if (!loaderIsRunningAsync) {
match._nonReactive.loaderPromise?.resolve()
match._nonReactive.loadPromise?.resolve()
match._nonReactive.loadPromise = undefined
}

clearTimeout(match._nonReactive.pendingTimeout)
Expand Down