diff --git a/.changeset/brown-kings-listen.md b/.changeset/brown-kings-listen.md new file mode 100644 index 00000000000..317eed597cf --- /dev/null +++ b/.changeset/brown-kings-listen.md @@ -0,0 +1,9 @@ +--- +'@tanstack/router-core': patch +--- + +Advance the pending boundary past matches that have already settled. + +The router offers a match outside the retained prefix as the pending boundary before it settles, even when the route has no `loader`. It also treated any presented match with status `pending` as the painted boundary, and every match below the boundary carries that status in the offered snapshot. A settled ancestor therefore kept selecting itself as the boundary and then stopped early on its own session, so the presented snapshot stayed `pending` until the slowest loader in the branch resolved. + +The painted boundary is now the first presented `pending` entry, and a settled match hands over toward pending descendants. A layout route with no loader, such as one that is only lazy, now renders as soon as it settles, and the fallback stays on the match that is still loading. A terminal settled match that is still painted is still offered, so data-only routes keep their pending phase while hydrating on the client. A painted boundary holds through its `pendingMinMs` window before handing over, so an outgoing fallback is never swapped faster than its minimum. diff --git a/packages/react-router/tests/pending-boundary-settled-ancestor.test.tsx b/packages/react-router/tests/pending-boundary-settled-ancestor.test.tsx new file mode 100644 index 00000000000..8b2df14fead --- /dev/null +++ b/packages/react-router/tests/pending-boundary-settled-ancestor.test.tsx @@ -0,0 +1,287 @@ +import { act, cleanup, render, screen, waitFor } from '@testing-library/react' +import { afterEach, expect, test, vi } from 'vitest' +import { + Outlet, + RouterProvider, + createControlledPromise, + createLazyRoute, + createMemoryHistory, + createRootRoute, + createRoute, + createRouter, +} from '../src' + +afterEach(() => { + cleanup() + vi.useRealTimers() + vi.restoreAllMocks() +}) + +// A lazy-only layout settles before its leaf loader, so its shell renders +// while the leaf fallback stays visible. +test('a settled layout renders its shell while its leaf is still loading', async () => { + const headerOptions = createLazyRoute('/users/$userId')({ + component: () => ( +
+

Header shell

+ +
+ ), + }) + const headerChunk = createControlledPromise() + const detailLoader = createControlledPromise() + + const rootRoute = createRootRoute({ component: Outlet }) + const authRoute = createRoute({ + getParentRoute: () => rootRoute, + id: '_auth', + component: Outlet, + }) + const sidebarRoute = createRoute({ + getParentRoute: () => authRoute, + id: '_sidebar', + component: Outlet, + }) + const headerRoute = createRoute({ + getParentRoute: () => authRoute, + id: '_header', + }).lazy(() => headerChunk) + + const listRoute = createRoute({ + getParentRoute: () => sidebarRoute, + path: '/users', + component: () =>

User list

, + }) + const detailRoute = createRoute({ + getParentRoute: () => headerRoute, + path: '/users/$userId', + loader: () => detailLoader, + component: () =>

User detail

, + }) + + const router = createRouter({ + routeTree: rootRoute.addChildren([ + authRoute.addChildren([ + sidebarRoute.addChildren([listRoute]), + headerRoute.addChildren([detailRoute]), + ]), + ]), + history: createMemoryHistory({ initialEntries: ['/users'] }), + defaultPendingComponent: () =>

Loading

, + defaultPendingMs: 0, + defaultPendingMinMs: 0, + }) + + render() + await waitFor(() => expect(screen.getByText('User list')).toBeInTheDocument()) + + const navigation = router.navigate({ + to: '/users/$userId', + params: { userId: 'u1' }, + }) + + await waitFor(() => expect(screen.getByRole('status')).toBeInTheDocument()) + expect(screen.queryByText('Header shell')).not.toBeInTheDocument() + + headerChunk.resolve(headerOptions) + + await waitFor(() => + expect(screen.getByText('Header shell')).toBeInTheDocument(), + ) + expect(screen.getByRole('status')).toBeInTheDocument() + expect(screen.queryByText('User detail')).not.toBeInTheDocument() + + detailLoader.resolve('detail data') + await navigation + + await waitFor(() => + expect(screen.getByText('User detail')).toBeInTheDocument(), + ) + expect(screen.getByText('Header shell')).toBeInTheDocument() +}) + +// A painted fallback holds through its minimum window before the boundary +// moves to the leaf that is still loading. +test('a painted fallback holds its minimum before the shell renders', async () => { + vi.useFakeTimers() + const headerOptions = createLazyRoute('/users/$userId')({ + component: () => ( +
+

