diff --git a/.changeset/fancy-camels-rhyme.md b/.changeset/fancy-camels-rhyme.md new file mode 100644 index 00000000000..0515d57fd9f --- /dev/null +++ b/.changeset/fancy-camels-rhyme.md @@ -0,0 +1,6 @@ +--- +'@tanstack/react-router': patch +'@tanstack/router-core': patch +--- + +fix throw undefined on immediate redirect during route load diff --git a/packages/react-router/src/Match.tsx b/packages/react-router/src/Match.tsx index 04e354d0835..79e26ffdb5f 100644 --- a/packages/react-router/src/Match.tsx +++ b/packages/react-router/src/Match.tsx @@ -413,7 +413,10 @@ export const MatchInner = React.memo(function MatchInnerImpl({ } } } - throw router.getMatch(match.id)?._nonReactive.loadPromise + if (match._nonReactive.pendingRenderPromise?.status !== 'pending') { + match._nonReactive.pendingRenderPromise = createControlledPromise() + } + throw match._nonReactive.pendingRenderPromise } if (match.status === 'notFound') { diff --git a/packages/react-router/tests/loaders.test.tsx b/packages/react-router/tests/loaders.test.tsx index d9b5968d723..e06e1fa7035 100644 --- a/packages/react-router/tests/loaders.test.tsx +++ b/packages/react-router/tests/loaders.test.tsx @@ -5,6 +5,7 @@ import { fireEvent, render, screen, + waitFor, } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest' @@ -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: () => , + }) + + const indexRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/', + component: () =>
Home page
, + }) + + const firstRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/first', + pendingMs: 0, + loader: async ({ abortController }) => { + await new Promise((_resolve, reject) => { + abortController.signal.addEventListener('abort', () => { + firstLoaderAborted() + reject(new DOMException('Aborted', 'AbortError')) + }) + }) + + return 'first' + }, + component: () => ( +
{firstRoute.useLoaderData()}
+ ), + pendingComponent: () => ( +
Pending first route
+ ), + errorComponent: ({ error }) => { + firstErrorComponentRenderCount(error) + return
{String(error)}
+ }, + }) + + const secondRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/second', + loader: async () => { + await new Promise((resolve) => { + resolveSecondLoader = resolve + }) + + return 'second' + }, + component: () => ( +
{secondRoute.useLoaderData()}
+ ), + }) + + const routeTree = rootRoute.addChildren([indexRoute, firstRoute, secondRoute]) + const router = createRouter({ + routeTree, + history, + defaultPreload: false, + }) + + render() + 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') +}) diff --git a/packages/react-router/tests/router.test.tsx b/packages/react-router/tests/router.test.tsx index 99a6e36d539..6b219871025 100644 --- a/packages/react-router/tests/router.test.tsx +++ b/packages/react-router/tests/router.test.tsx @@ -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: () => , + }) + + const forcePendingRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/force-pending', + pendingMs: 0, + pendingMinMs: 10, + loader: async () => { + if (shouldSuspendReload) { + await new Promise((resolve) => { + resolveReload = resolve + }) + } + + return 'done' + }, + component: () => ( +
+ {forcePendingRoute.useLoaderData()} +
+ ), + pendingComponent: () => ( +
Pending...
+ ), + errorComponent: ({ error }) => { + errorComponentRenderCount(error) + return
{String(error)}
+ }, + }) + + const router = createRouter({ + routeTree: rootRoute.addChildren([forcePendingRoute]), + history, + }) + + render() + + 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. diff --git a/packages/router-core/src/Matches.ts b/packages/router-core/src/Matches.ts index 852d186b67d..0ba56004d23 100644 --- a/packages/router-core/src/Matches.ts +++ b/packages/router-core/src/Matches.ts @@ -145,6 +145,7 @@ export interface RouteMatch< /** @internal */ pendingTimeout?: ReturnType loadPromise?: ControlledPromise + pendingRenderPromise?: ControlledPromise displayPendingPromise?: Promise minPendingPromise?: ControlledPromise dehydrated?: boolean diff --git a/packages/router-core/src/load-matches.ts b/packages/router-core/src/load-matches.ts index d212c72b926..fc4e0585a90 100644 --- a/packages/router-core/src/load-matches.ts +++ b/packages/router-core/src/load-matches.ts @@ -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) @@ -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) diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index 965fefda339..663a2b82d7a 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -112,6 +112,13 @@ export type ControllablePromise = Promise & { export type InjectedHtmlEntry = Promise +const clearPendingRenderPromise = (match: AnyRouteMatch | undefined) => { + if (match) { + match._nonReactive.pendingRenderPromise?.resolve() + match._nonReactive.pendingRenderPromise = undefined + } +} + export interface Register { // Lots of things on here like... // router @@ -2475,6 +2482,19 @@ export class RouterCore< * or reloads re-run their loaders instead of reusing the failed/not-found data. */ if (mountPending) { + const pendingMatchesById = new Map( + pendingMatches.map((match) => [match.id, match]), + ) + + currentMatches.forEach((currentMatch) => { + if ( + pendingMatchesById.get(currentMatch.id)?.status !== + 'pending' + ) { + clearPendingRenderPromise(currentMatch) + } + }) + this.stores.setActiveMatches(pendingMatches) this.stores.setPendingMatches([]) this.stores.setCachedMatches([ @@ -2634,7 +2654,11 @@ export class RouterCore< const activeMatch = this.stores.activeMatchStoresById.get(id) if (activeMatch) { - activeMatch.setState(updater) + const next = updater(activeMatch.state) + if (next.status !== 'pending') { + clearPendingRenderPromise(activeMatch.state) + } + activeMatch.setState(() => next) return } @@ -2696,9 +2720,16 @@ export class RouterCore< } this.batch(() => { - this.stores.setActiveMatches( - this.stores.activeMatchesSnapshot.state.map(invalidate), - ) + const activeMatches = this.stores.activeMatchesSnapshot.state + const nextActiveMatches = activeMatches.map(invalidate) + + activeMatches.forEach((activeMatch, index) => { + if (nextActiveMatches[index]?.status !== 'pending') { + clearPendingRenderPromise(activeMatch) + } + }) + + this.stores.setActiveMatches(nextActiveMatches) this.stores.setCachedMatches( this.stores.cachedMatchesSnapshot.state.map(invalidate), )