From 9c2ec9c1d4d3552f914ce0651730299f9822cfa6 Mon Sep 17 00:00:00 2001 From: Maciej Garncarski Date: Mon, 2 Sep 2024 11:41:19 +0200 Subject: [PATCH 1/3] fix(router): pendingComponent not showing --- packages/react-router/src/router.ts | 45 +++++++++++++++++++---------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/packages/react-router/src/router.ts b/packages/react-router/src/router.ts index cb07523e983..c2104924180 100644 --- a/packages/react-router/src/router.ts +++ b/packages/react-router/src/router.ts @@ -1975,12 +1975,42 @@ export class Router< const existingMatch = this.getMatch(matchId)! const parentMatchId = matches[index - 1]?.id + const route = this.looseRoutesById[routeId]! + + const pendingMs = + route.options.pendingMs ?? this.options.defaultPendingMs + + const shouldPending = !!( + onReady && + !this.isServer && + !preload && + (route.options.loader || route.options.beforeLoad) && + typeof pendingMs === 'number' && + pendingMs !== Infinity && + (route.options.pendingComponent ?? + this.options.defaultPendingComponent) + ) + if ( // If we are in the middle of a load, either of these will be present // (not to be confused with `loadPromise`, which is always defined) existingMatch.beforeLoadPromise || existingMatch.loaderPromise ) { + let pendingTimeout: ReturnType + + if (shouldPending) { + // If we might show a pending component, we need to wait for the + // pending promise to resolve before we start showing that state + pendingTimeout = setTimeout(() => { + try { + // Update the match and prematurely resolve the loadMatches promise so that + // the pending component can start rendering + triggerOnReady() + } catch {} + }, pendingMs) + } + // Wait for the beforeLoad to resolve before we continue await existingMatch.beforeLoadPromise } else { @@ -1994,23 +2024,8 @@ export class Router< beforeLoadPromise: createControlledPromise(), })) - const route = this.looseRoutesById[routeId]! const abortController = new AbortController() - const pendingMs = - route.options.pendingMs ?? this.options.defaultPendingMs - - const shouldPending = !!( - onReady && - !this.isServer && - !preload && - (route.options.loader || route.options.beforeLoad) && - typeof pendingMs === 'number' && - pendingMs !== Infinity && - (route.options.pendingComponent ?? - this.options.defaultPendingComponent) - ) - let pendingTimeout: ReturnType if (shouldPending) { From fdaf02262677929cfe6296d6d6b7c4668f25fc07 Mon Sep 17 00:00:00 2001 From: Maciej Garncarski Date: Mon, 2 Sep 2024 12:05:50 +0200 Subject: [PATCH 2/3] remove unused variable --- packages/react-router/src/router.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/react-router/src/router.ts b/packages/react-router/src/router.ts index c2104924180..627d82bc058 100644 --- a/packages/react-router/src/router.ts +++ b/packages/react-router/src/router.ts @@ -1997,12 +1997,8 @@ export class Router< existingMatch.beforeLoadPromise || existingMatch.loaderPromise ) { - let pendingTimeout: ReturnType - if (shouldPending) { - // If we might show a pending component, we need to wait for the - // pending promise to resolve before we start showing that state - pendingTimeout = setTimeout(() => { + setTimeout(() => { try { // Update the match and prematurely resolve the loadMatches promise so that // the pending component can start rendering From b46d5cc09689c219fb4044693102fdb71f5ad60d Mon Sep 17 00:00:00 2001 From: Maciej Garncarski Date: Mon, 2 Sep 2024 22:17:14 +0200 Subject: [PATCH 3/3] tests: add test --- packages/react-router/tests/link.test.tsx | 55 ++++++++++++++++++++++- 1 file changed, 54 insertions(+), 1 deletion(-) diff --git a/packages/react-router/tests/link.test.tsx b/packages/react-router/tests/link.test.tsx index 9bf54202b02..e2a5e0e1831 100644 --- a/packages/react-router/tests/link.test.tsx +++ b/packages/react-router/tests/link.test.tsx @@ -28,7 +28,7 @@ import { useRouterState, useSearch, } from '../src' -import { getIntersectionObserverMock } from './utils' +import { getIntersectionObserverMock, sleep } from './utils' const ioObserveMock = vi.fn() const ioDisconnectMock = vi.fn() @@ -47,6 +47,8 @@ afterEach(() => { cleanup() }) +const WAIT_TIME = 300 + describe('Link', () => { test('when using renderHook it returns a hook with same content to prove rerender works', async () => { /** @@ -3528,6 +3530,57 @@ describe('Link', () => { expect(ioObserveMock).not.toBeCalled() }) + + test('Router.preload="intent", pendingComponent renders during unresolved route loader', async () => { + const rootRoute = createRootRoute() + const indexRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/', + component: () => { + return ( +
+

Index page

+ + link to posts + +
+ ) + }, + }) + + const postRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/posts', + loader: () => sleep(WAIT_TIME), + component: () =>
Posts page
, + }) + + const routeTree = rootRoute.addChildren([postRoute, indexRoute]) + const router = createRouter({ + routeTree, + defaultPreload: 'intent', + defaultPendingMs: 200, + defaultPendingComponent: () =>

Loading...

, + }) + + render() + + const linkToPosts = await screen.findByRole('link', { + name: 'link to posts', + }) + expect(linkToPosts).toBeInTheDocument() + + fireEvent.focus(linkToPosts) + fireEvent.click(linkToPosts) + + const loadingElement = await screen.findByText('Loading...') + expect(loadingElement).toBeInTheDocument() + + const postsElement = await screen.findByText('Posts page') + expect(postsElement).toBeInTheDocument() + + expect(window.location.pathname).toBe('/posts') + }) }) describe('createLink', () => {