Header shell

+ +
+ ), + }) + const headerChunk = createControlledPromise() + const detailLoader = createControlledPromise() + + const rootRoute = createRootRoute({ component: Outlet }) + const authRoute = createRoute({ + getParentRoute: () => rootRoute, + id: '_auth', + component: Outlet, + }) + const sidebarRoute = createRoute({ + getParentRoute: () => authRoute, + id: '_sidebar', + component: Outlet, + }) + const headerRoute = createRoute({ + getParentRoute: () => authRoute, + id: '_header', + }).lazy(() => headerChunk) + + const listRoute = createRoute({ + getParentRoute: () => sidebarRoute, + path: '/users', + component: () =>

User list

, + }) + const detailRoute = createRoute({ + getParentRoute: () => headerRoute, + path: '/users/$userId', + loader: () => detailLoader, + component: () =>

User detail

, + }) + + const router = createRouter({ + routeTree: rootRoute.addChildren([ + authRoute.addChildren([ + sidebarRoute.addChildren([listRoute]), + headerRoute.addChildren([detailRoute]), + ]), + ]), + history: createMemoryHistory({ initialEntries: ['/users'] }), + defaultPendingComponent: () =>

Loading

, + defaultPendingMs: 0, + defaultPendingMinMs: 400, + }) + + await router.load() + render() + expect(screen.getByText('User list')).toBeInTheDocument() + + let navigation!: Promise + await act(async () => { + navigation = router.navigate({ + to: '/users/$userId', + params: { userId: 'u1' }, + }) + await vi.advanceTimersByTimeAsync(0) + }) + expect(screen.getByRole('status')).toBeInTheDocument() + expect(screen.queryByText('Header shell')).not.toBeInTheDocument() + + await act(async () => { + headerChunk.resolve(headerOptions) + await vi.advanceTimersByTimeAsync(0) + }) + + // The layout settled, but the painted fallback holds its minimum window. + await act(async () => { + await vi.advanceTimersByTimeAsync(100) + }) + expect(screen.getByRole('status')).toBeInTheDocument() + expect(screen.queryByText('Header shell')).not.toBeInTheDocument() + + // Once the minimum elapses the shell renders with the leaf still pending. + await act(async () => { + await vi.advanceTimersByTimeAsync(300) + }) + expect(screen.getByText('Header shell')).toBeInTheDocument() + expect(screen.getByRole('status')).toBeInTheDocument() + expect(screen.queryByText('User detail')).not.toBeInTheDocument() + + await act(async () => { + detailLoader.resolve('detail data') + await vi.advanceTimersByTimeAsync(400) + await navigation + }) + expect(screen.getByText('User detail')).toBeInTheDocument() + expect(screen.getByText('Header shell')).toBeInTheDocument() +}) + +// Each boundary shows its own fallback as the boundary moves down the branch. +test('the pending fallback moves from the layout to the leaf', async () => { + const headerOptions = createLazyRoute('/users/$userId')({ + component: () => ( +
+

Header shell

