diff --git a/packages/react-router/src/CatchBoundary.tsx b/packages/react-router/src/CatchBoundary.tsx index 98743f9447..5a2c393feb 100644 --- a/packages/react-router/src/CatchBoundary.tsx +++ b/packages/react-router/src/CatchBoundary.tsx @@ -38,7 +38,11 @@ class CatchBoundaryImpl extends React.Component<{ }) => React.ReactNode onCatch?: (error: Error, errorInfo: ErrorInfo) => void }> { - state = { error: null } as { error: Error | null; resetKey?: string | number } + state = { error: null, hasError: false } as { + error: Error | null + hasError: boolean + resetKey?: string | number + } static getDerivedStateFromProps( props: { getResetKey: () => string | number }, @@ -53,10 +57,10 @@ class CatchBoundaryImpl extends React.Component<{ return { resetKey } } static getDerivedStateFromError(error: Error) { - return { error } + return { error, hasError: true } } reset() { - this.setState({ error: null }) + this.setState({ error: null, hasError: false }) } componentDidCatch(error: Error, errorInfo: ErrorInfo) { if (this.props.onCatch) { @@ -65,7 +69,7 @@ class CatchBoundaryImpl extends React.Component<{ } render() { return this.props.children({ - error: this.state.error, + error: this.state.hasError ? this.state.error : null, reset: () => { this.reset() }, diff --git a/packages/react-router/src/Match.tsx b/packages/react-router/src/Match.tsx index 5de5546aeb..a31937001d 100644 --- a/packages/react-router/src/Match.tsx +++ b/packages/react-router/src/Match.tsx @@ -368,7 +368,12 @@ export const MatchInner = React.memo(function MatchInnerImpl({ invariant() } - throw getMatchPromise(match, 'loadPromise') + + const loadPromise = getMatchPromise(match, 'loadPromise') + + if (loadPromise) { + throw loadPromise + } } if (match.status === 'error') { @@ -491,7 +496,17 @@ export const MatchInner = React.memo(function MatchInnerImpl({ invariant() } - throw getMatchPromise(match, 'loadPromise') + const loadPromise = getMatchPromise(match, 'loadPromise') + + // The loadPromise may have been cleared by the time this stale render + // runs (race: the redirect navigation settled before MatchInner + // re-rendered). Throwing undefined would escape CatchBoundary (falsy + // check) and unmount the whole app. Instead, let React render the + // component — the redirect has already been processed and the match + // will be replaced on the next store update. + if (loadPromise) { + throw loadPromise + } } if (match.status === 'error') { diff --git a/packages/react-router/tests/issue-7753-redirect-loadPromise-race.test.tsx b/packages/react-router/tests/issue-7753-redirect-loadPromise-race.test.tsx new file mode 100644 index 0000000000..83570582c2 --- /dev/null +++ b/packages/react-router/tests/issue-7753-redirect-loadPromise-race.test.tsx @@ -0,0 +1,123 @@ +import * as React from 'react' +import { cleanup, render, screen } from '@testing-library/react' +import { afterEach, expect, test, vi } from 'vitest' + +import { + Outlet, + RouterProvider, + createMemoryHistory, + createRootRoute, + createRoute, + createRouter, + redirect, +} from '../src' +import { sleep } from './utils' + +afterEach(() => { + vi.restoreAllMocks() + cleanup() +}) + +// https://github.com/TanStack/router/issues/7753 +// MatchInner can throw undefined when a redirected match's loadPromise was +// already cleared — escapes CatchBoundary and unmounts the whole app. +// +// The race: an async beforeLoad throws redirect(), pending UI renders +// (defaultPendingMs: 0), and by the time MatchInner re-renders with +// status === 'redirected', the loadPromise has been resolved and cleared +// by load-matches.ts. Throwing undefined escapes CatchBoundary (falsy +// check) and unmounts the app with a white screen. +test('redirected match with cleared loadPromise does not throw undefined', async () => { + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}) + + const rootRoute = createRootRoute({ + component: () => , + }) + + const slowRedirectRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/', + beforeLoad: async () => { + // Simulate an async operation (e.g. token refresh) that takes long + // enough for pending UI to commit and the loadPromise to be cleared + await sleep(50) + throw redirect({ to: '/target', replace: true }) + }, + component: () =>
Source
, + }) + + const targetRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/target', + component: () =>
Target
, + }) + + const router = createRouter({ + routeTree: rootRoute.addChildren([slowRedirectRoute, targetRoute]), + defaultPendingMs: 0, + defaultPendingComponent: () =>
loading
, + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + + render() + + // Should land on the target route, not white-screen + expect( + await screen.findByTestId('target', undefined, { timeout: 5_000 }), + ).toBeInTheDocument() + expect(router.state.location.pathname).toBe('/target') + // No console.error means CatchBoundary didn't receive an uncaught throw + expect(consoleError).not.toHaveBeenCalled() +}) + +// A more aggressive variant: multiple async beforeLoad redirects in a chain, +// where each intermediate match's loadPromise may be cleared before the +// next render. This exercises the race more broadly. +test('chained async beforeLoad redirects with pending UI do not throw undefined', async () => { + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}) + + const rootRoute = createRootRoute({ + component: () => , + }) + + const firstRedirect = createRoute({ + getParentRoute: () => rootRoute, + path: '/', + beforeLoad: async () => { + await sleep(20) + throw redirect({ to: '/middle', replace: true }) + }, + component: () =>
First
, + }) + + const middleRedirect = createRoute({ + getParentRoute: () => rootRoute, + path: '/middle', + beforeLoad: async () => { + await sleep(30) + throw redirect({ to: '/final', replace: true }) + }, + component: () =>
Middle
, + }) + + const finalRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/final', + component: () =>
Final
, + }) + + const router = createRouter({ + routeTree: rootRoute.addChildren([firstRedirect, middleRedirect, finalRoute]), + defaultPendingMs: 0, + defaultPendingComponent: () =>
loading
, + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + + render() + + expect( + await screen.findByTestId('final', undefined, { timeout: 5_000 }), + ).toBeInTheDocument() + expect(router.state.location.pathname).toBe('/final') + expect(consoleError).not.toHaveBeenCalled() +})