diff --git a/packages/react-router/tests/loaders.test.tsx b/packages/react-router/tests/loaders.test.tsx index 174ae4eeb2..8d92506468 100644 --- a/packages/react-router/tests/loaders.test.tsx +++ b/packages/react-router/tests/loaders.test.tsx @@ -1,4 +1,5 @@ import { + act, cleanup, configure, fireEvent, @@ -17,6 +18,7 @@ import { createRootRoute, createRoute, createRouter, + useRouter, } from '../src' import { sleep } from './utils' @@ -380,7 +382,7 @@ test('reproducer #4245', async () => { render() // We wait for the initial loader to complete - await router.load() + await act(() => router.load()) const fooLink = await screen.findByTestId('link-to-foo') expect(fooLink).toBeInTheDocument() @@ -389,27 +391,257 @@ test('reproducer #4245', async () => { fireEvent.click(fooLink) // We immediately see the content of the foo route - const indexLink = await screen.findByTestId('link-to-index') + const indexLink = await screen.findByTestId('link-to-index', undefined, { + timeout: WAIT_TIME, + }) expect(indexLink).toBeInTheDocument() // We navigate to the index route fireEvent.click(indexLink) // We immediately see the content of the index route because the stale data is still available - const fooLink2 = await screen.findByTestId('link-to-foo') + const fooLink2 = await screen.findByTestId('link-to-foo', undefined, { + timeout: WAIT_TIME, + }) expect(fooLink2).toBeInTheDocument() // We navigate to the foo route again fireEvent.click(fooLink2) // We immediately see the content of the foo route - const indexLink2 = await screen.findByTestId('link-to-index') + const indexLink2 = await screen.findByTestId('link-to-index', undefined, { + timeout: WAIT_TIME, + }) expect(indexLink2).toBeInTheDocument() // We navigate to the index route again fireEvent.click(indexLink2) // We now should see the content of the index route immediately because the stale data is still available - const fooLink3 = await screen.findByTestId('link-to-foo') + const fooLink3 = await screen.findByTestId('link-to-foo', undefined, { + timeout: WAIT_TIME, + }) expect(fooLink3).toBeInTheDocument() }) + +test('reproducer #4546', async () => { + const rootRoute = createRootRoute({ + component: () => { + return ( + <> +
+ + Home + {' '} + + /1 + +
+
+ + + ) + }, + }) + + let counter = 0 + const appRoute = createRoute({ + getParentRoute: () => rootRoute, + id: '_app', + beforeLoad: () => { + counter += 1 + return { + counter, + } + }, + component: () => { + return ( +
+
+ +
+ ) + }, + }) + + function Header() { + const router = useRouter() + const { counter } = appRoute.useRouteContext() + + return ( +
+ Header Counter:

{counter}

+ +
+ ) + } + + const indexRoute = createRoute({ + getParentRoute: () => appRoute, + path: '/', + loader: ({ context }) => { + return { + counter: context.counter, + } + }, + + component: () => { + const data = indexRoute.useLoaderData() + const ctx = indexRoute.useRouteContext() + + return ( +
+
Index route
+
+ route context:{' '} +

{ctx.counter}

+
+
+ loader data:

{data.counter}

+
+
+ ) + }, + }) + const idRoute = createRoute({ + getParentRoute: () => appRoute, + path: '$id', + loader: ({ context }) => { + return { + counter: context.counter, + } + }, + + component: () => { + const data = idRoute.useLoaderData() + const ctx = idRoute.useRouteContext() + + return ( +
+
$id route
+
+ route context:

{ctx.counter}

+
+
+ loader data:

{data.counter}

+
+
+ ) + }, + }) + + const routeTree = rootRoute.addChildren([ + appRoute.addChildren([indexRoute, idRoute]), + ]) + const router = createRouter({ routeTree }) + + render() + + const indexLink = await screen.findByTestId('link-to-index') + expect(indexLink).toBeInTheDocument() + + const idLink = await screen.findByTestId('link-to-id') + expect(idLink).toBeInTheDocument() + + const invalidateRouterButton = await screen.findByTestId('invalidate-router') + expect(invalidateRouterButton).toBeInTheDocument() + + { + const headerCounter = await screen.findByTestId('header-counter') + expect(headerCounter).toHaveTextContent('1') + + const routeContext = await screen.findByTestId('index-route-context') + expect(routeContext).toHaveTextContent('1') + + const loaderData = await screen.findByTestId('index-loader-data') + expect(loaderData).toHaveTextContent('1') + } + + fireEvent.click(idLink) + + { + const headerCounter = await screen.findByTestId('header-counter') + expect(headerCounter).toHaveTextContent('2') + + const routeContext = await screen.findByTestId('id-route-context') + expect(routeContext).toHaveTextContent('2') + + const loaderData = await screen.findByTestId('id-loader-data') + expect(loaderData).toHaveTextContent('2') + } + + fireEvent.click(indexLink) + + { + const headerCounter = await screen.findByTestId('header-counter') + expect(headerCounter).toHaveTextContent('3') + + const routeContext = await screen.findByTestId('index-route-context') + expect(routeContext).toHaveTextContent('3') + + const loaderData = await screen.findByTestId('index-loader-data') + expect(loaderData).toHaveTextContent('3') + } + + fireEvent.click(invalidateRouterButton) + + { + const headerCounter = await screen.findByTestId('header-counter') + expect(headerCounter).toHaveTextContent('4') + + const routeContext = await screen.findByTestId('index-route-context') + expect(routeContext).toHaveTextContent('4') + + const loaderData = await screen.findByTestId('index-loader-data') + expect(loaderData).toHaveTextContent('4') + } + + fireEvent.click(idLink) + + { + const headerCounter = await screen.findByTestId('header-counter') + expect(headerCounter).toHaveTextContent('5') + + const routeContext = await screen.findByTestId('id-route-context') + expect(routeContext).toHaveTextContent('5') + + const loaderData = await screen.findByTestId('id-loader-data') + expect(loaderData).toHaveTextContent('5') + } +}) diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index e7fd84f792..71d75548d3 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -1791,16 +1791,9 @@ export class RouterCore< location: this.latestLocation, pendingMatches, // If a cached moved to pendingMatches, remove it from cachedMatches - cachedMatches: s.cachedMatches.filter((cachedMatch) => { - const pendingMatch = pendingMatches.find((e) => e.id === cachedMatch.id) - - if (!pendingMatch) return true - - return ( - cachedMatch.status === 'success' && - (cachedMatch.isFetching || cachedMatch.loaderData !== undefined) - ) - }), + cachedMatches: s.cachedMatches.filter( + (d) => !pendingMatches.find((e) => e.id === d.id), + ), })) } @@ -2208,7 +2201,15 @@ export class RouterCore< // Wait for the beforeLoad to resolve before we continue await existingMatch.beforeLoadPromise - executeBeforeLoad = this.getMatch(matchId)!.status === 'error' + const match = this.getMatch(matchId)! + if (match.status === 'error') { + executeBeforeLoad = true + } else if ( + match.preload && + (match.status === 'redirected' || match.status === 'notFound') + ) { + handleRedirectAndNotFound(match, match.error) + } } if (executeBeforeLoad) { // If we are not in the middle of a load OR the previous load failed, start it @@ -2337,14 +2338,23 @@ export class RouterCore< validResolvedMatches.forEach(({ id: matchId, routeId }, index) => { matchPromises.push( (async () => { - const { loaderPromise: prevLoaderPromise } = - this.getMatch(matchId)! - let loaderShouldRunAsync = false let loaderIsRunningAsync = false - if (prevLoaderPromise) { - await prevLoaderPromise + const prevMatch = this.getMatch(matchId)! + // there is a loaderPromise, so we are in the middle of a load + if (prevMatch.loaderPromise) { + // do not block if we already have stale data we can show + // but only if the ongoing load is not a preload since error handling is different for preloads + // and we don't want to swallow errors + if ( + prevMatch.status === 'success' && + !sync && + !prevMatch.preload + ) { + return this.getMatch(matchId)! + } + await prevMatch.loaderPromise const match = this.getMatch(matchId)! if (match.error) { handleRedirectAndNotFound(match, match.error)