+ +
+ ), + }) + const headerChunk = createControlledPromise() + const detailLoader = createControlledPromise() + + const rootRoute = createRootRoute({ component: Outlet }) + const authRoute = createRoute({ + getParentRoute: () => rootRoute, + id: '_auth', + component: Outlet, + }) + const sidebarRoute = createRoute({ + getParentRoute: () => authRoute, + id: '_sidebar', + component: Outlet, + }) + const headerRoute = createRoute({ + getParentRoute: () => authRoute, + id: '_header', + pendingComponent: () =>

Header loading

, + }).lazy(() => headerChunk) + + const listRoute = createRoute({ + getParentRoute: () => sidebarRoute, + path: '/users', + component: () =>

User list

, + }) + const detailRoute = createRoute({ + getParentRoute: () => headerRoute, + path: '/users/$userId', + loader: () => detailLoader, + pendingComponent: () =>

Leaf loading

, + component: () =>

User detail

, + }) + + const router = createRouter({ + routeTree: rootRoute.addChildren([ + authRoute.addChildren([ + sidebarRoute.addChildren([listRoute]), + headerRoute.addChildren([detailRoute]), + ]), + ]), + history: createMemoryHistory({ initialEntries: ['/users'] }), + defaultPendingMs: 0, + defaultPendingMinMs: 0, + }) + + render() + await waitFor(() => expect(screen.getByText('User list')).toBeInTheDocument()) + + const navigation = router.navigate({ + to: '/users/$userId', + params: { userId: 'u1' }, + }) + + await waitFor(() => + expect(screen.getByText('Header loading')).toBeInTheDocument(), + ) + expect(screen.queryByText('Header shell')).not.toBeInTheDocument() + + headerChunk.resolve(headerOptions) + + await waitFor(() => + expect(screen.getByText('Header shell')).toBeInTheDocument(), + ) + expect(screen.getByText('Leaf loading')).toBeInTheDocument() + expect(screen.queryByText('Header loading')).not.toBeInTheDocument() + expect(screen.queryByText('User detail')).not.toBeInTheDocument() + + detailLoader.resolve('detail data') + await navigation + + await waitFor(() => + expect(screen.getByText('User detail')).toBeInTheDocument(), + ) + expect(screen.getByText('Header shell')).toBeInTheDocument() +}) diff --git a/packages/router-core/src/load-client.ts b/packages/router-core/src/load-client.ts index ea4f2704471..c64a9359c7e 100644 --- a/packages/router-core/src/load-client.ts +++ b/packages/router-core/src/load-client.ts @@ -1453,21 +1453,58 @@ function offerPending(router: CoordinatorRouter, tx: LoadTransaction): void { } const matches = tx[3 /* matches */] const presented = router.stores.matches.get() + // Only the first presented `pending` entry is the painted boundary. The + // fallback replaces every match after it, so those carry `pending` in the + // offered snapshot without ever rendering. + const paintedBoundary = presented.findIndex( + (candidate) => candidate.status === 'pending', + ) let session = router._pending for (let index = 0; index < matches.length; index++) { const match = matches[index]! - const success = match.status === 'success' && !match._notFound const presentedPending = - presented[index]?.id === match.id && - presented[index]?.status === 'pending' - if (success && !presentedPending) { - continue + index === paintedBoundary && presented[index]?.id === match.id + // A settled match never keeps the boundary, even while it is painted. The + // boundary advances so the next presented snapshot carries its real status. + // A painted boundary holds through its minimum window. The render ack + // arms the session deadline, so a live deadline means the outgoing + // fallback was seen; anything else advances at once. + if (match.status === 'success' && !match._notFound) { + // A painted boundary holds through its minimum window. The render ack + // arms the session deadline, so a live deadline means the outgoing + // fallback was seen; anything else advances at once. + if (presentedPending && session?.[1 /* boundaryId */] === match.id) { + const remaining = session[2 /* deadline */] - Date.now() + if (remaining > 0) { + session[0 /* generation */] = tx + clearTimeout(session[3 /* revealTimer */]) + session[3 /* revealTimer */] = setTimeout( + () => offerPending(router, tx), + remaining, + ) + return + } + } + // Advance past a settled match only toward pending descendants. A + // terminal settled match that is still painted (e.g. a data-only route + // whose SSR data arrived as success while the client component still + // needs its pending phase) falls through and is offered instead. + let hasPendingDescendant = false + for (let next = index + 1; next < matches.length; next++) { + const descendant = matches[next]! + if (descendant.status !== 'success' || descendant._notFound) { + hasPendingDescendant = true + break + } + } + if (hasPendingDescendant || !presentedPending) { + continue + } } const route = getRoute(router, match as WorkMatch) - const delay = - success || match.invalid - ? 0 - : (route.options.pendingMs ?? router.options.defaultPendingMs) + const delay = match.invalid + ? 0 + : (route.options.pendingMs ?? router.options.defaultPendingMs) const component = route.options.pendingComponent ?? (router.options as any).defaultPendingComponent diff --git a/packages/router-core/tests/pending-boundary-settled-ancestor.test.ts b/packages/router-core/tests/pending-boundary-settled-ancestor.test.ts new file mode 100644 index 00000000000..007c1fcc6d4 --- /dev/null +++ b/packages/router-core/tests/pending-boundary-settled-ancestor.test.ts @@ -0,0 +1,346 @@ +import { afterEach, describe, expect, test, vi } from 'vitest' +import { createMemoryHistory } from '@tanstack/history' +import { BaseRootRoute, BaseRoute, createControlledPromise } from '../src' +import { createTestRouter } from './routerTestUtils' + +/** + * The pending boundary must advance past matches that have already settled. + * A layout route with no loader is offered as the boundary while its lazy + * chunk loads, but once it settles the boundary belongs on the leaf that is + * still loading so the layout shell renders instead of staying behind the + * fallback. + */ + +afterEach(() => { + vi.useRealTimers() + vi.restoreAllMocks() +}) + +describe('pending boundary must advance past settled ancestors', () => { + test('a layout route with no loader does not stay the pending boundary once it settles', async () => { + const leafLoader = createControlledPromise() + + const rootRoute = new BaseRootRoute({ component: () => null }) + const authRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + id: '_auth', + loader: () => ({}), + component: () => null, + }) + const sidebarRoute = new BaseRoute({ + getParentRoute: () => authRoute, + id: '_sidebar', + component: () => null, + }) + // A layout route with no loader that still has a lazy chunk to fetch. + const headerRoute = new BaseRoute({ + getParentRoute: () => authRoute, + id: '_header', + pendingComponent: () => null, + }) + const headerChunk = + createControlledPromise< + Awaited[0]>> + >() + headerRoute.lazy(() => headerChunk) + + const listRoute = new BaseRoute({ + getParentRoute: () => sidebarRoute, + path: '/users', + component: () => null, + }) + const detailRoute = new BaseRoute({ + getParentRoute: () => headerRoute, + path: '/users/$userId', + loader: () => leafLoader, + pendingComponent: () => null, + component: () => null, + }) + + const router = createTestRouter({ + routeTree: rootRoute.addChildren([ + authRoute.addChildren([ + sidebarRoute.addChildren([listRoute]), + headerRoute.addChildren([detailRoute]), + ]), + ]), + history: createMemoryHistory({ initialEntries: ['/users'] }), + defaultPendingMs: 0, + defaultPendingMinMs: 0, + }) + + await router.load() + + const navigation = router.navigate({ + to: '/users/$userId', + params: { userId: 'u1' }, + }) + + await vi.waitFor(() => + expect( + router.state.matches.find((match) => match.routeId === headerRoute.id) + ?.status, + ).toBe('pending'), + ) + + headerChunk.resolve({ + options: { id: headerRoute.id, component: () => null }, + }) + + await vi.waitFor(() => + expect( + router.state.matches.map((match) => [match.routeId, match.status]), + ).toEqual([ + [rootRoute.id, 'success'], + [authRoute.id, 'success'], + [headerRoute.id, 'success'], + [detailRoute.id, 'pending'], + ]), + ) + + leafLoader.resolve('leaf data') + await navigation + }) + + test('an initial load presents the layout as settled once its loader resolves', async () => { + const layoutLoader = createControlledPromise() + const childLoader = createControlledPromise() + + const rootRoute = new BaseRootRoute({ component: () => null }) + const layoutRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + id: '_layout', + loader: () => layoutLoader, + pendingComponent: () => null, + component: () => null, + }) + const childRoute = new BaseRoute({ + getParentRoute: () => layoutRoute, + path: '/child', + loader: () => childLoader, + pendingComponent: () => null, + component: () => null, + }) + + const router = createTestRouter({ + routeTree: rootRoute.addChildren([layoutRoute.addChildren([childRoute])]), + history: createMemoryHistory({ initialEntries: ['/child'] }), + defaultPendingMs: 0, + defaultPendingMinMs: 0, + }) + + const load = router.load() + + await vi.waitFor(() => + expect( + router.state.matches.find((match) => match.routeId === layoutRoute.id) + ?.status, + ).toBe('pending'), + ) + + layoutLoader.resolve('layout data') + + await vi.waitFor(() => + expect( + router.state.matches.map((match) => [match.routeId, match.status]), + ).toEqual([ + [rootRoute.id, 'success'], + [layoutRoute.id, 'success'], + [childRoute.id, 'pending'], + ]), + ) + + childLoader.resolve('child data') + await load + }) + + test('a boundary that never paints does not hold the presented snapshot', async () => { + const leafLoader = createControlledPromise() + + const rootRoute = new BaseRootRoute({ component: () => null }) + const indexRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/', + component: () => null, + }) + const layoutRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + id: '_layout', + pendingComponent: () => null, + }) + const layoutChunk = + createControlledPromise< + Awaited[0]>> + >() + layoutRoute.lazy(() => layoutChunk) + + const leafRoute = new BaseRoute({ + getParentRoute: () => layoutRoute, + path: '/leaf', + loader: () => leafLoader, + pendingComponent: () => null, + component: () => null, + }) + + const router = createTestRouter({ + routeTree: rootRoute.addChildren([ + indexRoute, + layoutRoute.addChildren([leafRoute]), + ]), + history: createMemoryHistory({ initialEntries: ['/'] }), + defaultPendingMs: 0, + defaultPendingMinMs: 400, + }) + // The bare test router never renders, so no render ack arms a minimum + // window. The painted hold is covered by the React test instead. + + await router.load() + const navigation = router.navigate({ to: '/leaf' }) + + await vi.waitFor(() => + expect( + router.state.matches.find((match) => match.routeId === layoutRoute.id) + ?.status, + ).toBe('pending'), + ) + + layoutChunk.resolve({ + options: { id: layoutRoute.id, component: () => null }, + }) + + // Nothing painted the boundary, so no minimum window holds it back and + // the boundary advances to the leaf as soon as the layout settles. + await vi.waitFor(() => + expect( + router.state.matches.map((match) => [match.routeId, match.status]), + ).toEqual([ + [rootRoute.id, 'success'], + [layoutRoute.id, 'success'], + [leafRoute.id, 'pending'], + ]), + ) + + leafLoader.resolve('leaf data') + await navigation + }) + + test('the boundary steps down as each ancestor in the chain settles', async () => { + const layoutLoader = createControlledPromise() + const midLoader = createControlledPromise() + const leafLoader = createControlledPromise() + + const rootRoute = new BaseRootRoute({ component: () => null }) + const layoutRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + id: '_layout', + loader: () => layoutLoader, + pendingComponent: () => null, + component: () => null, + }) + const midRoute = new BaseRoute({ + getParentRoute: () => layoutRoute, + path: '/mid', + loader: () => midLoader, + pendingComponent: () => null, + component: () => null, + }) + const leafRoute = new BaseRoute({ + getParentRoute: () => midRoute, + path: '/leaf', + loader: () => leafLoader, + pendingComponent: () => null, + component: () => null, + }) + + const router = createTestRouter({ + routeTree: rootRoute.addChildren([ + layoutRoute.addChildren([midRoute.addChildren([leafRoute])]), + ]), + history: createMemoryHistory({ initialEntries: ['/mid/leaf'] }), + defaultPendingMs: 0, + defaultPendingMinMs: 0, + }) + + const find = (id: string) => + router.state.matches.find((match) => match.routeId === id)?.status + + const load = router.load() + + await vi.waitFor(() => expect(find(layoutRoute.id)).toBe('pending')) + + layoutLoader.resolve('layout data') + // The layout turning successful is what proves the boundary stepped down. + // A stuck boundary would keep forcing it pending in the presented snapshot. + await vi.waitFor(() => expect(find(layoutRoute.id)).toBe('success')) + await vi.waitFor(() => expect(find(midRoute.id)).toBe('pending')) + + midLoader.resolve('mid data') + await vi.waitFor(() => expect(find(midRoute.id)).toBe('success')) + await vi.waitFor(() => expect(find(leafRoute.id)).toBe('pending')) + + leafLoader.resolve('leaf data') + await load + + expect(find(leafRoute.id)).toBe('success') + }) + + test('a branch with nothing left to load commits without offering a boundary', async () => { + const rootRoute = new BaseRootRoute({ component: () => null }) + const indexRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/', + component: () => null, + }) + const layoutRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + id: '_layout', + pendingComponent: () => null, + }) + const layoutChunk = + createControlledPromise< + Awaited[0]>> + >() + layoutRoute.lazy(() => layoutChunk) + + const leafRoute = new BaseRoute({ + getParentRoute: () => layoutRoute, + path: '/leaf', + component: () => null, + }) + + const router = createTestRouter({ + routeTree: rootRoute.addChildren([ + indexRoute, + layoutRoute.addChildren([leafRoute]), + ]), + history: createMemoryHistory({ initialEntries: ['/'] }), + defaultPendingMs: 0, + defaultPendingMinMs: 0, + }) + + await router.load() + const navigation = router.navigate({ to: '/leaf' }) + + await vi.waitFor(() => + expect( + router.state.matches.find((match) => match.routeId === layoutRoute.id) + ?.status, + ).toBe('pending'), + ) + + layoutChunk.resolve({ + options: { id: layoutRoute.id, component: () => null }, + }) + + // The leaf has no loader, so once the layout settles no match is left to + // offer and the branch commits without another boundary. + await navigation + expect( + router.state.matches.map((match) => [match.routeId, match.status]), + ).toEqual([ + [rootRoute.id, 'success'], + [layoutRoute.id, 'success'], + [leafRoute.id, 'success'], + ]) + }) +})