diff --git a/.changeset/violet-poets-wait.md b/.changeset/violet-poets-wait.md new file mode 100644 index 0000000000..1dbc5af367 --- /dev/null +++ b/.changeset/violet-poets-wait.md @@ -0,0 +1,7 @@ +--- +'@tanstack/router-core': patch +--- + +Fix context values from a parent route's `beforeLoad` not being propagated to sub-routes in several code paths: while a sub-route's loader reloads in the background, when re-entering a route whose background reload is still in flight, and in a sub-route's error state when its `beforeLoad` throws (the merged context is now committed together with the error status for the errorComponent to consume). + +Redirects no longer use a renderable `RouteMatch.status`; `RouteMatch.status` is now `'pending' | 'success' | 'error' | 'notFound'`. Abandoned pending, redirected, or failed matches are dropped from cache and their pending promises are settled so stale suspense work cannot keep rendering suspended. diff --git a/docs/router/api/router/RouteMatchType.md b/docs/router/api/router/RouteMatchType.md index 251c2c031e..67fe4f17f6 100644 --- a/docs/router/api/router/RouteMatchType.md +++ b/docs/router/api/router/RouteMatchType.md @@ -11,7 +11,7 @@ interface RouteMatch { routeId: string pathname: string params: Route['allParams'] - status: 'pending' | 'success' | 'error' | 'redirected' | 'notFound' + status: 'pending' | 'success' | 'error' | 'notFound' isFetching: false | 'beforeLoad' | 'loader' showPending: boolean error: unknown diff --git a/e2e/react-router/issue-7120/index.html b/e2e/react-router/issue-7120/index.html new file mode 100644 index 0000000000..de92cad2a3 --- /dev/null +++ b/e2e/react-router/issue-7120/index.html @@ -0,0 +1,12 @@ + + + + + + Issue 7120 + + +
+ + + diff --git a/e2e/react-router/issue-7120/package.json b/e2e/react-router/issue-7120/package.json new file mode 100644 index 0000000000..da30014471 --- /dev/null +++ b/e2e/react-router/issue-7120/package.json @@ -0,0 +1,26 @@ +{ + "name": "tanstack-router-e2e-react-issue-7120", + "private": true, + "type": "module", + "scripts": { + "dev": "vite --port 3000", + "dev:e2e": "vite", + "build": "vite build && tsc --noEmit", + "preview": "vite preview", + "start": "vite", + "test:e2e": "rm -rf port*.txt; playwright test --project=chromium" + }, + "dependencies": { + "@tanstack/react-router": "workspace:^", + "react": "^19.0.0", + "react-dom": "^19.0.0" + }, + "devDependencies": { + "@playwright/test": "^1.50.1", + "@tanstack/router-e2e-utils": "workspace:^", + "@types/react": "^19.0.8", + "@types/react-dom": "^19.0.3", + "@vitejs/plugin-react": "^6.0.1", + "vite": "^8.0.14" + } +} diff --git a/e2e/react-router/issue-7120/playwright.config.ts b/e2e/react-router/issue-7120/playwright.config.ts new file mode 100644 index 0000000000..6ba60eaff1 --- /dev/null +++ b/e2e/react-router/issue-7120/playwright.config.ts @@ -0,0 +1,27 @@ +import { defineConfig, devices } from '@playwright/test' +import { getTestServerPort } from '@tanstack/router-e2e-utils' +import packageJson from './package.json' with { type: 'json' } + +const PORT = await getTestServerPort(packageJson.name) +const baseURL = `http://localhost:${PORT}` + +export default defineConfig({ + testDir: './tests', + workers: 1, + reporter: [['line']], + use: { + baseURL, + }, + webServer: { + command: `VITE_NODE_ENV="test" VITE_SERVER_PORT=${PORT} pnpm build && pnpm preview --port ${PORT}`, + url: baseURL, + reuseExistingServer: !process.env.CI, + stdout: 'pipe', + }, + projects: [ + { + name: 'chromium', + use: { ...devices['Desktop Chrome'] }, + }, + ], +}) diff --git a/e2e/react-router/issue-7120/src/main.tsx b/e2e/react-router/issue-7120/src/main.tsx new file mode 100644 index 0000000000..08c46f529a --- /dev/null +++ b/e2e/react-router/issue-7120/src/main.tsx @@ -0,0 +1,72 @@ +import ReactDOM from 'react-dom/client' +import { + Outlet, + RouterProvider, + createRootRoute, + createRoute, + createRouter, + redirect, +} from '@tanstack/react-router' + +const posts = [ + { + id: '1', + title: 'sunt aut facere repellat provident occaecati', + }, +] + +const rootRoute = createRootRoute({ + component: RootComponent, + pendingMs: 0, + pendingComponent: () => { + ;(globalThis as any).__pendingSeen = true + return
loading
+ }, + beforeLoad: async ({ matches }) => { + if (matches.find((match) => match.routeId === '/posts')) { + return + } + + await new Promise((resolve) => setTimeout(resolve, 1000)) + throw redirect({ to: '/posts' }) + }, +}) + +function RootComponent() { + return +} + +const indexRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/', + component: () =>
Home
, +}) + +const postsRoute = createRoute({ + getParentRoute: () => rootRoute, + path: 'posts', + loader: async () => { + await new Promise((resolve) => setTimeout(resolve, 10)) + return posts + }, +}).lazy(() => import('./posts.lazy').then((d) => d.Route)) + +const routeTree = rootRoute.addChildren([indexRoute, postsRoute]) + +const router = createRouter({ + routeTree, + defaultViewTransition: true, +}) + +declare module '@tanstack/react-router' { + interface Register { + router: typeof router + } +} + +const rootElement = document.getElementById('app')! + +if (!rootElement.innerHTML) { + const root = ReactDOM.createRoot(rootElement) + root.render() +} diff --git a/e2e/react-router/issue-7120/src/posts.lazy.tsx b/e2e/react-router/issue-7120/src/posts.lazy.tsx new file mode 100644 index 0000000000..f2635f2e08 --- /dev/null +++ b/e2e/react-router/issue-7120/src/posts.lazy.tsx @@ -0,0 +1,25 @@ +import { Link, createLazyRoute } from '@tanstack/react-router' + +export const Route = createLazyRoute('/posts')({ + component: PostsComponent, +}) + +function PostsComponent() { + const posts = Route.useLoaderData() + + return ( +
+
    + {posts.map((post) => { + return ( +
  • + +
    {post.title.substring(0, 20)}
    + +
  • + ) + })} +
+
+ ) +} diff --git a/e2e/react-router/issue-7120/tests/issue-7120.repro.spec.ts b/e2e/react-router/issue-7120/tests/issue-7120.repro.spec.ts new file mode 100644 index 0000000000..3b4d9a65cb --- /dev/null +++ b/e2e/react-router/issue-7120/tests/issue-7120.repro.spec.ts @@ -0,0 +1,28 @@ +import { expect, test } from '@playwright/test' + +test('root beforeLoad redirect does not blank when pending UI and view transitions are enabled (https://github.com/TanStack/router/issues/7120)', async ({ + page, +}) => { + const pageErrors: Array = [] + const consoleErrors: Array = [] + + page.on('pageerror', (error) => { + pageErrors.push(error.message) + }) + page.on('console', (message) => { + if (message.type() === 'error') { + consoleErrors.push(message.text()) + } + }) + + await page.goto('/') + + await expect(page).toHaveURL(/\/posts$/) + await expect(page.getByText('sunt aut facere repe')).toBeVisible() + await expect + .poll(() => page.evaluate(() => (globalThis as any).__pendingSeen)) + .toBe(true) + await expect(page.getByTestId('root-pending')).not.toBeVisible() + expect(pageErrors).toEqual([]) + expect(consoleErrors).toEqual([]) +}) diff --git a/e2e/react-router/issue-7120/tsconfig.json b/e2e/react-router/issue-7120/tsconfig.json new file mode 100644 index 0000000000..4f6089bc08 --- /dev/null +++ b/e2e/react-router/issue-7120/tsconfig.json @@ -0,0 +1,15 @@ +{ + "compilerOptions": { + "strict": true, + "esModuleInterop": true, + "jsx": "react-jsx", + "target": "ESNext", + "moduleResolution": "Bundler", + "module": "ESNext", + "resolveJsonModule": true, + "allowJs": true, + "skipLibCheck": true, + "types": ["vite/client"] + }, + "exclude": ["node_modules", "dist"] +} diff --git a/e2e/react-router/issue-7120/vite.config.js b/e2e/react-router/issue-7120/vite.config.js new file mode 100644 index 0000000000..9ffcc67574 --- /dev/null +++ b/e2e/react-router/issue-7120/vite.config.js @@ -0,0 +1,6 @@ +import { defineConfig } from 'vite' +import react from '@vitejs/plugin-react' + +export default defineConfig({ + plugins: [react()], +}) diff --git a/e2e/react-router/issue-7457/index.html b/e2e/react-router/issue-7457/index.html new file mode 100644 index 0000000000..76369b5957 --- /dev/null +++ b/e2e/react-router/issue-7457/index.html @@ -0,0 +1,10 @@ + + + + + + Issue 7457 + + + + diff --git a/e2e/react-router/issue-7457/package.json b/e2e/react-router/issue-7457/package.json new file mode 100644 index 0000000000..9d9413e213 --- /dev/null +++ b/e2e/react-router/issue-7457/package.json @@ -0,0 +1,27 @@ +{ + "name": "tanstack-router-e2e-react-issue-7457", + "private": true, + "type": "module", + "scripts": { + "dev": "vite --port 3000", + "dev:e2e": "vite", + "build": "vite build && tsc --noEmit", + "preview": "vite preview", + "start": "vite", + "test:e2e": "rm -rf port*.txt; playwright test --project=chromium" + }, + "dependencies": { + "@tanstack/react-router": "workspace:^", + "@tanstack/router-plugin": "workspace:^", + "react": "^19.0.0", + "react-dom": "^19.0.0" + }, + "devDependencies": { + "@playwright/test": "^1.50.1", + "@tanstack/router-e2e-utils": "workspace:^", + "@types/react": "^19.0.8", + "@types/react-dom": "^19.0.3", + "@vitejs/plugin-react": "^6.0.1", + "vite": "^8.0.14" + } +} diff --git a/e2e/react-router/issue-7457/playwright.config.ts b/e2e/react-router/issue-7457/playwright.config.ts new file mode 100644 index 0000000000..6ba60eaff1 --- /dev/null +++ b/e2e/react-router/issue-7457/playwright.config.ts @@ -0,0 +1,27 @@ +import { defineConfig, devices } from '@playwright/test' +import { getTestServerPort } from '@tanstack/router-e2e-utils' +import packageJson from './package.json' with { type: 'json' } + +const PORT = await getTestServerPort(packageJson.name) +const baseURL = `http://localhost:${PORT}` + +export default defineConfig({ + testDir: './tests', + workers: 1, + reporter: [['line']], + use: { + baseURL, + }, + webServer: { + command: `VITE_NODE_ENV="test" VITE_SERVER_PORT=${PORT} pnpm build && pnpm preview --port ${PORT}`, + url: baseURL, + reuseExistingServer: !process.env.CI, + stdout: 'pipe', + }, + projects: [ + { + name: 'chromium', + use: { ...devices['Desktop Chrome'] }, + }, + ], +}) diff --git a/e2e/react-router/issue-7457/src/main.tsx b/e2e/react-router/issue-7457/src/main.tsx new file mode 100644 index 0000000000..ccf8ec77b2 --- /dev/null +++ b/e2e/react-router/issue-7457/src/main.tsx @@ -0,0 +1,28 @@ +import { StrictMode } from 'react' +import { createRoot } from 'react-dom/client' +import { RouterProvider, createRouter } from '@tanstack/react-router' +import { routeTree } from './routeTree.gen' + +const router = createRouter({ + routeTree, + defaultPendingComponent: DefaultPendingComponent, + defaultPreloadStaleTime: 0, +}) + +declare module '@tanstack/react-router' { + interface Register { + router: typeof router + } +} + +function DefaultPendingComponent() { + ;(globalThis as any).__pendingSeen = true + + return
loading
+} + +createRoot(document.body).render( + + + , +) diff --git a/e2e/react-router/issue-7457/src/routeTree.gen.ts b/e2e/react-router/issue-7457/src/routeTree.gen.ts new file mode 100644 index 0000000000..72a8f23384 --- /dev/null +++ b/e2e/react-router/issue-7457/src/routeTree.gen.ts @@ -0,0 +1,77 @@ +/* eslint-disable */ + +// @ts-nocheck + +// noinspection JSUnusedGlobalSymbols + +// This file was automatically generated by TanStack Router. +// You should NOT make any changes in this file as it will be overwritten. +// Additionally, you should also exclude this file from your linter and/or formatter to prevent it from being checked or modified. + +import { Route as rootRouteImport } from './routes/__root' +import { Route as AnotherRouteImport } from './routes/another' +import { Route as IndexRouteImport } from './routes/index' + +const AnotherRoute = AnotherRouteImport.update({ + id: '/another', + path: '/another', + getParentRoute: () => rootRouteImport, +} as any) +const IndexRoute = IndexRouteImport.update({ + id: '/', + path: '/', + getParentRoute: () => rootRouteImport, +} as any) + +export interface FileRoutesByFullPath { + '/': typeof IndexRoute + '/another': typeof AnotherRoute +} +export interface FileRoutesByTo { + '/': typeof IndexRoute + '/another': typeof AnotherRoute +} +export interface FileRoutesById { + __root__: typeof rootRouteImport + '/': typeof IndexRoute + '/another': typeof AnotherRoute +} +export interface FileRouteTypes { + fileRoutesByFullPath: FileRoutesByFullPath + fullPaths: '/' | '/another' + fileRoutesByTo: FileRoutesByTo + to: '/' | '/another' + id: '__root__' | '/' | '/another' + fileRoutesById: FileRoutesById +} +export interface RootRouteChildren { + IndexRoute: typeof IndexRoute + AnotherRoute: typeof AnotherRoute +} + +declare module '@tanstack/react-router' { + interface FileRoutesByPath { + '/another': { + id: '/another' + path: '/another' + fullPath: '/another' + preLoaderRoute: typeof AnotherRouteImport + parentRoute: typeof rootRouteImport + } + '/': { + id: '/' + path: '/' + fullPath: '/' + preLoaderRoute: typeof IndexRouteImport + parentRoute: typeof rootRouteImport + } + } +} + +const rootRouteChildren: RootRouteChildren = { + IndexRoute: IndexRoute, + AnotherRoute: AnotherRoute, +} +export const routeTree = rootRouteImport + ._addFileChildren(rootRouteChildren) + ._addFileTypes() diff --git a/e2e/react-router/issue-7457/src/routes/__root.tsx b/e2e/react-router/issue-7457/src/routes/__root.tsx new file mode 100644 index 0000000000..d3dced6736 --- /dev/null +++ b/e2e/react-router/issue-7457/src/routes/__root.tsx @@ -0,0 +1,16 @@ +import { Outlet, createRootRoute } from '@tanstack/react-router' + +export const Route = createRootRoute({ + beforeLoad: async () => { + await new Promise((resolve) => setTimeout(resolve, 1500)) + }, + component: RootComponent, +}) + +function RootComponent() { + return ( +
+ +
+ ) +} diff --git a/e2e/react-router/issue-7457/src/routes/another.tsx b/e2e/react-router/issue-7457/src/routes/another.tsx new file mode 100644 index 0000000000..2274ee634b --- /dev/null +++ b/e2e/react-router/issue-7457/src/routes/another.tsx @@ -0,0 +1,9 @@ +import { createFileRoute } from '@tanstack/react-router' + +export const Route = createFileRoute('/another')({ + component: RouteComponent, +}) + +function RouteComponent() { + return
Hello "/another"!
+} diff --git a/e2e/react-router/issue-7457/src/routes/index.tsx b/e2e/react-router/issue-7457/src/routes/index.tsx new file mode 100644 index 0000000000..91f9a1c4eb --- /dev/null +++ b/e2e/react-router/issue-7457/src/routes/index.tsx @@ -0,0 +1,15 @@ +import { createFileRoute, redirect } from '@tanstack/react-router' + +export const Route = createFileRoute('/')({ + beforeLoad: () => { + throw redirect({ + to: '/another', + replace: true, + }) + }, + component: RouteComponent, +}) + +function RouteComponent() { + return
You should never see this!
+} diff --git a/e2e/react-router/issue-7457/src/vite-env.d.ts b/e2e/react-router/issue-7457/src/vite-env.d.ts new file mode 100644 index 0000000000..11f02fe2a0 --- /dev/null +++ b/e2e/react-router/issue-7457/src/vite-env.d.ts @@ -0,0 +1 @@ +/// diff --git a/e2e/react-router/issue-7457/tests/issue-7457.repro.spec.ts b/e2e/react-router/issue-7457/tests/issue-7457.repro.spec.ts new file mode 100644 index 0000000000..fd1c582bcf --- /dev/null +++ b/e2e/react-router/issue-7457/tests/issue-7457.repro.spec.ts @@ -0,0 +1,28 @@ +import { expect, test } from '@playwright/test' + +test('initial child beforeLoad redirect after async root beforeLoad does not blank (https://github.com/TanStack/router/issues/7457)', async ({ + page, +}) => { + const pageErrors: Array = [] + const consoleErrors: Array = [] + + page.on('pageerror', (error) => { + pageErrors.push(error.message || String(error)) + }) + page.on('console', (message) => { + if (message.type() === 'error') { + consoleErrors.push(message.text()) + } + }) + + await page.goto('/') + + await expect(page).toHaveURL(/\/another$/) + await expect(page.getByText('Hello "/another"!')).toBeVisible() + await expect + .poll(() => page.evaluate(() => (globalThis as any).__pendingSeen)) + .toBe(true) + await expect(page.getByTestId('app-pending')).not.toBeVisible() + expect(pageErrors).toEqual([]) + expect(consoleErrors).toEqual([]) +}) diff --git a/e2e/react-router/issue-7457/tsconfig.json b/e2e/react-router/issue-7457/tsconfig.json new file mode 100644 index 0000000000..4f6089bc08 --- /dev/null +++ b/e2e/react-router/issue-7457/tsconfig.json @@ -0,0 +1,15 @@ +{ + "compilerOptions": { + "strict": true, + "esModuleInterop": true, + "jsx": "react-jsx", + "target": "ESNext", + "moduleResolution": "Bundler", + "module": "ESNext", + "resolveJsonModule": true, + "allowJs": true, + "skipLibCheck": true, + "types": ["vite/client"] + }, + "exclude": ["node_modules", "dist"] +} diff --git a/e2e/react-router/issue-7457/vite.config.js b/e2e/react-router/issue-7457/vite.config.js new file mode 100644 index 0000000000..9e39ea83d9 --- /dev/null +++ b/e2e/react-router/issue-7457/vite.config.js @@ -0,0 +1,10 @@ +import { defineConfig } from 'vite' +import react from '@vitejs/plugin-react' +import { tanstackRouter } from '@tanstack/router-plugin/vite' + +export default defineConfig({ + plugins: [ + tanstackRouter({ target: 'react', autoCodeSplitting: true }), + react(), + ], +}) diff --git a/e2e/vue-router/view-transitions/tests/view-transitions.spec.ts b/e2e/vue-router/view-transitions/tests/view-transitions.spec.ts index 31d8b12b04..d38e059360 100644 --- a/e2e/vue-router/view-transitions/tests/view-transitions.spec.ts +++ b/e2e/vue-router/view-transitions/tests/view-transitions.spec.ts @@ -20,7 +20,8 @@ test.beforeEach(async ({ page }) => { // ignore } - return { finished: Promise.resolve() } + const updateCallbackDone = Promise.resolve() + return { finished: updateCallbackDone, updateCallbackDone } } }) diff --git a/packages/react-router/src/Match.tsx b/packages/react-router/src/Match.tsx index 5de5546aeb..97b996041d 100644 --- a/packages/react-router/src/Match.tsx +++ b/packages/react-router/src/Match.tsx @@ -7,7 +7,6 @@ import { getLocationChangeInfo, invariant, isNotFound, - isRedirect, rootRouteId, } from '@tanstack/router-core' import { isServer } from '@tanstack/router-core/isServer' @@ -306,7 +305,9 @@ export const MatchInner = React.memo(function MatchInnerImpl({ key: 'displayPendingPromise' | 'minPendingPromise' | 'loadPromise', ) => { return ( - router.getMatch(match.id)?._nonReactive[key] ?? match._nonReactive[key] + router.getMatch(match.id)?._nonReactive[key] ?? + match._nonReactive[key] ?? + router.latestLoadPromise ) } @@ -325,8 +326,7 @@ export const MatchInner = React.memo(function MatchInnerImpl({ const routeId = match.routeId as string const route = router.routesById[routeId] as AnyRoute const remountFn = - (router.routesById[routeId] as AnyRoute).options.remountDeps ?? - router.options.defaultRemountDeps + route.options.remountDeps ?? router.options.defaultRemountDeps const remountDeps = remountFn?.({ routeId, loaderDeps: match.loaderDeps, @@ -360,17 +360,6 @@ export const MatchInner = React.memo(function MatchInnerImpl({ return renderRouteNotFound(router, route, match.error) } - if (match.status === 'redirected') { - if (!isRedirect(match.error)) { - if (process.env.NODE_ENV !== 'production') { - throw new Error('Invariant failed: Expected a redirect error') - } - - invariant() - } - throw getMatchPromise(match, 'loadPromise') - } - if (match.status === 'error') { const RouteErrorComponent = (route.options.errorComponent ?? @@ -407,8 +396,7 @@ export const MatchInner = React.memo(function MatchInnerImpl({ // eslint-disable-next-line react-hooks/rules-of-hooks const key = React.useMemo(() => { const remountFn = - (router.routesById[routeId] as AnyRoute).options.remountDeps ?? - router.options.defaultRemountDeps + route.options.remountDeps ?? router.options.defaultRemountDeps const remountDeps = remountFn?.({ routeId, loaderDeps: match.loaderDeps, @@ -421,8 +409,8 @@ export const MatchInner = React.memo(function MatchInnerImpl({ match.loaderDeps, match._strictParams, match._strictSearch, + route.options.remountDeps, router.options.defaultRemountDeps, - router.routesById, ]) // eslint-disable-next-line react-hooks/rules-of-hooks @@ -478,22 +466,6 @@ export const MatchInner = React.memo(function MatchInnerImpl({ return renderRouteNotFound(router, route, match.error) } - if (match.status === 'redirected') { - // A match can be observed as redirected during an in-flight transition, - // especially when pending UI is already rendering. Suspend on the match's - // load promise so React can abandon this stale render and continue the - // redirect transition. - if (!isRedirect(match.error)) { - if (process.env.NODE_ENV !== 'production') { - throw new Error('Invariant failed: Expected a redirect error') - } - - invariant() - } - - throw getMatchPromise(match, 'loadPromise') - } - if (match.status === 'error') { // If we're on the server, we need to use React's new and super // wonky api for throwing errors from a server side render inside diff --git a/packages/react-router/tests/Scripts.test.tsx b/packages/react-router/tests/Scripts.test.tsx index b7605c9402..8422b1d29f 100644 --- a/packages/react-router/tests/Scripts.test.tsx +++ b/packages/react-router/tests/Scripts.test.tsx @@ -508,6 +508,82 @@ describe('ssr HeadContent', () => { ).toHaveLength(1) }) + test('removes stale child title when parent beforeLoad throws', async () => { + const history = createMemoryHistory({ + initialEntries: ['/parent/child?fail=false'], + }) + + const rootRoute = createRootRoute({ + component: () => { + return ( + <> + {createPortal(, document.head)} + + + ) + }, + }) + + const parentRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/parent', + validateSearch: (search: Record) => ({ + fail: search.fail === true || search.fail === 'true', + }), + beforeLoad: ({ search }) => { + if (search.fail) { + throw new Error('Parent beforeLoad failed') + } + }, + head: ({ match }) => ({ + meta: [ + { + title: match.error ? 'Parent error title' : 'Parent success title', + }, + ], + }), + errorComponent: () =>
Parent error boundary
, + component: () => , + }) + + const childRoute = createRoute({ + getParentRoute: () => parentRoute, + path: '/child', + head: () => ({ + meta: [{ title: 'Child success title' }], + }), + component: () =>
Child success
, + }) + + const router = createRouter({ + history, + routeTree: rootRoute.addChildren([parentRoute.addChildren([childRoute])]), + }) + + await act(() => render()) + + expect(await screen.findByText('Child success')).toBeInTheDocument() + await waitFor(() => { + expect(document.title).toBe('Child success title') + }) + + await act(() => + router.navigate({ + to: '/parent/child', + search: { fail: true }, + } as never), + ) + + expect(await screen.findByText('Parent error boundary')).toBeInTheDocument() + expect(router.state.matches.map((match) => match.routeId)).toEqual([ + rootRoute.id, + parentRoute.id, + ]) + await waitFor(() => { + expect(document.title).toBe('Parent error title') + }) + }) + test('applies assetCrossOrigin to manifest stylesheets and preloads', async () => { const history = createTestBrowserHistory() const stylesheetHref = '/asset-cross-origin.css' diff --git a/packages/react-router/tests/not-found.test.tsx b/packages/react-router/tests/not-found.test.tsx index 3754785d35..4ea1f00ae9 100644 --- a/packages/react-router/tests/not-found.test.tsx +++ b/packages/react-router/tests/not-found.test.tsx @@ -409,6 +409,94 @@ test('beforeLoad notFound with routeId targets parent boundary and preserves par expect(screen.queryByTestId('child-component')).not.toBeInTheDocument() }) +test('loader notFound with routeId preserves parent route context for notFoundComponent', async () => { + const rootRoute = createRootRoute({ + component: () => , + }) + + const parentRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/parent', + beforeLoad: () => ({ number: 42 }), + component: () => , + notFoundComponent: () => { + const context = parentRoute.useRouteContext() + return ( + {context.number} + ) + }, + }) + + const childRoute = createRoute({ + getParentRoute: () => parentRoute, + path: '/child', + loader: () => { + throw notFound({ routeId: parentRoute.id }) + }, + component: () => Child, + }) + + const router = createRouter({ + routeTree: rootRoute.addChildren([parentRoute.addChildren([childRoute])]), + history, + notFoundMode: 'fuzzy', + }) + + render() + await router.navigate({ to: '/parent/child' }) + + expect( + await screen.findByTestId('parent-not-found-context'), + ).toHaveTextContent('42') + expect(screen.queryByTestId('child-component')).not.toBeInTheDocument() +}) + +test('beforeLoad notFound clears stale context before rendering own notFoundComponent', async () => { + let shouldThrow = false + + const rootRoute = createRootRoute({ + component: () => , + }) + + const childRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/child', + beforeLoad: () => { + if (shouldThrow) { + throw notFound() + } + + return { number: 42 } + }, + component: () => Child, + notFoundComponent: () => { + const context = childRoute.useRouteContext() + return ( + + {String(context.number)} + + ) + }, + }) + + const router = createRouter({ + routeTree: rootRoute.addChildren([childRoute]), + history, + notFoundMode: 'fuzzy', + }) + + render() + await router.navigate({ to: '/child' }) + expect(await screen.findByTestId('child-component')).toBeInTheDocument() + + shouldThrow = true + await router.invalidate() + + expect( + await screen.findByTestId('child-not-found-context'), + ).toHaveTextContent('undefined') +}) + test('beforeLoad notFound with non-exact routeId falls back to root notFoundComponent', async () => { const rootRoute = createRootRoute({ component: () => , diff --git a/packages/react-router/tests/routeContext.test.tsx b/packages/react-router/tests/routeContext.test.tsx index d11f86420e..a9c8b0f1ec 100644 --- a/packages/react-router/tests/routeContext.test.tsx +++ b/packages/react-router/tests/routeContext.test.tsx @@ -2796,6 +2796,422 @@ describe('useRouteContext in the component', () => { expect(content).toBeInTheDocument() }) + test('context value from beforeLoad is propagated to a sub-route while its loader reloads in the background', async () => { + let sawUndefinedContext = false + + const rootRoute = createRootRoute({ + component: () => , + }) + const homeRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/', + component: () =>
Home page
, + }) + const contextPropagationRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/context-propagation', + beforeLoad: () => ({ number: 42 }), + component: () => , + }) + const contextPropagationIndexRoute = createRoute({ + getParentRoute: () => contextPropagationRoute, + path: '/', + staleTime: 0, + loader: async () => { + await sleep(WAIT_TIME) + }, + component: () => { + const { number } = contextPropagationIndexRoute.useRouteContext() + sawUndefinedContext ||= number === undefined + + return ( +
+ number = {String(number)}, saw undefined ={' '} + {String(sawUndefinedContext)} +
+ ) + }, + }) + + const routeTree = rootRoute.addChildren([ + homeRoute, + contextPropagationRoute.addChildren([contextPropagationIndexRoute]), + ]) + const router = createRouter({ routeTree, history }) + + render() + + await act(() => router.navigate({ to: '/context-propagation' })) + + expect( + await screen.findByText('number = 42, saw undefined = false'), + ).toBeInTheDocument() + + await act(() => router.navigate({ to: '/' })) + + expect(await screen.findByText('Home page')).toBeInTheDocument() + + act(() => router.history.back()) + + expect( + await screen.findByText('number = 42, saw undefined = false'), + ).toBeInTheDocument() + + // let the background reload settle, the context must survive the + // loader's success update + await act(() => sleep(WAIT_TIME + 50)) + + expect( + await screen.findByText('number = 42, saw undefined = false'), + ).toBeInTheDocument() + }) + + test('context value from beforeLoad is propagated to a sub-route when it is re-entered while its loader is still reloading in the background', async () => { + let sawUndefinedContext = false + const loaderTime = WAIT_TIME * 3 + + const rootRoute = createRootRoute({ + component: () => , + }) + const homeRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/', + component: () =>
Home page
, + }) + const reloadInFlightRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/reload-in-flight', + beforeLoad: () => ({ number: 42 }), + component: () => , + }) + const reloadInFlightIndexRoute = createRoute({ + getParentRoute: () => reloadInFlightRoute, + path: '/', + staleTime: 0, + loader: async () => { + await sleep(loaderTime) + }, + component: () => { + const { number } = reloadInFlightIndexRoute.useRouteContext() + sawUndefinedContext ||= number === undefined + + return ( +
+ number = {String(number)}, saw undefined ={' '} + {String(sawUndefinedContext)} +
+ ) + }, + }) + + const routeTree = rootRoute.addChildren([ + homeRoute, + reloadInFlightRoute.addChildren([reloadInFlightIndexRoute]), + ]) + const router = createRouter({ routeTree, history }) + + render() + + await act(() => router.navigate({ to: '/reload-in-flight' })) + + expect( + await screen.findByText('number = 42, saw undefined = false'), + ).toBeInTheDocument() + + // re-entering the route starts a background reload of the sub-route + await act(() => router.navigate({ to: '/' })) + expect(await screen.findByText('Home page')).toBeInTheDocument() + act(() => router.history.back()) + + expect( + await screen.findByText('number = 42, saw undefined = false'), + ).toBeInTheDocument() + + // re-enter once more while that background reload is still in flight + await act(() => router.navigate({ to: '/' })) + expect(await screen.findByText('Home page')).toBeInTheDocument() + act(() => router.history.back()) + + expect( + await screen.findByText('number = 42, saw undefined = false'), + ).toBeInTheDocument() + + // let the in-flight reload settle, the context must survive its completion + await act(() => sleep(loaderTime + 50)) + + expect( + await screen.findByText('number = 42, saw undefined = false'), + ).toBeInTheDocument() + }) + + test('context value from beforeLoad is kept when the background reload of a sub-route is aborted', async () => { + let sawUndefinedContext = false + let loaderRuns = 0 + + const rootRoute = createRootRoute({ + component: () => , + }) + const homeRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/', + component: () =>
Home page
, + }) + const reloadAbortRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/reload-abort', + beforeLoad: () => ({ number: 42 }), + component: () => , + }) + const reloadAbortIndexRoute = createRoute({ + getParentRoute: () => reloadAbortRoute, + path: '/', + staleTime: 0, + loader: async () => { + loaderRuns++ + await sleep(WAIT_TIME) + if (loaderRuns > 1) { + const error = new Error('aborted') + error.name = 'AbortError' + throw error + } + }, + component: () => { + const { number } = reloadAbortIndexRoute.useRouteContext() + sawUndefinedContext ||= number === undefined + + return ( +
+ number = {String(number)}, saw undefined ={' '} + {String(sawUndefinedContext)} +
+ ) + }, + }) + + const routeTree = rootRoute.addChildren([ + homeRoute, + reloadAbortRoute.addChildren([reloadAbortIndexRoute]), + ]) + const router = createRouter({ routeTree, history }) + + render() + + await act(() => router.navigate({ to: '/reload-abort' })) + + expect( + await screen.findByText('number = 42, saw undefined = false'), + ).toBeInTheDocument() + + // re-entering the route starts a background reload which gets aborted + await act(() => router.navigate({ to: '/' })) + expect(await screen.findByText('Home page')).toBeInTheDocument() + act(() => router.history.back()) + + expect( + await screen.findByText('number = 42, saw undefined = false'), + ).toBeInTheDocument() + + // let the aborted reload settle, the context must survive the abort + await act(() => sleep(WAIT_TIME + 50)) + + expect( + await screen.findByText('number = 42, saw undefined = false'), + ).toBeInTheDocument() + }) + + test("context value from beforeLoad is available in a sub-route's errorComponent when its background reload fails", async () => { + let loaderRuns = 0 + + const rootRoute = createRootRoute({ + component: () => , + }) + const homeRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/', + component: () =>
Home page
, + }) + const reloadErrorRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/reload-error', + beforeLoad: () => ({ number: 42 }), + component: () => , + }) + const reloadErrorIndexRoute = createRoute({ + getParentRoute: () => reloadErrorRoute, + path: '/', + staleTime: 0, + loader: async () => { + loaderRuns++ + await sleep(WAIT_TIME) + if (loaderRuns > 1) { + throw new Error('loader failed') + } + }, + component: () => { + const { number } = reloadErrorIndexRoute.useRouteContext() + + return
number = {String(number)}
+ }, + errorComponent: () => { + const { number } = reloadErrorIndexRoute.useRouteContext() + + return
error number = {String(number)}
+ }, + }) + + const routeTree = rootRoute.addChildren([ + homeRoute, + reloadErrorRoute.addChildren([reloadErrorIndexRoute]), + ]) + const router = createRouter({ routeTree, history }) + + render() + + await act(() => router.navigate({ to: '/reload-error' })) + + expect(await screen.findByText('number = 42')).toBeInTheDocument() + + // re-entering the route starts a background reload which fails + await act(() => router.navigate({ to: '/' })) + expect(await screen.findByText('Home page')).toBeInTheDocument() + act(() => router.history.back()) + + expect(await screen.findByText('number = 42')).toBeInTheDocument() + + // once the failed reload settles, the errorComponent must still see + // the context value provided by the parent's beforeLoad + await act(() => sleep(WAIT_TIME + 50)) + + expect(await screen.findByText('error number = 42')).toBeInTheDocument() + }) + + test("context value from beforeLoad is available in a sub-route's errorComponent when the sub-route's beforeLoad throws", async () => { + const rootRoute = createRootRoute({ + component: () => , + }) + const homeRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/', + component: () =>
Home page
, + }) + const beforeLoadErrorRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/before-load-error', + beforeLoad: () => ({ number: 42 }), + component: () => , + }) + const beforeLoadErrorIndexRoute = createRoute({ + getParentRoute: () => beforeLoadErrorRoute, + path: '/', + beforeLoad: () => { + throw new Error('beforeLoad failed') + }, + component: () =>
never rendered
, + errorComponent: () => { + const { number } = beforeLoadErrorIndexRoute.useRouteContext() + + return
error number = {String(number)}
+ }, + }) + + const routeTree = rootRoute.addChildren([ + homeRoute, + beforeLoadErrorRoute.addChildren([beforeLoadErrorIndexRoute]), + ]) + const router = createRouter({ routeTree, history }) + + render() + + await act(() => router.navigate({ to: '/before-load-error' })) + + expect(await screen.findByText('error number = 42')).toBeInTheDocument() + }) + + test('updated context value from beforeLoad is committed atomically with a blocking reload of the sub-route', async () => { + let sawUndefinedContext = false + let beforeLoadRuns = 0 + let loaderRuns = 0 + let releaseLoader!: () => void + const loaderGate = new Promise((resolve) => { + releaseLoader = resolve + }) + + const rootRoute = createRootRoute({ + component: () => , + }) + const homeRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/', + component: () =>
Home page
, + }) + const blockingReloadRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/blocking-reload', + beforeLoad: () => ({ number: ++beforeLoadRuns }), + component: () => , + }) + const blockingReloadIndexRoute = createRoute({ + getParentRoute: () => blockingReloadRoute, + path: '/', + loader: async () => { + loaderRuns++ + if (loaderRuns > 1) { + await loaderGate + } + }, + component: () => { + const { number } = blockingReloadIndexRoute.useRouteContext() + sawUndefinedContext ||= number === undefined + + return ( +
+ number = {String(number)}, saw undefined ={' '} + {String(sawUndefinedContext)} +
+ ) + }, + }) + + const routeTree = rootRoute.addChildren([ + homeRoute, + blockingReloadRoute.addChildren([blockingReloadIndexRoute]), + ]) + const router = createRouter({ + routeTree, + history, + defaultStaleReloadMode: 'blocking', + }) + + render() + + await act(() => router.navigate({ to: '/blocking-reload' })) + + expect( + await screen.findByText('number = 1, saw undefined = false'), + ).toBeInTheDocument() + + // invalidating re-runs beforeLoad and blocks on the gated loader + act(() => { + void router.invalidate() + }) + + // while the blocking reload is in flight, the visible UI must keep the + // old, consistent pass: updates of the in-progress pass stay isolated + // in the pending match pool until the whole pass commits + await act(() => sleep(25)) + expect( + screen.getByText('number = 1, saw undefined = false'), + ).toBeInTheDocument() + + // releasing the loader commits the pass: the updated context must become + // visible together with the reload result + act(() => releaseLoader()) + + expect( + await screen.findByText('number = 2, saw undefined = false'), + ).toBeInTheDocument() + }) + // Check if context that is updated at the root, is the same in the root route test('modified route context, present in the root route', async () => { const rootRoute = createRootRoute({ diff --git a/packages/react-router/tests/store-updates-during-navigation.test.tsx b/packages/react-router/tests/store-updates-during-navigation.test.tsx index 0c4c1b4146..1e9cbd8e20 100644 --- a/packages/react-router/tests/store-updates-during-navigation.test.tsx +++ b/packages/react-router/tests/store-updates-during-navigation.test.tsx @@ -136,7 +136,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // This number should be as small as possible to minimize the amount of work // that needs to be done during a navigation. // Any change that increases this number should be investigated. - expect(updates).toBe(8) + expect(updates).toBe(7) }) test('redirection in preload', async () => { diff --git a/packages/router-core/src/Matches.ts b/packages/router-core/src/Matches.ts index 852d186b67..3e3e6fe888 100644 --- a/packages/router-core/src/Matches.ts +++ b/packages/router-core/src/Matches.ts @@ -131,7 +131,7 @@ export interface RouteMatch< pathname: string params: TAllParams _strictParams: TAllParams - status: 'pending' | 'success' | 'error' | 'redirected' | 'notFound' + status: 'pending' | 'success' | 'error' | 'notFound' isFetching: false | 'beforeLoad' | 'loader' error: unknown paramsError: unknown diff --git a/packages/router-core/src/load-matches.ts b/packages/router-core/src/load-matches.ts index f901a0c97d..28fc8c7fd6 100644 --- a/packages/router-core/src/load-matches.ts +++ b/packages/router-core/src/load-matches.ts @@ -1,9 +1,9 @@ import { isServer } from '@tanstack/router-core/isServer' -import { invariant } from './invariant' import { createControlledPromise, isPromise } from './utils' import { isNotFound } from './not-found' import { rootRouteId } from './root' import { isRedirect } from './redirect' +import type { ControlledPromise } from './utils' import type { NotFoundError } from './not-found' import type { ParsedLocation } from './location' import type { @@ -13,7 +13,7 @@ import type { SsrContextOptions, } from './route' import type { AnyRouteMatch, MakeRouteMatch } from './Matches' -import type { AnyRouter, SSROption, UpdateMatchFn } from './router' +import type { AnyRouter, SSROption } from './router' /** * An object of this shape is created when calling `loadMatches`. @@ -28,75 +28,177 @@ type InnerLoadContext = { firstBadMatchIndex?: number /** mutable state, scoped to a `loadMatches` call */ rendered?: boolean - serialError?: unknown - updateMatch: UpdateMatchFn matches: Array - preload?: boolean + /** Set only for preload passes. Contains active ids this preload must join, not mutate. */ + preload?: Set + cancel?: ControlledPromise forceStaleReload?: boolean - onReady?: () => Promise + onReady?: (matches: Array) => Promise sync?: boolean } const triggerOnReady = (inner: InnerLoadContext): void | Promise => { if (!inner.rendered) { inner.rendered = true - return inner.onReady?.() + return inner.onReady?.(inner.matches) } } -const hasForcePendingActiveMatch = (router: AnyRouter): boolean => { - return router.stores.matchesId.get().some((matchId) => { - return router.stores.matchStores.get(matchId)?.get()._forcePending - }) +const isPreloadMatch = (inner: InnerLoadContext, matchId: string): boolean => { + return !!inner.preload && !isJoinedPreload(inner, matchId) +} + +const isJoinedPreload = (inner: InnerLoadContext, matchId: string): boolean => { + return !!inner.preload?.has(matchId) } -const resolvePreload = (inner: InnerLoadContext, matchId: string): boolean => { - return !!(inner.preload && !inner.router.stores.matchStores.has(matchId)) +const joinPreloadedActiveMatch = async ( + inner: InnerLoadContext, + index: number, + waitForLoader: boolean, +): Promise => { + const matchId = inner.matches[index]!.id + const cancelJoinedPreload = (): void => { + inner.router.clearCache({ + filter: (match) => inner.matches.includes(match), + }) + inner.cancel?.resolve(inner) + } + const throwCancelledPreload = (): never => { + cancelJoinedPreload() + throw inner + } + const cancelIfOwnerMissing = () => { + // Cached matches do not prove a borrowed active/pending owner still exists. + if (!inner.router.getMatch(matchId, false)) { + cancelJoinedPreload() + } + } + + let match = inner.router.getMatch(matchId, false) ?? throwCancelledPreload() + const route = inner.router.looseRoutesById[match.routeId]! + + const beforeLoadPromise = + match._nonReactive.beforeLoadPromise || + (route.options.beforeLoad && + match.status === 'pending' && + match.fetchCount === 0 + ? match._nonReactive.loadPromise + : undefined) + if (beforeLoadPromise?.status === 'pending') { + await beforeLoadPromise + match = inner.router.getMatch(matchId, false) ?? throwCancelledPreload() + } + + inner.matches[index] = match + let error = match._nonReactive.error || match.error + + if (!error && waitForLoader && match.status === 'pending') { + const loaderPromise = + match._nonReactive.loaderPromise || match._nonReactive.loadPromise + if (loaderPromise?.status === 'pending') { + await loaderPromise + match = inner.router.getMatch(matchId, false) ?? throwCancelledPreload() + } + inner.matches[index] = match + error = match._nonReactive.error || match.error + } else if (!waitForLoader && match.status === 'pending') { + const loaderPromise = + match._nonReactive.loaderPromise || match._nonReactive.loadPromise + if (loaderPromise?.status === 'pending') { + inner.cancel ??= createControlledPromise() + loaderPromise.then(cancelIfOwnerMissing, cancelIfOwnerMissing) + } + } + + handleRedirectOrNotFound(inner, match, error) + if (match.status === 'error' || match.status === 'notFound') { + inner.firstBadMatchIndex ??= index + throw error + } + + return match } /** - * Builds the accumulated context from router options and all matches up to (and optionally including) the given index. + * Builds the accumulated context from router options and all matches up to the given index. * Merges __routeContext and __beforeLoadContext from each match. */ const buildMatchContext = ( inner: InnerLoadContext, index: number, - includeCurrentMatch: boolean = true, ): Record => { - const context: Record = { - ...(inner.router.options.context ?? {}), - } - const end = includeCurrentMatch ? index : index - 1 - for (let i = 0; i <= end; i++) { - const innerMatch = inner.matches[i] - if (!innerMatch) continue - const m = inner.router.getMatch(innerMatch.id) - if (!m) continue - Object.assign(context, m.__routeContext, m.__beforeLoadContext) + const context: Record = Object.assign( + {}, + inner.router.options.context, + ) + for (let i = 0; i <= index; i++) { + const match = inner.matches[i]! + Object.assign(context, match.__routeContext, match.__beforeLoadContext) } return context } -const getNotFoundBoundaryIndex = ( +const commitMatch = ( inner: InnerLoadContext, - err: NotFoundError, -): number | undefined => { - if (!inner.matches.length) { - return undefined + matchId: string, + patch: Partial, +): void => { + if (isJoinedPreload(inner, matchId)) { + return + } + + inner.router.updateMatch(matchId, (prev) => ({ + ...prev, + ...patch, + })) + const match = inner.router.getMatch(matchId) + if (match) { + inner.matches[match.index] = match } +} + +const settleBeforeLoadPromise = (match: AnyRouteMatch): void => { + match._nonReactive.beforeLoadPromise?.resolve() + match._nonReactive.beforeLoadPromise = undefined +} +const settleLoaderPromise = (match: AnyRouteMatch): void => { + match._nonReactive.loaderPromise?.resolve() + match._nonReactive.loaderPromise = undefined +} + +const settleLoadPromises = (match: AnyRouteMatch): void => { + settleLoaderPromise(match) + match._nonReactive.loadPromise?.resolve() + match._nonReactive.loadPromise = undefined +} + +const clearPending = (match: AnyRouteMatch): void => { + clearTimeout(match._nonReactive.pendingTimeout) + match._nonReactive.pendingTimeout = undefined + match._nonReactive.minPendingPromise?.resolve() + match._nonReactive.minPendingPromise = undefined +} + +export const clearMatchPromises = (match: AnyRouteMatch): void => { + clearPending(match) + settleBeforeLoadPromise(match) + settleLoadPromises(match) +} + +const getNotFoundBoundaryIndex = ( + inner: InnerLoadContext, + err: NotFoundError, +): number => { const requestedRouteId = err.routeId - const matchedRootIndex = inner.matches.findIndex( - (m) => m.routeId === inner.router.routeTree.id, - ) - const rootIndex = matchedRootIndex >= 0 ? matchedRootIndex : 0 let startIndex = requestedRouteId ? inner.matches.findIndex((match) => match.routeId === requestedRouteId) : (inner.firstBadMatchIndex ?? inner.matches.length - 1) if (startIndex < 0) { - startIndex = rootIndex + startIndex = 0 } for (let i = startIndex; i >= 0; i--) { @@ -109,98 +211,84 @@ const getNotFoundBoundaryIndex = ( // If no boundary component is found, preserve explicit routeId targeting behavior, // otherwise default to root for untargeted notFounds. - return requestedRouteId ? startIndex : rootIndex + return requestedRouteId ? startIndex : 0 } -const handleRedirectAndNotFound = ( +const handleRedirectOrNotFound = ( inner: InnerLoadContext, - match: AnyRouteMatch | undefined, + match: AnyRouteMatch, err: unknown, ): void => { - if (!isRedirect(err) && !isNotFound(err)) return + if (isRedirect(err)) { + if (err.redirectHandled && !err.options.reloadDocument) { + throw err + } + if (isJoinedPreload(inner, match.id)) { + throw err + } - if (isRedirect(err) && err.redirectHandled && !err.options.reloadDocument) { - throw err + match._nonReactive.error = err + + if ( + inner.preload || + inner.router.stores.cachedMatchStores.get(match.id)?.get() === match + ) { + clearMatchPromises(match) + inner.router.clearCache({ filter: (d) => d.id === match.id }) + } else { + // A redirect is not renderable navigation state. Keep the current + // renderable status (pending or success) until the redirect target + // commits, but clear fetching state. + clearPending(match) + settleBeforeLoadPromise(match) + settleLoaderPromise(match) + commitMatch(inner, match.id, { + isFetching: false as const, + }) + } + + inner.rendered = true + err.options._fromLocation = inner.location + err.redirectHandled = true + throw inner.router.resolveRedirect(err) } - // in case of a redirecting match during preload, the match does not exist - if (match) { - match._nonReactive.beforeLoadPromise?.resolve() - match._nonReactive.loaderPromise?.resolve() - match._nonReactive.beforeLoadPromise = undefined - match._nonReactive.loaderPromise = undefined + if (isNotFound(err)) { + if (isJoinedPreload(inner, match.id)) { + throw err + } match._nonReactive.error = err + clearMatchPromises(match) - inner.updateMatch(match.id, (prev) => ({ - ...prev, - status: isRedirect(err) - ? 'redirected' - : isNotFound(err) - ? 'notFound' - : prev.status === 'pending' - ? 'success' - : prev.status, - context: buildMatchContext(inner, match.index), - isFetching: false, - error: err, - })) - - if (isNotFound(err) && !err.routeId) { + if (!err.routeId) { // Stamp the throwing match's routeId so that the finalization step in - // loadMatches knows where the notFound originated. The actual boundary - // resolution (walking up to the nearest notFoundComponent) is deferred to - // the finalization step, where firstBadMatchIndex is stable and - // headMaxIndex can be capped correctly. + // loadMatches knows where the notFound originated. The actual boundary + // resolution is deferred until firstBadMatchIndex is stable. err.routeId = match.routeId } - match._nonReactive.loadPromise?.resolve() - } + commitMatch(inner, match.id, { + status: 'notFound', + error: err, + isFetching: false, + _forcePending: undefined, + }) - if (isRedirect(err)) { - inner.rendered = true - err.options._fromLocation = inner.location - err.redirectHandled = true - err = inner.router.resolveRedirect(err) + throw err } - - throw err } -const shouldSkipLoader = ( +const shouldSkipMatchLoad = ( inner: InnerLoadContext, - matchId: string, + match: AnyRouteMatch, ): boolean => { - const match = inner.router.getMatch(matchId) - if (!match) { - return true + if (isServer ?? inner.router.isServer) { + return match.ssr === false } - // upon hydration, we skip the loader if the match has been dehydrated on the server - if (!(isServer ?? inner.router.isServer) && match._nonReactive.dehydrated) { - return true - } - - if ((isServer ?? inner.router.isServer) && match.ssr === false) { - return true - } - - return false -} - -const syncMatchContext = ( - inner: InnerLoadContext, - matchId: string, - index: number, -): void => { - const nextContext = buildMatchContext(inner, index) - inner.updateMatch(matchId, (prev) => { - return { - ...prev, - context: nextContext, - } - }) + // upon hydration, we skip the loader if the match has been dehydrated on the server + return !!match._nonReactive.dehydrated } const handleSerialError = ( @@ -208,7 +296,8 @@ const handleSerialError = ( index: number, err: any, ): void => { - const { id: matchId, routeId } = inner.matches[index]! + const match = inner.matches[index]! + const { id: matchId, routeId } = match const route = inner.router.looseRoutesById[routeId]! // Much like suspense, we use a promise here to know if @@ -219,32 +308,37 @@ const handleSerialError = ( } inner.firstBadMatchIndex ??= index - handleRedirectAndNotFound(inner, inner.router.getMatch(matchId), err) + match.__beforeLoadContext = undefined + + const currentMatch = inner.router.getMatch(matchId)! + currentMatch.__beforeLoadContext = undefined + + handleRedirectOrNotFound(inner, currentMatch, err) try { route.options.onError?.(err) } catch (errorHandlerErr) { err = errorHandlerErr - handleRedirectAndNotFound(inner, inner.router.getMatch(matchId), err) + // The current match's pending beforeLoad context was already cleared above. + handleRedirectOrNotFound(inner, currentMatch, err) } - inner.updateMatch(matchId, (prev) => { - prev._nonReactive.beforeLoadPromise?.resolve() - prev._nonReactive.beforeLoadPromise = undefined - prev._nonReactive.loadPromise?.resolve() - - return { - ...prev, - error: err, - status: 'error', - isFetching: false, - updatedAt: Date.now(), - abortController: new AbortController(), - } + // A match that errors during the beforeLoad phase never reaches the loader + // phase. Settle its promises after committing the error state. + commitMatch(inner, matchId, { + __beforeLoadContext: undefined, + error: err, + status: 'error', + isFetching: false, + _forcePending: undefined, + context: buildMatchContext(inner, index), + updatedAt: Date.now(), + abortController: new AbortController(), }) - if (!inner.preload && !isRedirect(err) && !isNotFound(err)) { - inner.serialError ??= err + const updatedMatch = inner.router.getMatch(matchId) + if (updatedMatch) { + clearMatchPromises(updatedMatch) } } @@ -332,18 +426,21 @@ const setupPendingTimeout = ( const shouldPending = !!( inner.onReady && !(isServer ?? inner.router.isServer) && - !resolvePreload(inner, matchId) && + !isPreloadMatch(inner, matchId) && (route.options.loader || route.options.beforeLoad || routeNeedsPreload(route)) && typeof pendingMs === 'number' && pendingMs !== Infinity && (route.options.pendingComponent ?? - (inner.router.options as any)?.defaultPendingComponent) + (inner.router.options as any).defaultPendingComponent) ) if (shouldPending) { const pendingTimeout = setTimeout(() => { + // the timeout has served its purpose, clear it so that a later load + // pass of this match can arm a new one + match._nonReactive.pendingTimeout = undefined // Update the match and prematurely resolve the loadMatches promise so that // the pending component can start rendering triggerOnReady(inner) @@ -352,39 +449,6 @@ const setupPendingTimeout = ( } } -const preBeforeLoadSetup = ( - inner: InnerLoadContext, - matchId: string, - route: AnyRoute, -): void | Promise => { - const existingMatch = inner.router.getMatch(matchId)! - - // 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) - if ( - !existingMatch._nonReactive.beforeLoadPromise && - !existingMatch._nonReactive.loaderPromise - ) - return - - setupPendingTimeout(inner, matchId, route, existingMatch) - - const then = () => { - const match = inner.router.getMatch(matchId)! - if ( - match.preload && - (match.status === 'redirected' || match.status === 'notFound') - ) { - handleRedirectAndNotFound(inner, match, match.error) - } - } - - // Wait for the previous beforeLoad to resolve before we continue - return existingMatch._nonReactive.beforeLoadPromise - ? existingMatch._nonReactive.beforeLoadPromise.then(then) - : then() -} - const executeBeforeLoad = ( inner: InnerLoadContext, matchId: string, @@ -400,64 +464,84 @@ const executeBeforeLoad = ( prevLoadPromise = undefined }) - const { paramsError, searchError } = match - - if (paramsError) { - handleSerialError(inner, index, paramsError) - } - - if (searchError) { - handleSerialError(inner, index, searchError) + const serialError = match.paramsError || match.searchError + if (serialError) { + handleSerialError(inner, index, serialError) + return } setupPendingTimeout(inner, matchId, route, match) + const beforeLoad = route.options.beforeLoad const abortController = new AbortController() - let isPending = false const pending = () => { - if (isPending) return + if (isPending) { + return + } isPending = true - inner.updateMatch(matchId, (prev) => ({ - ...prev, + const currentMatch = inner.router.getMatch(matchId)! + commitMatch(inner, matchId, { isFetching: 'beforeLoad', - fetchCount: prev.fetchCount + 1, + fetchCount: currentMatch.fetchCount + 1, abortController, // Note: We intentionally don't update context here. // Context should only be updated after beforeLoad resolves to avoid // components seeing incomplete context during async beforeLoad execution. - })) - } - - const resolve = () => { - match._nonReactive.beforeLoadPromise?.resolve() - match._nonReactive.beforeLoadPromise = undefined - inner.updateMatch(matchId, (prev) => ({ - ...prev, - isFetching: false, - })) + }) } - // if there is no `beforeLoad` option, just mark as pending and resolve - // Context will be updated later in loadRouteMatch after loader completes - if (!route.options.beforeLoad) { + const commitBeforeLoad = (beforeLoadContext: any) => { inner.router.batch(() => { pending() - resolve() + inner.matches[index]!.__beforeLoadContext = beforeLoadContext + commitMatch(inner, matchId, { + isFetching: false as const, + __beforeLoadContext: beforeLoadContext, + context: buildMatchContext(inner, index), + }) }) + settleBeforeLoadPromise(match) + } + + // if there is no `beforeLoad` option, just mark as pending and resolve. + // The undefined beforeLoad context is still committed here to clear any + // stale context from a previous load generation of the same match. + if (!beforeLoad) { + commitBeforeLoad(undefined) return } - match._nonReactive.beforeLoadPromise = createControlledPromise() + const beforeLoadPromise = createControlledPromise() + const isCurrentBeforeLoad = () => + inner.router.getMatch(matchId)?._nonReactive.beforeLoadPromise === + beforeLoadPromise + + // commits the result of the beforeLoad phase and settles its promise + const updateContext = (beforeLoadContext: any) => { + if (!isCurrentBeforeLoad()) { + return + } + + if (isRedirect(beforeLoadContext) || isNotFound(beforeLoadContext)) { + pending() + handleSerialError(inner, index, beforeLoadContext) + return + } + + commitBeforeLoad(beforeLoadContext) + } + + match._nonReactive.beforeLoadPromise = beforeLoadPromise // Build context from all parent matches, excluding current match's __beforeLoadContext // (since we're about to execute beforeLoad for this match) - const context = { - ...buildMatchContext(inner, index, false), - ...match.__routeContext, - } + const context = Object.assign( + buildMatchContext(inner, index - 1), + match.__routeContext, + ) const { search, params, cause } = match - const preload = resolvePreload(inner, matchId) + const preload = isPreloadMatch(inner, matchId) const beforeLoadFnContext: BeforeLoadContextOptions< any, any, @@ -487,43 +571,22 @@ const executeBeforeLoad = ( ...inner.router.options.additionalContext, } - const updateContext = (beforeLoadContext: any) => { - if (beforeLoadContext === undefined) { - inner.router.batch(() => { - pending() - resolve() - }) - return - } - if (isRedirect(beforeLoadContext) || isNotFound(beforeLoadContext)) { - pending() - handleSerialError(inner, index, beforeLoadContext) - } - - inner.router.batch(() => { - pending() - inner.updateMatch(matchId, (prev) => ({ - ...prev, - __beforeLoadContext: beforeLoadContext, - })) - resolve() - }) - } - let beforeLoadContext try { - beforeLoadContext = route.options.beforeLoad(beforeLoadFnContext) + beforeLoadContext = beforeLoad(beforeLoadFnContext) if (isPromise(beforeLoadContext)) { pending() - return beforeLoadContext - .catch((err) => { - handleSerialError(inner, index, err) - }) - .then(updateContext) + return beforeLoadContext.then(updateContext, (err) => { + if (!isCurrentBeforeLoad()) { + return + } + handleSerialError(inner, index, err) + }) } } catch (err) { pending() handleSerialError(inner, index, err) + return } updateContext(beforeLoadContext) @@ -533,75 +596,48 @@ const executeBeforeLoad = ( const handleBeforeLoad = ( inner: InnerLoadContext, index: number, -): void | Promise => { +): void | Promise => { const { id: matchId, routeId } = inner.matches[index]! const route = inner.router.looseRoutesById[routeId]! - const serverSsr = () => { - // on the server, determine whether SSR the current match or not - if (isServer ?? inner.router.isServer) { - const maybePromise = isBeforeLoadSsr(inner, matchId, index, route) - if (isPromise(maybePromise)) return maybePromise.then(queueExecution) - } - return queueExecution() + if (isJoinedPreload(inner, matchId)) { + return joinPreloadedActiveMatch(inner, index, false) } - const execute = () => executeBeforeLoad(inner, matchId, index, route) - const queueExecution = () => { - if (shouldSkipLoader(inner, matchId)) return - const result = preBeforeLoadSetup(inner, matchId, route) - return isPromise(result) ? result.then(execute) : execute() - } + const existingMatch = inner.router.getMatch(matchId) + if (!existingMatch || shouldSkipMatchLoad(inner, existingMatch)) { + return + } - return serverSsr() -} + // 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) + const pendingBeforeLoad = existingMatch._nonReactive.beforeLoadPromise + if (pendingBeforeLoad) { + setupPendingTimeout(inner, matchId, route, existingMatch) + return pendingBeforeLoad.then(() => { + const match = inner.router.getMatch(matchId) + if (!match || shouldSkipMatchLoad(inner, match)) { + return + } -const executeHead = ( - inner: InnerLoadContext, - matchId: string, - route: AnyRoute, -): void | Promise< - Pick< - AnyRouteMatch, - 'meta' | 'links' | 'headScripts' | 'headers' | 'scripts' | 'styles' - > -> => { - const match = inner.router.getMatch(matchId) - // in case of a redirecting match during preload, the match does not exist - if (!match) { - return - } - if (!route.options.head && !route.options.scripts && !route.options.headers) { - return - } - const assetContext = { - ssr: inner.router.options.ssr, - matches: inner.matches, - match, - params: match.params, - loaderData: match.loaderData, - } + return executeBeforeLoad(inner, matchId, index, route) + }) + } - return Promise.all([ - route.options.head?.(assetContext), - route.options.scripts?.(assetContext), - route.options.headers?.(assetContext), - ]).then(([headFnContent, scripts, headers]) => { - const meta = headFnContent?.meta - const links = headFnContent?.links - const headScripts = headFnContent?.scripts - const styles = headFnContent?.styles - - return { - meta, - links, - headScripts, - headers, - scripts, - styles, + if (existingMatch._nonReactive.loaderPromise) { + setupPendingTimeout(inner, matchId, route, existingMatch) } - }) + + return executeBeforeLoad(inner, matchId, index, route) + } + + // on the server, determine whether to SSR the current match or not + if (isServer ?? inner.router.isServer) { + const maybePromise = isBeforeLoadSsr(inner, matchId, index, route) + if (isPromise(maybePromise)) return maybePromise.then(queueExecution) + } + return queueExecution() } const getLoaderContext = ( @@ -617,17 +653,17 @@ const getLoaderContext = ( const context = buildMatchContext(inner, index) - const preload = resolvePreload(inner, matchId) + const preload = isPreloadMatch(inner, matchId) return { params, deps: loaderDeps, - preload: !!preload, + preload, parentMatchPromise, abortController, context, location: inner.location, - navigate: (opts) => + navigate: (opts: any) => inner.router.navigate({ ...opts, _fromLocation: inner.location, @@ -645,139 +681,143 @@ const runLoader = async ( index: number, route: AnyRoute, ): Promise => { + // If the Matches component rendered the pending component and needs to show + // it for a minimum duration, we'll wait for it to resolve before committing + // to the match and resolving the loadPromise. + const match = inner.router.getMatch(matchId)! + const loaderBucket = match._nonReactive + const loaderPromise = loaderBucket.loaderPromise + const isCurrentLoader = () => loaderBucket.loaderPromise === loaderPromise + const getCurrentMatch = () => + isCurrentLoader() ? inner.router.getMatch(matchId) : undefined + + // Actually run the loader and handle the result try { - // If the Matches component rendered - // the pending component and needs to show it for - // a minimum duration, we''ll wait for it to resolve - // before committing to the match and resolving - // the loadPromise + if (!(isServer ?? inner.router.isServer) || match.ssr === true) { + loadRouteChunk(route) + } - const match = inner.router.getMatch(matchId)! + // Kick off the loader! + const routeLoader = route.options.loader + const loader = + typeof routeLoader === 'function' ? routeLoader : routeLoader?.handler + const loaderResult = loader?.( + getLoaderContext(inner, matchPromises, matchId, index, route), + ) + const loaderResultIsPromise = isPromise(loaderResult) - // Actually run the loader and handle the result - try { - if (!(isServer ?? inner.router.isServer) || match.ssr === true) { - loadRouteChunk(route) - } + if ( + loaderResultIsPromise || + route._lazyPromise || + route._componentsPromise || + route.options.head || + route.options.scripts || + route.options.headers || + loaderBucket.minPendingPromise + ) { + commitMatch(inner, matchId, { + isFetching: 'loader', + }) + } - // Kick off the loader! - const routeLoader = route.options.loader - const loader = - typeof routeLoader === 'function' ? routeLoader : routeLoader?.handler - const loaderResult = loader?.( - getLoaderContext(inner, matchPromises, matchId, index, route), - ) - const loaderResultIsPromise = !!loader && isPromise(loaderResult) - - const willLoadSomething = !!( - loaderResultIsPromise || - route._lazyPromise || - route._componentsPromise || - route.options.head || - route.options.scripts || - route.options.headers || - match._nonReactive.minPendingPromise - ) + if (loader) { + const loaderData = loaderResultIsPromise + ? await loaderResult + : loaderResult - if (willLoadSomething) { - inner.updateMatch(matchId, (prev) => ({ - ...prev, - isFetching: 'loader', - })) + if (!getCurrentMatch()) { + return } - if (loader) { - const loaderData = loaderResultIsPromise - ? await loaderResult - : loaderResult + if (isRedirect(loaderData) || isNotFound(loaderData)) { + throw loaderData + } - handleRedirectAndNotFound( - inner, - inner.router.getMatch(matchId), + if (loaderData !== undefined) { + commitMatch(inner, matchId, { loaderData, - ) - if (loaderData !== undefined) { - inner.updateMatch(matchId, (prev) => ({ - ...prev, - loaderData, - })) - } + }) } + } - // Lazy option can modify the route options, - // so we need to wait for it to resolve before - // we can use the options - if (route._lazyPromise) await route._lazyPromise - const pendingPromise = match._nonReactive.minPendingPromise - if (pendingPromise) await pendingPromise - - // Last but not least, wait for the components - // to be preloaded before we resolve the match - if (route._componentsPromise) await route._componentsPromise - inner.updateMatch(matchId, (prev) => ({ - ...prev, - error: undefined, - context: buildMatchContext(inner, index), - status: 'success', - isFetching: false, - updatedAt: Date.now(), - })) - } catch (e) { - let error = e - - if ((error as any)?.name === 'AbortError') { - if (match.abortController.signal.aborted) { - match._nonReactive.loaderPromise?.resolve() - match._nonReactive.loaderPromise = undefined - return - } - inner.updateMatch(matchId, (prev) => ({ - ...prev, - status: prev.status === 'pending' ? 'success' : prev.status, - isFetching: false, - context: buildMatchContext(inner, index), - })) + // Lazy option can modify the route options, + // so we need to wait for it to resolve before + // we can use the options + if (route._lazyPromise) await route._lazyPromise + const pendingPromise = loaderBucket.minPendingPromise + if (pendingPromise) await pendingPromise + + // Last but not least, wait for the components + // to be preloaded before we resolve the match + if (route._componentsPromise) await route._componentsPromise + if (!isCurrentLoader()) { + return + } + commitMatch(inner, matchId, { + error: undefined, + status: 'success', + isFetching: false as const, + updatedAt: Date.now(), + }) + } catch (e) { + let error = e + + if (isRedirect(e) && e.redirectHandled) { + throw e + } + + if ((error as any)?.name === 'AbortError') { + if (match.abortController.signal.aborted) { + return + } + const currentMatch = getCurrentMatch() + if (!currentMatch) { return } + // a softly aborted pending match keeps its previous data and is + // committed as success + commitMatch(inner, matchId, { + status: + currentMatch.status === 'pending' ? 'success' : currentMatch.status, + isFetching: false, + }) + return + } - const pendingPromise = match._nonReactive.minPendingPromise - if (pendingPromise) await pendingPromise + const pendingPromise = loaderBucket.minPendingPromise + if (pendingPromise) await pendingPromise + let currentMatch = getCurrentMatch() + if (!currentMatch) { + return + } - if (isNotFound(e)) { - await (route.options.notFoundComponent as any)?.preload?.() + if (isNotFound(e)) { + await (route.options.notFoundComponent as any)?.preload?.() + currentMatch = getCurrentMatch() + if (!currentMatch) { + return } + } - handleRedirectAndNotFound(inner, inner.router.getMatch(matchId), e) - - try { - route.options.onError?.(e) - } catch (onErrorError) { - error = onErrorError - handleRedirectAndNotFound( - inner, - inner.router.getMatch(matchId), - onErrorError, - ) - } - if (!isRedirect(error) && !isNotFound(error)) { - await loadRouteChunk(route, ['errorComponent']) - } + handleRedirectOrNotFound(inner, currentMatch, e) + inner.firstBadMatchIndex ??= index - inner.updateMatch(matchId, (prev) => ({ - ...prev, - error, - context: buildMatchContext(inner, index), - status: 'error', - isFetching: false, - })) + try { + route.options.onError?.(e) + } catch (onErrorError) { + error = onErrorError + handleRedirectOrNotFound(inner, currentMatch, onErrorError) } - } catch (err) { - const match = inner.router.getMatch(matchId) - // in case of a redirecting match during preload, the match does not exist - if (match) { - match._nonReactive.loaderPromise = undefined + await loadRouteChunk(route, ['errorComponent']) + if (!isCurrentLoader()) { + return } - handleRedirectAndNotFound(inner, match, err) + + commitMatch(inner, matchId, { + error, + status: 'error', + isFetching: false, + }) } } @@ -786,111 +826,47 @@ const loadRouteMatch = async ( matchPromises: Array>, index: number, ): Promise => { - async function handleLoader( - preload: boolean, - prevMatch: AnyRouteMatch, - previousRouteMatchId: string | undefined, - match: AnyRouteMatch, - route: AnyRoute, - ) { - const age = Date.now() - prevMatch.updatedAt - - const staleAge = preload - ? (route.options.preloadStaleTime ?? - inner.router.options.defaultPreloadStaleTime ?? - 30_000) // 30 seconds for preloads by default - : (route.options.staleTime ?? inner.router.options.defaultStaleTime ?? 0) - - const shouldReloadOption = route.options.shouldReload - - // Default to reloading the route all the time - // Allow shouldReload to get the last say, - // if provided. - const shouldReload = - typeof shouldReloadOption === 'function' - ? shouldReloadOption( - getLoaderContext(inner, matchPromises, matchId, index, route), - ) - : shouldReloadOption - - // If the route is successful and still fresh, just resolve - const { status, invalid } = match - const staleMatchShouldReload = - age >= staleAge && - (!!inner.forceStaleReload || - match.cause === 'enter' || - (previousRouteMatchId !== undefined && - previousRouteMatchId !== match.id)) - loaderShouldRunAsync = - status === 'success' && - (invalid || (shouldReload ?? staleMatchShouldReload)) - if (preload && route.options.preload === false) { - // Do nothing - } else if ( - loaderShouldRunAsync && - !inner.sync && - shouldReloadInBackground - ) { - loaderIsRunningAsync = true - ;(async () => { - try { - await runLoader(inner, matchPromises, matchId, index, route) - const match = inner.router.getMatch(matchId)! - 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) - } - } - })() - } else if (status !== 'success' || loaderShouldRunAsync) { - await runLoader(inner, matchPromises, matchId, index, route) - } else { - syncMatchContext(inner, matchId, index) - } - } - const { id: matchId, routeId } = inner.matches[index]! - let loaderShouldRunAsync = false - let loaderIsRunningAsync = false const route = inner.router.looseRoutesById[routeId]! - const routeLoader = route.options.loader - const shouldReloadInBackground = - ((typeof routeLoader === 'function' - ? undefined - : routeLoader?.staleReloadMode) ?? - inner.router.options.defaultStaleReloadMode) !== 'blocking' - - if (shouldSkipLoader(inner, matchId)) { - const match = inner.router.getMatch(matchId) - if (!match) { - return inner.matches[index]! - } + // becomes true when this pass leaves the loader running detached in the + // background, in which case finalization is deferred to that detached run + let loaderIsRunningAsync = false + let loaderGeneration: AnyRouteMatch['_nonReactive']['loaderPromise'] + let matchToLoad: AnyRouteMatch | undefined - syncMatchContext(inner, matchId, index) + if (isJoinedPreload(inner, matchId)) { + return await joinPreloadedActiveMatch(inner, index, true) + } + + const prevMatch = inner.router.getMatch(matchId) + if (!prevMatch) { + // in case of a redirecting match during preload, the match does not exist + return inner.matches[index]! + } + + if (shouldSkipMatchLoad(inner, prevMatch)) { + // the beforeLoad phase (and with it the context commit) does not run for + // skipped matches, so commit the merged route context here + commitMatch(inner, matchId, { + invalid: false, + context: buildMatchContext(inner, index), + }) if (isServer ?? inner.router.isServer) { return inner.router.getMatch(matchId)! } } else { - const prevMatch = inner.router.getMatch(matchId)! // This is where all of the stale-while-revalidate magic happens - const activeIdAtIndex = inner.router.stores.matchesId.get()[index] - const activeAtIndex = - (activeIdAtIndex && - inner.router.stores.matchStores.get(activeIdAtIndex)) || - null - const previousRouteMatchId = - activeAtIndex?.routeId === routeId - ? activeIdAtIndex - : inner.router.stores.matches.get().find((d) => d.routeId === routeId) - ?.id - const preload = resolvePreload(inner, matchId) + const routeLoader = route.options.loader + const shouldReloadInBackground = + ((typeof routeLoader === 'function' + ? undefined + : routeLoader?.staleReloadMode) ?? + inner.router.options.defaultStaleReloadMode) !== 'blocking' + const preload = isPreloadMatch(inner, matchId) // there is a loaderPromise, so we are in the middle of a load if (prevMatch._nonReactive.loaderPromise) { + loaderGeneration = prevMatch._nonReactive.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 @@ -900,72 +876,160 @@ const loadRouteMatch = async ( !prevMatch.preload && shouldReloadInBackground ) { - return prevMatch - } - await prevMatch._nonReactive.loaderPromise - const match = inner.router.getMatch(matchId)! - const error = match._nonReactive.error || match.error - if (error) { - handleRedirectAndNotFound(inner, match, error) + // this load pass hands the match over to the still in-flight reload; + // finalization is skipped, so clear invalid here without touching + // promises or loader state. + if (prevMatch.invalid !== false) { + commitMatch(inner, matchId, { + invalid: false, + }) + } + return inner.router.getMatch(matchId)! } + await loaderGeneration + const match = inner.router.getMatch(matchId) + if (match) { + const error = match._nonReactive.error || match.error + if (error) { + handleRedirectOrNotFound(inner, match, error) + } - if (match.status === 'pending') { - await handleLoader( - preload, - prevMatch, - previousRouteMatchId, - match, - route, - ) + matchToLoad = match.status === 'pending' ? match : undefined } } else { - const nextPreload = - preload && !inner.router.stores.matchStores.has(matchId) - const match = inner.router.getMatch(matchId)! + const match = prevMatch + // a new load generation starts: any settle error stored by a previous + // generation no longer applies to this one + match._nonReactive.error = undefined match._nonReactive.loaderPromise = createControlledPromise() - if (nextPreload !== match.preload) { - inner.updateMatch(matchId, (prev) => ({ - ...prev, - preload: nextPreload, - })) + loaderGeneration = match._nonReactive.loaderPromise + if (preload !== match.preload) { + commitMatch(inner, matchId, { + preload, + }) } - await handleLoader(preload, prevMatch, previousRouteMatchId, match, route) + matchToLoad = match + } + + if (matchToLoad && !(preload && route.options.preload === false)) { + const { status, invalid } = matchToLoad + let loaderShouldRun = status !== 'success' + + if (!loaderShouldRun) { + const activeIdAtIndex = inner.router.stores.matchesId.get()[index] + const activeAtIndex = + (activeIdAtIndex && + inner.router.stores.matchStores.get(activeIdAtIndex)) || + null + const previousRouteMatchId = + activeAtIndex?.routeId === routeId + ? activeIdAtIndex + : inner.router.stores.matches + .get() + .find((d) => d.routeId === routeId)?.id + const age = Date.now() - prevMatch.updatedAt + const staleAge = preload + ? (route.options.preloadStaleTime ?? + inner.router.options.defaultPreloadStaleTime ?? + 30_000) // 30 seconds for preloads by default + : (route.options.staleTime ?? + inner.router.options.defaultStaleTime ?? + 0) + const shouldReloadOption = route.options.shouldReload + const shouldReload = + typeof shouldReloadOption === 'function' + ? shouldReloadOption( + getLoaderContext(inner, matchPromises, matchId, index, route), + ) + : shouldReloadOption + const staleMatchShouldReload = + age >= staleAge && + (!!inner.forceStaleReload || + matchToLoad.cause === 'enter' || + (previousRouteMatchId !== undefined && + previousRouteMatchId !== matchToLoad.id)) + + loaderShouldRun = invalid || (shouldReload ?? staleMatchShouldReload) + } + + if ( + loaderShouldRun && + status === 'success' && + !inner.sync && + shouldReloadInBackground + ) { + // stale-while-revalidate: leave the loader running detached + loaderIsRunningAsync = true + const backgroundGeneration = matchToLoad._nonReactive.loaderPromise + if (matchToLoad.invalid !== false) { + commitMatch(inner, matchId, { invalid: false }) + } + ;(async () => { + try { + await runLoader(inner, matchPromises, matchId, index, route) + } catch (err) { + if (isRedirect(err)) { + await inner.router.navigate(err.options) + return + } + } + const latestMatch = inner.router.getMatch(matchId) + if ( + latestMatch && + latestMatch._nonReactive.loaderPromise === backgroundGeneration + ) { + settleLoadPromises(latestMatch) + } + })() + } else if (loaderShouldRun) { + const run = runLoader(inner, matchPromises, matchId, index, route) + await (preload && loaderGeneration + ? Promise.race([run, loaderGeneration]) + : run) + } } } - const match = inner.router.getMatch(matchId)! - if (!loaderIsRunningAsync) { - match._nonReactive.loaderPromise?.resolve() - match._nonReactive.loadPromise?.resolve() - match._nonReactive.loadPromise = undefined + + let match = inner.router.getMatch(matchId) + if (!match) { + return inner.matches[index]! + } + if ( + loaderGeneration && + match._nonReactive.loaderPromise && + match._nonReactive.loaderPromise !== loaderGeneration + ) { + return inner.matches[index]! } clearTimeout(match._nonReactive.pendingTimeout) match._nonReactive.pendingTimeout = undefined - if (!loaderIsRunningAsync) match._nonReactive.loaderPromise = undefined match._nonReactive.dehydrated = undefined const nextIsFetching = loaderIsRunningAsync ? match.isFetching : false if (nextIsFetching !== match.isFetching || match.invalid !== false) { - inner.updateMatch(matchId, (prev) => ({ - ...prev, + commitMatch(inner, matchId, { isFetching: nextIsFetching, invalid: false, - })) - return inner.router.getMatch(matchId)! - } else { - return match + }) + match = inner.router.getMatch(matchId)! + } + + if (!loaderIsRunningAsync) { + settleLoadPromises(match) } + + return (inner.matches[index] = match) } export async function loadMatches(arg: { router: AnyRouter location: ParsedLocation matches: Array - preload?: boolean + preload?: Set forceStaleReload?: boolean - onReady?: () => Promise - updateMatch: UpdateMatchFn + onReady?: (matches: Array) => Promise sync?: boolean }): Promise> { const inner: InnerLoadContext = arg @@ -975,7 +1039,7 @@ export async function loadMatches(arg: { // the pending component was already rendered on the server and we want to keep it shown on the client until minPendingMs is reached if ( !(isServer ?? inner.router.isServer) && - hasForcePendingActiveMatch(inner.router) + inner.router.stores.matches.get().some((match) => match._forcePending) ) { triggerOnReady(inner) } @@ -986,82 +1050,88 @@ export async function loadMatches(arg: { for (let i = 0; i < inner.matches.length; i++) { try { const beforeLoad = handleBeforeLoad(inner, i) - if (isPromise(beforeLoad)) await beforeLoad + if (isPromise(beforeLoad)) { + const result = await (inner.cancel + ? Promise.race([beforeLoad, inner.cancel]) + : beforeLoad) + if (result === inner) { + return inner.matches + } + } } catch (err) { - if (isRedirect(err)) { - throw err + if (err === inner) { + return inner.matches } if (isNotFound(err)) { beforeLoadNotFound = err - } else { - if (!inner.preload) throw err + } else if (isRedirect(err) || !inner.preload) { + throw err } break } - if (inner.serialError || inner.firstBadMatchIndex != null) { + if (inner.firstBadMatchIndex != null) { break } } // Execute loaders once, with max index adapted for beforeLoad notFound handling. const baseMaxIndexExclusive = inner.firstBadMatchIndex ?? inner.matches.length - - const boundaryIndex = - beforeLoadNotFound && !inner.preload - ? getNotFoundBoundaryIndex(inner, beforeLoadNotFound) - : undefined - - const maxIndexExclusive = - beforeLoadNotFound && inner.preload - ? 0 - : boundaryIndex !== undefined - ? Math.min(boundaryIndex + 1, baseMaxIndexExclusive) - : baseMaxIndexExclusive + const maxIndexExclusive = beforeLoadNotFound + ? Math.min( + getNotFoundBoundaryIndex(inner, beforeLoadNotFound) + 1, + baseMaxIndexExclusive, + ) + : baseMaxIndexExclusive let firstNotFound: NotFoundError | undefined - let firstUnhandledRejection: unknown for (let i = 0; i < maxIndexExclusive; i++) { matchPromises.push(loadRouteMatch(inner, matchPromises, i)) } - try { - await Promise.all(matchPromises) - } catch { - const settled = await Promise.allSettled(matchPromises) + let settled: Array> | undefined + if (inner.preload) { + settled = await Promise.allSettled(matchPromises) + } else { + try { + await Promise.all(matchPromises) + } catch { + settled = await Promise.allSettled(matchPromises) + } + } + + if (settled) { + let firstRedirect: unknown + let firstUnhandledRejection: unknown for (const result of settled) { if (result.status !== 'rejected') continue const reason = result.reason - if (isRedirect(reason)) { - throw reason + if (inner.preload && reason === inner) { + return inner.matches } - if (isNotFound(reason)) { + if (isRedirect(reason)) { + firstRedirect ??= reason + } else if (isNotFound(reason)) { firstNotFound ??= reason } else { firstUnhandledRejection ??= reason } } + if (firstRedirect) { + throw firstRedirect + } + if (firstUnhandledRejection !== undefined) { throw firstUnhandledRejection } } - const notFoundToThrow = - firstNotFound ?? - (beforeLoadNotFound && !inner.preload ? beforeLoadNotFound : undefined) - - let headMaxIndex = - inner.firstBadMatchIndex !== undefined - ? inner.firstBadMatchIndex - : inner.matches.length - 1 - - if (!notFoundToThrow && beforeLoadNotFound && inner.preload) { - return inner.matches - } + const notFoundToThrow = firstNotFound ?? beforeLoadNotFound + let headMatches = inner.matches if (notFoundToThrow) { // Determine once which matched route will actually render the @@ -1074,20 +1144,11 @@ export async function loadMatches(arg: { notFoundToThrow, ) - if (renderedBoundaryIndex === undefined) { - if (process.env.NODE_ENV !== 'production') { - throw new Error( - 'Invariant failed: Could not find match for notFound boundary', - ) - } - - invariant() - } const boundaryMatch = inner.matches[renderedBoundaryIndex]! const boundaryRoute = inner.router.looseRoutesById[boundaryMatch.routeId]! const defaultNotFoundComponent = (inner.router.options as any) - ?.defaultNotFoundComponent + .defaultNotFoundComponent // Ensure a notFoundComponent exists on the boundary route if (!boundaryRoute.options.notFoundComponent && defaultNotFoundComponent) { @@ -1095,76 +1156,97 @@ export async function loadMatches(arg: { } notFoundToThrow.routeId = boundaryMatch.routeId + const context = buildMatchContext(inner, renderedBoundaryIndex) - const boundaryIsRoot = boundaryMatch.routeId === inner.router.routeTree.id - - inner.updateMatch(boundaryMatch.id, (prev) => ({ - ...prev, - ...(boundaryIsRoot + commitMatch( + inner, + boundaryMatch.id, + boundaryMatch.routeId === rootRouteId ? // For root boundary, use globalNotFound so the root component's // shell still renders and handles the not-found display, // instead of replacing the entire root shell via status='notFound'. - { status: 'success' as const, globalNotFound: true, error: undefined } + { + status: 'success' as const, + globalNotFound: true, + error: undefined, + isFetching: false, + _forcePending: undefined, + context, + } : // For non-root boundaries, set status:'notFound' so MatchInner // renders the notFoundComponent directly. - { status: 'notFound' as const, error: notFoundToThrow }), - isFetching: false, - })) + { + status: 'notFound' as const, + error: notFoundToThrow, + isFetching: false, + _forcePending: undefined, + context, + }, + ) - headMaxIndex = renderedBoundaryIndex + headMatches = inner.matches.slice(0, renderedBoundaryIndex + 1) // Ensure the rendering boundary route chunk (and its lazy components, including // lazy notFoundComponent) is loaded before we continue to head execution/render. await loadRouteChunk(boundaryRoute, ['notFoundComponent']) - } else if (!inner.preload) { - // Clear stale root global-not-found state on normal navigations that do not - // throw notFound. This must live here (instead of only in runLoader success) - // because the root loader may be skipped when data is still fresh. - const rootMatch = inner.matches[0]! - // `rootMatch` is the next match for this navigation. If it is not global - // not-found, then any currently stored root global-not-found is stale. - if (!rootMatch.globalNotFound) { - // `currentRootMatch` is the current store state (from the previous - // navigation/load). Update only when a stale flag is actually present. - const currentRootMatch = inner.router.getMatch(rootMatch.id) - if (currentRootMatch?.globalNotFound) { - inner.updateMatch(rootMatch.id, (prev) => ({ - ...prev, - globalNotFound: false, - error: undefined, - })) - } + } else if (inner.firstBadMatchIndex !== undefined) { + // When a serial error occurred (e.g. beforeLoad threw a regular Error), + // the erroring route's lazy chunk wasn't loaded because loaders were skipped. + // We need to load it so the code-split errorComponent is available for rendering. + if (!inner.preload) { + const errorRoute = + inner.router.looseRoutesById[ + inner.matches[inner.firstBadMatchIndex]!.routeId + ]! + await loadRouteChunk(errorRoute, ['errorComponent']) } - } - // When a serial error occurred (e.g. beforeLoad threw a regular Error), - // the erroring route's lazy chunk wasn't loaded because loaders were skipped. - // We need to load it so the code-split errorComponent is available for rendering. - if (inner.serialError && inner.firstBadMatchIndex !== undefined) { - const errorRoute = - inner.router.looseRoutesById[ - inner.matches[inner.firstBadMatchIndex]!.routeId - ]! - await loadRouteChunk(errorRoute, ['errorComponent']) + for (const match of inner.matches.splice(inner.firstBadMatchIndex + 1)) { + clearMatchPromises(match) + } } // serially execute heads once after loaders/notFound handling, ensuring // all head functions get a chance even if one throws. - for (let i = 0; i <= headMaxIndex; i++) { - const match = inner.matches[i]! + for (const match of headMatches) { const { id: matchId, routeId } = match - const route = inner.router.looseRoutesById[routeId]! + const routeOptions = inner.router.looseRoutesById[routeId]!.options + if (isJoinedPreload(inner, matchId)) { + continue + } try { - const headResult = executeHead(inner, matchId, route) - if (headResult) { - const head = await headResult - inner.updateMatch(matchId, (prev) => ({ - ...prev, - ...head, - })) + const headMatch = + inner.router.getMatch(matchId) ?? (inner.preload && match) + if ( + headMatch && + (routeOptions.head || routeOptions.scripts || routeOptions.headers) + ) { + const assetContext = { + ssr: inner.router.options.ssr, + matches: inner.matches, + match: headMatch, + params: headMatch.params, + loaderData: headMatch.loaderData, + } + + const [headFnContent, scripts, headers] = await Promise.all([ + routeOptions.head?.(assetContext), + routeOptions.scripts?.(assetContext), + routeOptions.headers?.(assetContext), + ]) + commitMatch(inner, matchId, { + meta: headFnContent?.meta, + links: headFnContent?.links, + headScripts: headFnContent?.scripts, + headers, + scripts, + styles: headFnContent?.styles, + }) } } catch (err) { - console.error(`Error executing head for route ${routeId}:`, err) + if (process.env.NODE_ENV !== 'production') { + console.error(`Error executing head for route ${routeId}:`, err) + } } } @@ -1177,14 +1259,23 @@ export async function loadMatches(arg: { throw notFoundToThrow } - if (inner.serialError && !inner.preload && !inner.onReady) { - throw inner.serialError + if ( + !inner.preload && + !inner.onReady && + inner.firstBadMatchIndex !== undefined + ) { + const errorMatch = + inner.router.getMatch(inner.matches[inner.firstBadMatchIndex]!.id) ?? + inner.matches[inner.firstBadMatchIndex]! + if (errorMatch.status === 'error') { + throw errorMatch.error + } } return inner.matches } -export type RouteComponentType = +type RouteComponentType = | 'component' | 'errorComponent' | 'pendingComponent' @@ -1194,11 +1285,16 @@ function preloadRouteComponents( route: AnyRoute, componentTypesToLoad: Array, ): Promise | undefined { - const preloads = componentTypesToLoad - .map((type) => (route.options[type] as any)?.preload?.()) - .filter(Boolean) + let preloads: Array> | undefined + for (const type of componentTypesToLoad) { + const preload = (route.options[type] as any)?.preload?.() + if (preload) { + preloads ||= [] + preloads.push(preload) + } + } - if (preloads.length === 0) return undefined + if (!preloads) return undefined return Promise.all(preloads) as any as Promise } @@ -1221,30 +1317,27 @@ export function loadRouteChunk( } } - const runAfterLazy = () => - route._componentsLoaded - ? undefined - : componentTypesToLoad === componentTypes - ? (() => { - if (route._componentsPromise === undefined) { - const componentsPromise = preloadRouteComponents( - route, - componentTypes, - ) - - if (componentsPromise) { - route._componentsPromise = componentsPromise.then(() => { - route._componentsLoaded = true - route._componentsPromise = undefined // gc promise, we won't need it anymore - }) - } else { - route._componentsLoaded = true - } - } + const runAfterLazy = () => { + if (route._componentsLoaded) { + return + } + if (componentTypesToLoad !== componentTypes) { + return preloadRouteComponents(route, componentTypesToLoad) + } + if (route._componentsPromise === undefined) { + const componentsPromise = preloadRouteComponents(route, componentTypes) - return route._componentsPromise - })() - : preloadRouteComponents(route, componentTypesToLoad) + if (componentsPromise) { + route._componentsPromise = componentsPromise.then(() => { + route._componentsLoaded = true + route._componentsPromise = undefined // gc promise, we won't need it anymore + }) + } else { + route._componentsLoaded = true + } + } + return route._componentsPromise + } return route._lazyPromise ? route._lazyPromise.then(runAfterLazy) @@ -1262,15 +1355,12 @@ function makeMaybe( } export function routeNeedsPreload(route: AnyRoute) { - for (const componentType of componentTypes) { - if ((route.options[componentType] as any)?.preload) { - return true - } - } - return false + return componentTypes.some( + (componentType) => (route.options[componentType] as any)?.preload, + ) } -export const componentTypes: Array = [ +const componentTypes: Array = [ 'component', 'errorComponent', 'pendingComponent', diff --git a/packages/router-core/src/redirect.ts b/packages/router-core/src/redirect.ts index b10fac6d7a..baaacd80d3 100644 --- a/packages/router-core/src/redirect.ts +++ b/packages/router-core/src/redirect.ts @@ -1,3 +1,4 @@ +import { isServer } from '@tanstack/router-core/isServer' import type { NavigateOptions } from './link' import type { AnyRouter, RegisteredRouter } from './router' import type { ParsedLocation } from './location' @@ -134,14 +135,12 @@ export function redirect< } catch {} } - const headers = new Headers(opts.headers) - if (opts.href && headers.get('Location') === null) { - headers.set('Location', opts.href) - } - const response = new Response(null, { status: opts.statusCode, - headers, + headers: + (isServer ?? typeof document === 'undefined') && opts.href + ? getRedirectHeaders(opts) + : opts.headers, }) ;(response as Redirect).options = @@ -154,6 +153,14 @@ export function redirect< return response as Redirect } +function getRedirectHeaders(opts: { href?: string; headers?: HeadersInit }) { + const headers = new Headers(opts.headers) + if (headers.get('Location') === null) { + headers.set('Location', opts.href!) + } + return headers +} + /** Check whether a value is a TanStack Router redirect Response. */ /** Check whether a value is a TanStack Router redirect Response. */ export function isRedirect(obj: any): obj is AnyRedirect { diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index 2197dab737..f20a5737bc 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -36,7 +36,12 @@ import { setupScrollRestoration } from './scroll-restoration' import { defaultParseSearch, defaultStringifySearch } from './searchParams' import { rootRouteId } from './root' import { isRedirect, redirect } from './redirect' -import { loadMatches, loadRouteChunk, routeNeedsPreload } from './load-matches' +import { + clearMatchPromises, + loadMatches, + loadRouteChunk, + routeNeedsPreload, +} from './load-matches' import { composeRewrites, executeRewriteInput, @@ -106,7 +111,6 @@ import type { } from './manifest' import type { AnySchema, AnyValidator } from './validators' import type { NavigateOptions, ResolveRelativePath, ToOptions } from './link' -import type { NotFoundError } from './not-found' import type { AnySerializationAdapter, ValidateSerializableInput, @@ -775,7 +779,14 @@ export interface MatchRoutesFn { ): Array } -export type GetMatchFn = (matchId: string) => AnyRouteMatch | undefined +/** + * Internal match lookup. By default this includes cached matches; pass + * `false` when checking whether a match still has an active/pending owner. + */ +export type GetMatchFn = ( + matchId: string, + includeCached?: boolean, +) => AnyRouteMatch | undefined export type UpdateMatchFn = ( id: string, @@ -1178,7 +1189,6 @@ export class RouterCore< } } - let needsLocationUpdate = false const nextBasepath = this.options.basepath ?? '/' const nextRewriteOption = this.options.rewrite const basepathChanged = basepathWasUnset || prevBasepath !== nextBasepath @@ -1201,26 +1211,19 @@ export class RouterCore< } this.rewrite = - rewrites.length === 0 - ? undefined - : rewrites.length === 1 - ? rewrites[0] - : composeRewrites(rewrites) + rewrites.length > 1 ? composeRewrites(rewrites) : rewrites[0] if (this.history) { this.updateLatestLocation() } - needsLocationUpdate = true - } - - if (needsLocationUpdate && this.stores) { - this.stores.location.set(this.latestLocation) + if (this.stores) { + this.stores.location.set(this.latestLocation) + } } if ( typeof window !== 'undefined' && - 'CSS' in window && typeof window.CSS?.supports === 'function' ) { this.isViewTransitionTypesSupported = window.CSS.supports( @@ -1250,6 +1253,7 @@ export class RouterCore< }) }, ) + if (this.options.routeMasks) { processRouteMasks(this.options.routeMasks, result.processedTree) } @@ -1430,20 +1434,12 @@ export class RouterCore< return this.matchRoutesInternal(pathnameOrNext, locationSearchOrOpts) } - private getParentContext(parentMatch?: AnyRouteMatch) { - const parentMatchId = parentMatch?.id - - const parentContext = !parentMatchId - ? ((this.options.context as any) ?? undefined) - : (parentMatch.context ?? this.options.context ?? undefined) - - return parentContext - } - private matchRoutesInternal( next: ParsedLocation, opts?: MatchRoutesOpts, ): Array { + const throwOnError = opts?.throwOnError + const preload = opts?.preload const matchedRoutesResult = this.getMatchedRoutes(next.pathname) const { foundRoute, routeParams } = matchedRoutesResult let { matchedRoutes } = matchedRoutesResult @@ -1471,14 +1467,9 @@ export class RouterCore< : undefined const matches = new Array(matchedRoutes.length) - // Snapshot of active match state keyed by routeId, used to stabilise - // params/search across navigations. - const previousActiveMatchesByRouteId = new Map() - for (const store of this.stores.matchStores.values()) { - if (store.routeId) { - previousActiveMatchesByRouteId.set(store.routeId, store.get()) - } - } + const previousActiveMatches = this.stores.matches.get() + const getPreviousMatch = (routeId: string) => + previousActiveMatches.find((match) => match.routeId === routeId) for (let index = 0; index < matchedRoutes.length; index++) { const route = matchedRoutes[index]! @@ -1518,7 +1509,7 @@ export class RouterCore< }) } - if (opts?.throwOnError) { + if (throwOnError) { throw searchParamError } @@ -1544,7 +1535,7 @@ export class RouterCore< path: route.fullPath, params: routeParams, decoder: this.pathParamsDecoder, - server: this.isServer, + server: isServer ?? this.isServer, }) // Waste not, want not. If we already have a match for this route, @@ -1563,7 +1554,7 @@ export class RouterCore< const existingMatch = this.getMatch(matchId) - const previousMatch = previousActiveMatchesByRouteId.get(route.id) + const previousMatch = getPreviousMatch(route.id) const strictParams = existingMatch?._strictParams ?? usedParams @@ -1581,7 +1572,7 @@ export class RouterCore< }) } - if (opts?.throwOnError) { + if (throwOnError) { throw paramsError } } @@ -1594,9 +1585,20 @@ export class RouterCore< let match: AnyRouteMatch if (existingMatch) { + const cachedMatch = this.stores.cachedMatchStores.has(matchId) + const promotePreloadedMatch = + !preload && cachedMatch && existingMatch.preload match = { ...existingMatch, cause, + ...(promotePreloadedMatch + ? { + _nonReactive: { + ...existingMatch._nonReactive, + loadPromise: createControlledPromise(), + }, + } + : undefined), params: previousMatch?.params ?? routeParams, _strictParams: strictParams, search: previousMatch @@ -1654,7 +1656,7 @@ export class RouterCore< } } - if (!opts?.preload) { + if (!preload) { // If we have a global not found, mark the right match as global not found match.globalNotFound = globalNotFoundRouteId === route.id } @@ -1662,7 +1664,7 @@ export class RouterCore< // update the searchError if there is one match.searchError = searchError - const parentContext = this.getParentContext(parentMatch) + const parentContext = parentMatch?.context ?? this.options.context match.context = { ...parentContext, @@ -1675,24 +1677,25 @@ export class RouterCore< for (let index = 0; index < matches.length; index++) { const match = matches[index]! - const route = this.looseRoutesById[match.routeId]! + const route = matchedRoutes[index]! const existingMatch = this.getMatch(match.id) // Update the match's params - const previousMatch = previousActiveMatchesByRouteId.get(match.routeId) + const previousMatch = getPreviousMatch(match.routeId) match.params = previousMatch ? nullReplaceEqualDeep(previousMatch.params, routeParams) : routeParams if (!existingMatch) { const parentMatch = matches[index - 1] - const parentContext = this.getParentContext(parentMatch) + const parentContext = parentMatch?.context ?? this.options.context // Update the match's context if (route.options.context) { - const contextFnContext: RouteContextOptions = - { + // Get the route context + match.__routeContext = + route.options.context({ deps: match.loaderDeps, params: match.params, context: parentContext ?? {}, @@ -1705,10 +1708,7 @@ export class RouterCore< preload: !!match.preload, matches, routeId: route.id, - } - // Get the route context - match.__routeContext = - route.options.context(contextFnContext) ?? undefined + } as RouteContextOptions) ?? undefined } match.context = { @@ -1898,10 +1898,10 @@ export class RouterCore< lightweightResult.params, ) - const isAbsoluteTo = destTo?.charCodeAt(0) === 47 - const sourcePath = isAbsoluteTo - ? '/' - : this.resolvePathWithBase(defaultedFromPath, '.') + const sourcePath = + destTo && destTo.charCodeAt(0) === 47 + ? '/' + : this.resolvePathWithBase(defaultedFromPath, '.') // Resolve the destination. Absolute destinations don't need the source path. const nextTo = destTo @@ -1971,7 +1971,7 @@ export class RouterCore< path: nextTo, params: nextParams, decoder: this.pathParamsDecoder, - server: this.isServer, + server: isServer ?? this.isServer, }).interpolatedPath, ).path @@ -2015,12 +2015,12 @@ export class RouterCore< nextSearch = validatedSearch } - nextSearch = applySearchMiddleware({ - search: nextSearch, - dest, + nextSearch = buildMiddlewareChain( destRoutes, - _includeValidateSearch: opts._includeValidateSearch, - }) + nextSearch, + dest, + opts._includeValidateSearch ?? false, + ) // Replace the equal deep nextSearch = nullReplaceEqualDeep(fromSearch, nextSearch) @@ -2227,11 +2227,7 @@ export class RouterCore< }, } - if ( - nextHistory.unmaskOnReload ?? - this.options.unmaskOnReload ?? - false - ) { + if (nextHistory.unmaskOnReload ?? this.options.unmaskOnReload) { nextHistory.state.__tempKey = this.tempLocationKey } } @@ -2341,7 +2337,7 @@ export class RouterCore< if (href) { try { - new URL(`${href}`) + new URL(href) hrefIsUrl = true } catch {} } @@ -2463,148 +2459,122 @@ export class RouterCore< load: LoadFn = async (opts): Promise => { const historyAction = opts?.action?.type - let redirect: AnyRedirect | undefined - let notFound: NotFoundError | undefined - let loadPromise: Promise + const loadPromise = createControlledPromise() const previousLocation = this.stores.resolvedLocation.get() ?? this.stores.location.get() - // eslint-disable-next-line prefer-const - loadPromise = new Promise((resolve) => { - this.startTransition(async () => { - try { - this.beforeLoad() - if (historyAction) { - locationHistoryActions.set(this.latestLocation, historyAction) - } else { - locationHistoryActions.delete(this.latestLocation) - } - const next = this.latestLocation - const prevLocation = this.stores.resolvedLocation.get() - const locationChangeInfo = getLocationChangeInfo(next, prevLocation) - - if (!this.stores.redirect.get()) { - this.emit({ - type: 'onBeforeNavigate', - ...locationChangeInfo, - }) - } + this.latestLoadPromise = loadPromise + this.startTransition(async () => { + try { + this.beforeLoad() + if (historyAction) { + locationHistoryActions.set(this.latestLocation, historyAction) + } else { + locationHistoryActions.delete(this.latestLocation) + } + const next = this.latestLocation + const prevLocation = this.stores.resolvedLocation.get() + const locationChangeInfo = getLocationChangeInfo(next, prevLocation) + + if (!this.stores.redirect.get()) { this.emit({ - type: 'onBeforeLoad', + type: 'onBeforeNavigate', ...locationChangeInfo, }) + } - await loadMatches({ - router: this, - sync: opts?.sync, - forceStaleReload: previousLocation.href === next.href, - matches: this.stores.pendingMatches.get(), - location: next, - updateMatch: this.updateMatch, - // eslint-disable-next-line @typescript-eslint/require-await - onReady: async () => { + this.emit({ + type: 'onBeforeLoad', + ...locationChangeInfo, + }) + await loadMatches({ + router: this, + sync: opts?.sync, + forceStaleReload: previousLocation.href === next.href, + matches: this.stores.pendingMatches.get(), + location: next, + onReady: (pendingMatches) => + new Promise((resolve, reject) => { // Wrap batch in framework-specific transition wrapper (e.g., Solid's startTransition) this.startTransition(() => { this.startViewTransition(async () => { - // this.viewTransitionPromise = createControlledPromise() + if (this.latestLoadPromise !== loadPromise) { + return + } // Commit the pending matches. If a previous match was - // removed, place it in the cachedMatches + // removed, place it in the cachedMatches. // - // exitingMatches uses match.id (routeId + params + loaderDeps) so - // navigating /foo?page=1 → /foo?page=2 correctly caches the page=1 entry. - let exitingMatches: Array | null = null - - // Lifecycle-hook identity uses routeId only so that navigating between - // different params/deps of the same route fires onStay (not onLeave+onEnter). - let hookExitingMatches: Array | null = null - let hookEnteringMatches: Array | null = null - let hookStayingMatches: Array | null = null + const currentMatches = this.stores.matches.get() this.batch(() => { - const pendingMatches = this.stores.pendingMatches.get() - const mountPending = pendingMatches.length - const currentMatches = this.stores.matches.get() - - exitingMatches = mountPending - ? currentMatches.filter( - (match) => - !this.stores.pendingMatchStores.has(match.id), - ) - : null - - // Lifecycle-hook identity: routeId only (route presence in tree) - // Build routeId sets from pools to avoid derived stores. - const pendingRouteIds = new Set() - for (const s of this.stores.pendingMatchStores.values()) { - if (s.routeId) pendingRouteIds.add(s.routeId) - } - const activeRouteIds = new Set() - for (const s of this.stores.matchStores.values()) { - if (s.routeId) activeRouteIds.add(s.routeId) - } - - hookExitingMatches = mountPending - ? currentMatches.filter( - (match) => !pendingRouteIds.has(match.routeId), - ) - : null - hookEnteringMatches = mountPending - ? pendingMatches.filter( - (match) => !activeRouteIds.has(match.routeId), - ) - : null - hookStayingMatches = mountPending - ? pendingMatches.filter((match) => - activeRouteIds.has(match.routeId), - ) - : currentMatches - this.stores.isLoading.set(false) this.stores.loadedAt.set(Date.now()) /** - * When committing new matches, cache any exiting matches that are still usable. - * Routes that resolved with `status: 'error'` or `status: 'notFound'` are - * deliberately excluded from `cachedMatches` so that subsequent invalidations - * or reloads re-run their loaders instead of reusing the failed/not-found data. + * Only successful exiting matches are reusable. Everything + * else must be dropped and have its stale loadPromise + * released so abandoned renders cannot stay suspended. */ - if (mountPending) { + if (pendingMatches.length) { this.stores.setMatches(pendingMatches) this.stores.setPending([]) - this.stores.setCached([ + const nextCachedMatches = [ ...this.stores.cachedMatches.get(), - ...exitingMatches!.filter( - (d) => - d.status !== 'error' && - d.status !== 'notFound' && - d.status !== 'redirected', - ), - ]) + ] + for (const match of currentMatches) { + // Exiting uses match.id (routeId + params + loaderDeps), + // so changing loader deps correctly caches the old entry. + if (!pendingMatches.some((d) => d.id === match.id)) { + if ( + match.status === 'success' && + !isRedirect(match._nonReactive.error) + ) { + nextCachedMatches.push(match) + } else { + clearMatchPromises(match) + } + } + } + this.stores.setCached(nextCachedMatches) this.clearExpiredCache() } }) - // - for (const [matches, hook] of [ - [hookExitingMatches, 'onLeave'], - [hookEnteringMatches, 'onEnter'], - [hookStayingMatches, 'onStay'], - ] as const) { - if (!matches) continue - for (const match of matches as Array) { - this.looseRoutesById[match.routeId]!.options[hook]?.( + for (const match of currentMatches) { + if ( + pendingMatches.length && + !pendingMatches.some((d) => d.routeId === match.routeId) + ) { + this.looseRoutesById[match.routeId]!.options.onLeave?.( match, ) } } - }) + + for (const match of pendingMatches.length + ? pendingMatches + : currentMatches) { + const hook = currentMatches.some( + (d) => d.routeId === match.routeId, + ) + ? 'onStay' + : 'onEnter' + this.looseRoutesById[match.routeId]!.options[hook]?.(match) + } + }).then(resolve, reject) }) - }, - }) - } catch (err) { - if (isRedirect(err)) { - redirect = err + }), + }) + } catch (err) { + if (this.latestLoadPromise === loadPromise) { + const redirect = isRedirect(err) + ? (isServer ?? this.isServer) + ? this.resolveRedirect(err) + : err + : undefined + + if (redirect) { if (!(isServer ?? this.isServer)) { this.navigate({ ...redirect.options, @@ -2612,13 +2582,11 @@ export class RouterCore< ignoreBlocker: true, }) } - } else if (isNotFound(err)) { - notFound = err } const nextStatusCode = redirect ? redirect.status - : notFound + : isNotFound(err) ? 404 : this.stores.matches.get().some((d) => d.status === 'error') ? 500 @@ -2629,129 +2597,101 @@ export class RouterCore< this.stores.redirect.set(redirect) }) } + } - if (this.latestLoadPromise === loadPromise) { - this.commitLocationPromise?.resolve() - this.latestLoadPromise = undefined - this.commitLocationPromise = undefined - } + if (this.latestLoadPromise === loadPromise) { + this.commitLocationPromise?.resolve() + this.latestLoadPromise = undefined + this.commitLocationPromise = undefined + } - resolve() - }) + loadPromise.resolve() }) - this.latestLoadPromise = loadPromise - await loadPromise - while ( - (this.latestLoadPromise as any) && - loadPromise !== this.latestLoadPromise - ) { + while (this.latestLoadPromise && loadPromise !== this.latestLoadPromise) { await this.latestLoadPromise } - let newStatusCode: number | undefined = undefined - if (this.hasNotFoundMatch()) { - newStatusCode = 404 - } else if (this.stores.matches.get().some((d) => d.status === 'error')) { - newStatusCode = 500 - } - if (newStatusCode !== undefined) { + const newStatusCode = this.hasNotFoundMatch() + ? 404 + : this.stores.matches.get().some((d) => d.status === 'error') + ? 500 + : undefined + if (newStatusCode) { this.stores.statusCode.set(newStatusCode) } } - startViewTransition = (fn: () => Promise) => { - // Determine if we should start a view transition from the navigation - // or from the router default + startViewTransition = (fn: () => Promise): Promise => { const shouldViewTransition = this.shouldViewTransition ?? this.options.defaultViewTransition - // Reset the view transition flag this.shouldViewTransition = undefined - // Attempt to start a view transition (or just apply the changes if we can't) if ( - shouldViewTransition && - typeof document !== 'undefined' && - 'startViewTransition' in document && - typeof document.startViewTransition === 'function' + !shouldViewTransition || + typeof document === 'undefined' || + typeof (document as any).startViewTransition !== 'function' ) { - // lib.dom.ts doesn't support viewTransition types variant yet. - // TODO: Fix this when dom types are updated - let startViewTransitionParams: any - - if ( - typeof shouldViewTransition === 'object' && - this.isViewTransitionTypesSupported - ) { - const next = this.latestLocation - const prevLocation = this.stores.resolvedLocation.get() - - const resolvedViewTransitionTypes = - typeof shouldViewTransition.types === 'function' - ? shouldViewTransition.types( - getLocationChangeInfo(next, prevLocation), - ) - : shouldViewTransition.types + return fn() + } - if (resolvedViewTransitionTypes === false) { - fn() - return - } + if ( + typeof shouldViewTransition === 'object' && + this.isViewTransitionTypesSupported + ) { + const types = + typeof shouldViewTransition.types === 'function' + ? shouldViewTransition.types( + getLocationChangeInfo( + this.latestLocation, + this.stores.resolvedLocation.get(), + ), + ) + : shouldViewTransition.types - startViewTransitionParams = { - update: fn, - types: resolvedViewTransitionTypes, - } - } else { - startViewTransitionParams = fn + if (types === false) { + return fn() } - document.startViewTransition(startViewTransitionParams) - } else { - fn() + return (document as any).startViewTransition({ update: fn, types }) + .updateCallbackDone } + + return (document as any).startViewTransition(fn).updateCallbackDone } updateMatch: UpdateMatchFn = (id, updater) => { - this.startTransition(() => { - const pendingMatch = this.stores.pendingMatchStores.get(id) - if (pendingMatch) { - pendingMatch.set(updater) - return - } - - const activeMatch = this.stores.matchStores.get(id) - if (activeMatch) { - activeMatch.set(updater) - return - } + const matchStore = + this.stores.pendingMatchStores.get(id) ?? this.stores.matchStores.get(id) + if (matchStore) { + matchStore.set(updater) + return + } - const cachedMatch = this.stores.cachedMatchStores.get(id) - if (cachedMatch) { - const next = updater(cachedMatch.get()) - if (next.status === 'redirected') { - const deleted = this.stores.cachedMatchStores.delete(id) - if (deleted) { - this.stores.cachedIds.set((prev) => - prev.filter((matchId) => matchId !== id), - ) - } - } else { - cachedMatch.set(next) - } + const cachedMatch = this.stores.cachedMatchStores.get(id) + if (cachedMatch) { + const match = cachedMatch.get() + const next = updater(match) + if (next.status !== 'success' && next.status !== 'pending') { + this.clearCache({ filter: (d) => d.id === id }) + } else { + cachedMatch.set(next) } - }) + } } - getMatch: GetMatchFn = (matchId: string): AnyRouteMatch | undefined => { + getMatch: GetMatchFn = (matchId, includeCached) => { + const matchStore = + this.stores.pendingMatchStores.get(matchId) ?? + this.stores.matchStores.get(matchId) return ( - this.stores.cachedMatchStores.get(matchId)?.get() ?? - this.stores.pendingMatchStores.get(matchId)?.get() ?? - this.stores.matchStores.get(matchId)?.get() - ) + includeCached === false + ? matchStore + : (matchStore ?? this.stores.cachedMatchStores.get(matchId)) + )?.get() } /** @@ -2803,21 +2743,25 @@ export class RouterCore< } resolveRedirect = (redirect: AnyRedirect): AnyRedirect => { - const locationHeader = redirect.headers.get('Location') + const options = redirect.options - if (!redirect.options.href || redirect.options._builtLocation) { - const location = - redirect.options._builtLocation ?? this.buildLocation(redirect.options) + if (!options.href || options._builtLocation) { + const location = options._builtLocation ?? this.buildLocation(options) const href = this.getParsedLocationHref(location) - redirect.options.href = href - redirect.headers.set('Location', href) - } else if (locationHeader) { + options.href = href + if (isServer ?? this.isServer) { + redirect.headers.set('Location', href) + } + } else if (isServer ?? this.isServer) { + const locationHeader = redirect.headers.get('Location') try { - const url = new URL(locationHeader) - if (this.origin && url.origin === this.origin) { - const href = url.pathname + url.search + url.hash - redirect.options.href = href - redirect.headers.set('Location', href) + if (locationHeader) { + const url = new URL(locationHeader) + if (url.origin === this.origin) { + const href = url.pathname + url.search + url.hash + options.href = href + redirect.headers.set('Location', href) + } } } catch { // ignore invalid URLs @@ -2825,20 +2769,20 @@ export class RouterCore< } if ( - redirect.options.href && - !redirect.options._builtLocation && + options.href && + !options._builtLocation && // Check for dangerous protocols before processing the redirect - isDangerousProtocol(redirect.options.href, this.protocolAllowlist) + isDangerousProtocol(options.href, this.protocolAllowlist) ) { throw new Error( process.env.NODE_ENV !== 'production' - ? `Redirect blocked: unsafe protocol in href "${redirect.options.href}". Allowed protocols: ${Array.from(this.protocolAllowlist).join(', ')}.` + ? `Redirect blocked: unsafe protocol in href "${options.href}". Allowed protocols: ${Array.from(this.protocolAllowlist).join(', ')}.` : 'Redirect blocked: unsafe protocol', ) } - if (!redirect.headers.get('Location')) { - redirect.headers.set('Location', redirect.options.href) + if ((isServer ?? this.isServer) && !redirect.headers.get('Location')) { + redirect.headers.set('Location', options.href) } return redirect @@ -2846,15 +2790,18 @@ export class RouterCore< clearCache: ClearCacheFn = (opts) => { const filter = opts?.filter - if (filter !== undefined) { - this.stores.setCached( - this.stores.cachedMatches - .get() - .filter((m) => !filter(m as MakeRouteMatchUnion)), - ) - } else { - this.stores.setCached([]) + const cachedMatches = this.stores.cachedMatches.get() + const nextCachedMatches: Array = [] + + for (const match of cachedMatches) { + if (!filter || filter(match as MakeRouteMatchUnion)) { + clearMatchPromises(match) + } else { + nextCachedMatches.push(match) + } } + + this.stores.setCached(nextCachedMatches) } clearExpiredCache = () => { @@ -2875,11 +2822,9 @@ export class RouterCore< : (route.options.gcTime ?? this.options.defaultGcTime)) ?? 5 * 60 * 1000 - const isError = d.status === 'error' - if (isError) return true + if (d.status === 'error') return true - const gcEligible = now - d.updatedAt >= gcTime - return gcEligible + return now - d.updatedAt >= gcTime } this.clearCache({ filter }) } @@ -2897,7 +2842,6 @@ export class RouterCore< let matches = this.matchRoutes(next, { throwOnError: true, preload: true, - dest: opts, }) const activeMatchIds = new Set([ @@ -2905,14 +2849,11 @@ export class RouterCore< ...this.stores.pendingIds.get(), ]) - const loadedMatchIds = new Set([ - ...activeMatchIds, - ...this.stores.cachedIds.get(), - ]) - // If the matches are already loaded, we need to add them to the cached matches. const matchesToCache = matches.filter( - (match) => !loadedMatchIds.has(match.id), + (match) => + !activeMatchIds.has(match.id) && + !this.stores.cachedMatchStores.has(match.id), ) if (matchesToCache.length) { const cachedMatches = this.stores.cachedMatches.get() @@ -2924,25 +2865,17 @@ export class RouterCore< router: this, matches, location: next, - preload: true, - updateMatch: (id, updater) => { - // Don't update the match if it's currently loaded - if (activeMatchIds.has(id)) { - matches = matches.map((d) => (d.id === id ? updater(d) : d)) - } else { - this.updateMatch(id, updater) - } - }, + preload: activeMatchIds, }) return matches } catch (err) { if (isRedirect(err)) { if (err.options.reloadDocument) { - return undefined + return } - return await this.preloadRoute({ + return this.preloadRoute({ ...err.options, _fromLocation: next, }) @@ -2951,7 +2884,7 @@ export class RouterCore< // Preload errors are not fatal, but we should still log them console.error(err) } - return undefined + return } } @@ -3124,28 +3057,14 @@ export function getMatchedRoutes({ return { matchedRoutes, routeParams, foundRoute } } -/** - * TODO: once caches are persisted across requests on the server, - * we can cache the built middleware chain using `last(destRoutes)` as the key - */ -function applySearchMiddleware({ - search, - dest, - destRoutes, - _includeValidateSearch, -}: { - search: any - dest: { search?: unknown } - destRoutes: ReadonlyArray - _includeValidateSearch: boolean | undefined -}) { - const middleware = buildMiddlewareChain(destRoutes) - return middleware(search, dest, _includeValidateSearch ?? false) -} - -function buildMiddlewareChain(destRoutes: ReadonlyArray) { - let dest: BuildNextOptions - let includeValidateSearch: boolean | undefined +// TODO: once caches are persisted across requests on the server, +// we can cache the built middleware chain using `last(destRoutes)` as the key +function buildMiddlewareChain( + destRoutes: ReadonlyArray, + search: any, + dest: BuildNextOptions, + includeValidateSearch: boolean, +) { const middlewares = [] as Array> for (const route of destRoutes) { @@ -3238,15 +3157,7 @@ function buildMiddlewareChain(destRoutes: ReadonlyArray) { return (middlewares[index]! as any)({ search: currentSearch, next, meta }) } - return function middleware( - search: any, - nextDest: BuildNextOptions, - _includeValidateSearch: boolean, - ) { - dest = nextDest - includeValidateSearch = _includeValidateSearch - return applyNext(0, search) - } + return applyNext(0, search) } function findGlobalNotFoundRouteId( diff --git a/packages/router-core/src/ssr/ssr-client.ts b/packages/router-core/src/ssr/ssr-client.ts index 2aa5358ac0..0d87a18e48 100644 --- a/packages/router-core/src/ssr/ssr-client.ts +++ b/packages/router-core/src/ssr/ssr-client.ts @@ -298,10 +298,10 @@ export async function hydrate(router: AnyRouter): Promise { router.stores.resolvedLocation.set(router.stores.location.get()) } // hide the pending component once the load is finished + match._nonReactive.displayPendingPromise = undefined router.updateMatch(match.id, (prev) => ({ ...prev, _displayPending: undefined, - displayPendingPromise: undefined, })) }) }) diff --git a/packages/router-core/src/utils.ts b/packages/router-core/src/utils.ts index f017215b5c..0e48cbe2b4 100644 --- a/packages/router-core/src/utils.ts +++ b/packages/router-core/src/utils.ts @@ -471,15 +471,19 @@ export function createControlledPromise(onResolve?: (value: T) => void) { controlledPromise.status = 'pending' controlledPromise.resolve = (value: T) => { - controlledPromise.status = 'resolved' - controlledPromise.value = value - resolveLoadPromise(value) - onResolve?.(value) + if (controlledPromise.status === 'pending') { + controlledPromise.status = 'resolved' + controlledPromise.value = value + resolveLoadPromise(value) + onResolve?.(value) + } } controlledPromise.reject = (e) => { - controlledPromise.status = 'rejected' - rejectLoadPromise(e) + if (controlledPromise.status === 'pending') { + controlledPromise.status = 'rejected' + rejectLoadPromise(e) + } } return controlledPromise diff --git a/packages/router-core/tests/granular-stores.test.ts b/packages/router-core/tests/granular-stores.test.ts index 54d2a4efab..18db398dca 100644 --- a/packages/router-core/tests/granular-stores.test.ts +++ b/packages/router-core/tests/granular-stores.test.ts @@ -349,6 +349,6 @@ describe('granular stores', () => { ).toBe('pending') expect(router.stores.pendingMatches.get()[0]?.status).toBe('pending') expect(router.stores.cachedMatches.get()[0]?.status).toBe('success') - expect(router.getMatch(duplicatedId)?.status).toBe('success') + expect(router.getMatch(duplicatedId)?.status).toBe('pending') }) }) diff --git a/packages/router-core/tests/hydrate.test.ts b/packages/router-core/tests/hydrate.test.ts index efac230e49..41e23774f2 100644 --- a/packages/router-core/tests/hydrate.test.ts +++ b/packages/router-core/tests/hydrate.test.ts @@ -517,4 +517,167 @@ describe('hydrate', () => { consoleSpy.mockRestore() }) + + it('should clear SPA displayPendingPromise when load finishes', async () => { + let resolveLoad!: () => void + const loadPromise = new Promise((resolve) => { + resolveLoad = resolve + }) + vi.spyOn(mockRouter, 'load').mockReturnValue(loadPromise) + + const matches = mockRouter.matchRoutes(mockRouter.stores.location.get()) + mockRouter.matchRoutes = vi.fn().mockReturnValue(matches) + + mockWindow.$_TSR = { + router: { + manifest: testManifest, + dehydratedData: {}, + lastMatchId: '/not-the-current-leaf', + matches: [ + { + i: matches[0].id, + s: 'success', + ssr: true, + u: Date.now(), + }, + ], + }, + h: vi.fn(), + e: vi.fn(), + c: vi.fn(), + p: vi.fn(), + buffer: [], + initialized: false, + } + + await hydrate(mockRouter) + await Promise.resolve() + + const match = mockRouter.stores.matches.get()[1] as AnyRouteMatch + const displayPendingPromise = match._nonReactive.displayPendingPromise + + expect(match._displayPending).toBe(true) + expect(displayPendingPromise).toBeDefined() + + resolveLoad() + await displayPendingPromise + await Promise.resolve() + + const updatedMatch = mockRouter.getMatch(match.id) as AnyRouteMatch + expect(updatedMatch._displayPending).toBeUndefined() + expect(updatedMatch._nonReactive.displayPendingPromise).toBeUndefined() + }) + + it('should clear SPA displayPendingPromise when load rejects after match exits', async () => { + let rejectLoad!: (err: unknown) => void + const loadPromise = new Promise((_resolve, reject) => { + rejectLoad = reject + }) + vi.spyOn(mockRouter, 'load').mockReturnValue(loadPromise) + const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + + const matches = mockRouter.matchRoutes(mockRouter.stores.location.get()) + mockRouter.matchRoutes = vi.fn().mockReturnValue(matches) + + mockWindow.$_TSR = { + router: { + manifest: testManifest, + dehydratedData: {}, + lastMatchId: '/not-the-current-leaf', + matches: [ + { + i: matches[0].id, + s: 'success', + ssr: true, + u: Date.now(), + }, + ], + }, + h: vi.fn(), + e: vi.fn(), + c: vi.fn(), + p: vi.fn(), + buffer: [], + initialized: false, + } + + await hydrate(mockRouter) + await Promise.resolve() + + const match = mockRouter.stores.matches.get()[1] as AnyRouteMatch + const displayPendingPromise = match._nonReactive.displayPendingPromise + + expect(match._displayPending).toBe(true) + expect(displayPendingPromise).toBeDefined() + + mockRouter.stores.setMatches([mockRouter.stores.matches.get()[0]!]) + rejectLoad(new Error('load failed')) + await displayPendingPromise + await Promise.resolve() + + expect(match._nonReactive.displayPendingPromise).toBeUndefined() + expect(consoleSpy).toHaveBeenCalledWith( + 'Error during router hydration:', + expect.any(Error), + ) + + consoleSpy.mockRestore() + }) + + it('should clear current SPA displayPendingPromise when load rejects', async () => { + let rejectLoad!: (err: unknown) => void + const loadPromise = new Promise((_resolve, reject) => { + rejectLoad = reject + }) + vi.spyOn(mockRouter, 'load').mockReturnValue(loadPromise) + const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + + const matches = mockRouter.matchRoutes(mockRouter.stores.location.get()) + mockRouter.matchRoutes = vi.fn().mockReturnValue(matches) + + mockWindow.$_TSR = { + router: { + manifest: testManifest, + dehydratedData: {}, + lastMatchId: '/not-the-current-leaf', + matches: [ + { + i: matches[0].id, + s: 'success', + ssr: true, + u: Date.now(), + }, + ], + }, + h: vi.fn(), + e: vi.fn(), + c: vi.fn(), + p: vi.fn(), + buffer: [], + initialized: false, + } + + await hydrate(mockRouter) + await Promise.resolve() + + const match = mockRouter.stores.matches.get()[1] as AnyRouteMatch + const displayPendingPromise = match._nonReactive.displayPendingPromise + + expect(match._displayPending).toBe(true) + expect(displayPendingPromise).toBeDefined() + + rejectLoad(new Error('load failed')) + await displayPendingPromise + await Promise.resolve() + + const updatedMatch = mockRouter.getMatch(match.id) as AnyRouteMatch + expect(updatedMatch._displayPending).toBeUndefined() + expect(updatedMatch._nonReactive.displayPendingPromise).toBeUndefined() + expect(consoleSpy).toHaveBeenCalledWith( + 'Error during router hydration:', + expect.any(Error), + ) + + consoleSpy.mockRestore() + }) }) diff --git a/packages/router-core/tests/load.test.ts b/packages/router-core/tests/load.test.ts index 1ea6fca30e..482f0037e8 100644 --- a/packages/router-core/tests/load.test.ts +++ b/packages/router-core/tests/load.test.ts @@ -3,12 +3,13 @@ import { createMemoryHistory } from '@tanstack/history' import { BaseRootRoute, BaseRoute, + createControlledPromise, notFound, redirect, rootRouteId, } from '../src' import { createTestRouter } from './routerTestUtils' -import { loadMatches } from '../src/load-matches' +import { loadMatches, loadRouteChunk } from '../src/load-matches' import type { AnyRouter, LoaderStaleReloadMode, @@ -20,9 +21,10 @@ type AnyRouteOptions = RootRouteOptions type BeforeLoad = NonNullable type Loader = NonNullable type LoaderEntry = Exclude +type LoaderFn = Exclude describe('redirect resolution', () => { - test('resolveRedirect normalizes same-origin Location to path-only', async () => { + test('resolveRedirect normalizes same-origin Location to path-only on the server', async () => { const rootRoute = new BaseRootRoute({}) const fooRoute = new BaseRoute({ getParentRoute: () => rootRoute, @@ -37,6 +39,7 @@ describe('redirect resolution', () => { initialEntries: ['https://example.com/foo'], }), origin: 'https://example.com', + isServer: true, }) // This redirect already includes an absolute Location header (external-ish), @@ -53,6 +56,57 @@ describe('redirect resolution', () => { expect(resolved.options.href).toBe('/foo') }) + test('resolveRedirect does not rewrite Location on the client', async () => { + const rootRoute = new BaseRootRoute({}) + const fooRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/foo', + }) + + const routeTree = rootRoute.addChildren([fooRoute]) + + const router = createTestRouter({ + routeTree, + history: createMemoryHistory({ + initialEntries: ['https://example.com/foo'], + }), + origin: 'https://example.com', + isServer: false, + }) + + const unresolved = redirect({ + to: '/foo', + headers: { Location: 'https://example.com/foo' }, + }) + + const resolved = router.resolveRedirect(unresolved) + + expect(resolved.headers.get('Location')).toBe('https://example.com/foo') + expect(resolved.options.href).toBe('/foo') + }) + + test('resolveRedirect does not add Location on the client', async () => { + const rootRoute = new BaseRootRoute({}) + const fooRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/foo', + }) + + const routeTree = rootRoute.addChildren([fooRoute]) + + const router = createTestRouter({ + routeTree, + history: createMemoryHistory({ initialEntries: ['/foo'] }), + isServer: false, + }) + + const unresolved = redirect({ to: '/foo' }) + const resolved = router.resolveRedirect(unresolved) + + expect(resolved.headers.get('Location')).toBe(null) + expect(resolved.options.href).toBe('/foo') + }) + test.each(['/$a', '/$toString', '/$__proto__'])( 'server startup redirects initial path %s to /undefined', async (initialPath) => { @@ -209,6 +263,100 @@ describe('beforeLoad skip or exec', () => { expect(thrown).toEqual({ type: 'domain-error' }) }) + test.each([false, true])( + 'handles %s async returned redirects from beforeLoad', + async (asyncReturn) => { + const beforeLoad = vi.fn(() => { + const result = redirect({ to: '/bar' }) + return asyncReturn ? Promise.resolve(result) : result + }) + const router = setup({ beforeLoad }) + + await router.navigate({ to: '/foo' }) + + expect(router.state.location.pathname).toBe('/bar') + expect(beforeLoad).toHaveBeenCalledTimes(1) + }, + ) + + test.each([false, true])( + 'handles %s async returned notFounds from beforeLoad', + async (asyncReturn) => { + const loader = vi.fn() + const rootRoute = new BaseRootRoute({}) + const fooRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/foo', + beforeLoad: () => { + const result = notFound() + return asyncReturn ? Promise.resolve(result) : result + }, + loader, + notFoundComponent: () => null, + }) + + const routeTree = rootRoute.addChildren([fooRoute]) + const router = createTestRouter({ + routeTree, + history: createMemoryHistory(), + }) + + await router.navigate({ to: '/foo' }) + + const match = router.state.matches.find((m) => m.routeId === fooRoute.id) + expect(match?.status).toBe('notFound') + expect(router.state.statusCode).toBe(404) + expect(loader).not.toHaveBeenCalled() + }, + ) + + test.each([false, true])( + 'exec if %s async returned preload redirect from beforeLoad', + async (asyncReturn) => { + const beforeLoad = vi.fn(({ preload }) => { + if (preload) { + const result = redirect({ to: '/bar' }) + return asyncReturn ? Promise.resolve(result) : result + } + return undefined + }) + const router = setup({ beforeLoad }) + + await router.preloadRoute({ to: '/foo' }) + expect( + router.stores.cachedMatches.get().some((d) => d.id === '/foo/foo'), + ).toBe(false) + + await router.navigate({ to: '/foo' }) + + expect(router.state.location.pathname).toBe('/foo') + expect(beforeLoad).toHaveBeenCalledTimes(2) + }, + ) + + test.each([false, true])( + 'exec if %s async returned preload notFound from beforeLoad', + async (asyncReturn) => { + const beforeLoad = vi.fn(({ preload }) => { + if (preload) { + const result = notFound() + return asyncReturn ? Promise.resolve(result) : result + } + return undefined + }) + const router = setup({ beforeLoad }) + + await router.preloadRoute({ to: '/foo' }) + expect( + router.stores.cachedMatches.get().some((d) => d.id === '/foo/foo'), + ).toBe(false) + await router.navigate({ to: '/foo' }) + + expect(router.state.location.pathname).toBe('/foo') + expect(beforeLoad).toHaveBeenCalledTimes(2) + }, + ) + test('exec if resolved preload (success)', async () => { const beforeLoad = vi.fn() const router = setup({ beforeLoad }) @@ -275,14 +423,14 @@ describe('beforeLoad skip or exec', () => { }) await router.preloadRoute({ to: '/foo' }) expect( - router.stores.cachedMatches.get().some((d) => d.status === 'redirected'), + router.stores.cachedMatches.get().some((d) => d.id === '/foo/foo'), ).toBe(false) await sleep(10) await router.navigate({ to: '/foo' }) expect(router.state.location.pathname).toBe('/foo') expect( - router.stores.cachedMatches.get().some((d) => d.status === 'redirected'), + router.stores.cachedMatches.get().some((d) => d.id === '/foo/foo'), ).toBe(false) expect(beforeLoad).toHaveBeenCalledTimes(2) }) @@ -297,14 +445,11 @@ describe('beforeLoad skip or exec', () => { }) router.preloadRoute({ to: '/foo' }) await Promise.resolve() - expect( - router.stores.cachedMatches.get().some((d) => d.status === 'redirected'), - ).toBe(false) await router.navigate({ to: '/foo' }) expect(router.state.location.pathname).toBe('/foo') expect( - router.stores.cachedMatches.get().some((d) => d.status === 'redirected'), + router.stores.cachedMatches.get().some((d) => d.id === '/foo/foo'), ).toBe(false) expect(beforeLoad).toHaveBeenCalledTimes(2) }) @@ -359,86 +504,1328 @@ describe('beforeLoad skip or exec', () => { expect(childHead).not.toHaveBeenCalled() }) - test('exec if pending preload (error)', async () => { - const beforeLoad = vi.fn(async ({ preload }) => { - await sleep(100) - if (preload) throw new Error('error') + test('preload descendant waits for active parent beforeLoad context', async () => { + const parentBeforeLoadPromise = createControlledPromise<{ auth: string }>() + const parentBeforeLoad = vi.fn(() => parentBeforeLoadPromise) + const childLoader = vi.fn(({ context }) => context) + + const rootRoute = new BaseRootRoute({}) + const parentRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/parent', + beforeLoad: parentBeforeLoad, }) - const router = setup({ - beforeLoad, + const childRoute = new BaseRoute({ + getParentRoute: () => parentRoute, + path: '/child', + loader: childLoader, }) - router.preloadRoute({ to: '/foo' }) + + const router = createTestRouter({ + routeTree: rootRoute.addChildren([parentRoute.addChildren([childRoute])]), + history: createMemoryHistory(), + }) + + const navigation = router.navigate({ to: '/parent' }) await Promise.resolve() - await router.navigate({ to: '/foo' }) + expect(parentBeforeLoad).toHaveBeenCalledTimes(1) - expect(beforeLoad).toHaveBeenCalledTimes(2) + const preload = router.preloadRoute({ to: '/parent/child' }) + await Promise.resolve() + expect(childLoader).not.toHaveBeenCalled() + + parentBeforeLoadPromise.resolve({ auth: 'ok' }) + await navigation + await preload + + expect(childLoader).toHaveBeenCalledTimes(1) + expect(childLoader.mock.calls[0]?.[0].context).toMatchObject({ + auth: 'ok', + }) }) -}) -describe('loader skip or exec', () => { - const setup = ({ - loader, - staleTime, - defaultStaleReloadMode, - }: { - loader?: Loader - staleTime?: number - defaultStaleReloadMode?: LoaderStaleReloadMode - }) => { - const rootRoute = new BaseRootRoute({}) + test('preload does not continue loader-owned descendants when joined active beforeLoad owner exits before settling', async () => { + vi.useFakeTimers() + const consoleError = vi + .spyOn(console, 'error') + .mockImplementation(() => undefined) + + try { + const parentBeforeLoadPromise = createControlledPromise<{ + auth: string + }>() + const parentBeforeLoad = vi.fn(() => parentBeforeLoadPromise) + const childBeforeLoad = vi.fn() + const childLoader = vi.fn(() => undefined) - const fooRoute = new BaseRoute({ + const rootRoute = new BaseRootRoute({}) + const indexRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/', + }) + const parentRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/parent', + beforeLoad: parentBeforeLoad, + pendingMs: 1, + pendingComponent: {}, + }) + const childRoute = new BaseRoute({ + getParentRoute: () => parentRoute, + path: '/child', + beforeLoad: childBeforeLoad, + loader: childLoader, + }) + const otherRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/other', + }) + + const router = createTestRouter({ + routeTree: rootRoute.addChildren([ + indexRoute, + parentRoute.addChildren([childRoute]), + otherRoute, + ]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + + await router.load() + + const parentNavigation = router.navigate({ to: '/parent' }) + await vi.waitFor(() => expect(parentBeforeLoad).toHaveBeenCalledTimes(1)) + + await vi.advanceTimersByTimeAsync(1) + await vi.waitFor(() => + expect( + router.state.matches.some( + (match) => + match.routeId === parentRoute.id && match.status === 'pending', + ), + ).toBe(true), + ) + + const preload = router.preloadRoute({ to: '/parent/child' }) + await Promise.resolve() + expect(childBeforeLoad).not.toHaveBeenCalled() + + const childCachedMatch = router.stores.cachedMatches + .get() + .find((match) => match.routeId === childRoute.id)! + const childLoadPromise = childCachedMatch._nonReactive.loadPromise + expect(childLoadPromise?.status).toBe('pending') + + await router.navigate({ to: '/other' }) + + parentBeforeLoadPromise.resolve({ auth: 'late' }) + await Promise.all([parentNavigation, preload]) + + expect(router.state.location.pathname).toBe('/other') + expect(childBeforeLoad).not.toHaveBeenCalled() + expect(childLoader).not.toHaveBeenCalled() + expect( + router.stores.cachedMatches + .get() + .some((match) => match.routeId === childRoute.id), + ).toBe(false) + expect(childLoadPromise?.status).toBe('resolved') + } finally { + consoleError.mockRestore() + vi.useRealTimers() + } + }) + + test('beforeLoad error commits only the renderable match prefix', async () => { + const parentHead = vi.fn(({ match }) => ({ + meta: [{ title: match.error ? 'Parent error' : 'Parent success' }], + })) + const childHead = vi.fn(() => ({ + meta: [{ title: 'Child success' }], + })) + + const rootRoute = new BaseRootRoute({}) + const parentRoute = new BaseRoute({ getParentRoute: () => rootRoute, - path: '/foo', - loader, - staleTime, - gcTime: staleTime, + path: '/parent', + validateSearch: (search: Record) => ({ + fail: search.fail === true || search.fail === 'true', + }), + beforeLoad: ({ search }) => { + if (search.fail) { + throw new Error('Parent beforeLoad failed') + } + }, + head: parentHead, + }) + const childRoute = new BaseRoute({ + getParentRoute: () => parentRoute, + path: '/child', + head: childHead, }) - const barRoute = new BaseRoute({ - getParentRoute: () => rootRoute, - path: '/bar', + const router = createTestRouter({ + routeTree: rootRoute.addChildren([parentRoute.addChildren([childRoute])]), + history: createMemoryHistory({ + initialEntries: ['/parent/child?fail=false'], + }), }) - const routeTree = rootRoute.addChildren([fooRoute, barRoute]) + await router.load() - const router = createTestRouter({ - routeTree, - defaultStaleReloadMode, - history: createMemoryHistory(), + expect(router.state.matches.map((match) => match.routeId)).toContain( + childRoute.id, + ) + expect(childHead).toHaveBeenCalledTimes(1) + + await router.navigate({ + to: '/parent/child', + search: { fail: true }, + } as never) + + expect(router.state.matches.map((match) => match.routeId)).toEqual([ + rootRoute.id, + parentRoute.id, + ]) + expect( + router.state.matches.find((match) => match.routeId === parentRoute.id) + ?.status, + ).toBe('error') + expect(parentHead).toHaveBeenCalledTimes(2) + expect(childHead).toHaveBeenCalledTimes(1) + }) + + test('loader error commits only the renderable match prefix', async () => { + const parentHead = vi.fn(({ match }) => ({ + meta: [{ title: match.error ? 'Parent error' : 'Parent success' }], + })) + const childHead = vi.fn(() => ({ + meta: [{ title: 'Child success' }], + })) + + const rootRoute = new BaseRootRoute({}) + const parentRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/parent', + validateSearch: (search: Record) => ({ + fail: search.fail === true || search.fail === 'true', + }), + loaderDeps: ({ search }) => ({ fail: search.fail }), + loader: (({ deps }) => { + if ((deps as { fail?: boolean }).fail) { + throw new Error('Parent loader failed') + } + }) as LoaderFn, + head: parentHead, + }) + const childRoute = new BaseRoute({ + getParentRoute: () => parentRoute, + path: '/child', + head: childHead, }) - return router - } + const router = createTestRouter({ + routeTree: rootRoute.addChildren([parentRoute.addChildren([childRoute])]), + history: createMemoryHistory({ + initialEntries: ['/parent/child?fail=false'], + }), + }) - test('baseline', async () => { - const loader = vi.fn() - const router = setup({ loader }) await router.load() - expect(loader).toHaveBeenCalledTimes(0) + + expect(router.state.matches.map((match) => match.routeId)).toContain( + childRoute.id, + ) + expect(childHead).toHaveBeenCalledTimes(1) + + await router.navigate({ + to: '/parent/child', + search: { fail: true }, + } as never) + + expect(router.state.matches.map((match) => match.routeId)).toEqual([ + rootRoute.id, + parentRoute.id, + ]) + expect( + router.state.matches.find((match) => match.routeId === parentRoute.id) + ?.status, + ).toBe('error') + expect(parentHead).toHaveBeenCalledTimes(2) + expect(childHead).toHaveBeenCalledTimes(1) }) - test('exec on regular nav', async () => { - const loader = vi.fn(() => Promise.resolve({ hello: 'world' })) - const router = setup({ loader }) - const navigation = router.navigate({ to: '/foo' }) - expect(loader).toHaveBeenCalledTimes(1) - expect(router.stores.pendingMatches.get()).toEqual( - expect.arrayContaining([expect.objectContaining({ id: '/foo/foo' })]), + test('preload from onBeforeLoad waits for active root beforeLoad context', async () => { + vi.useFakeTimers() + + try { + const rootBeforeLoadPromise = createControlledPromise<{ auth: string }>() + const rootBeforeLoad = vi.fn(() => rootBeforeLoadPromise) + const childLoader = vi.fn(({ context }) => context) + + const rootRoute = new BaseRootRoute({ + beforeLoad: rootBeforeLoad, + }) + const parentRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/parent', + }) + const childRoute = new BaseRoute({ + getParentRoute: () => parentRoute, + path: '/child', + loader: childLoader, + }) + + const router = createTestRouter({ + routeTree: rootRoute.addChildren([ + parentRoute.addChildren([childRoute]), + ]), + history: createMemoryHistory(), + }) + + let preload: ReturnType | undefined + const unsubscribe = router.subscribe('onBeforeLoad', (event) => { + if (!preload && event.toLocation.pathname === '/parent') { + preload = router.preloadRoute({ to: '/parent/child' }) + } + }) + + try { + const navigation = router.navigate({ to: '/parent' }) + await vi.advanceTimersByTimeAsync(0) + + expect(rootBeforeLoad).toHaveBeenCalledTimes(1) + expect(childLoader).not.toHaveBeenCalled() + + rootBeforeLoadPromise.resolve({ auth: 'ok' }) + await navigation + await preload + + expect(childLoader).toHaveBeenCalledTimes(1) + expect(childLoader.mock.calls[0]?.[0].context).toMatchObject({ + auth: 'ok', + }) + } finally { + unsubscribe() + } + } finally { + vi.useRealTimers() + } + }) + + test('preload descendant waits for active parent loader data', async () => { + vi.useFakeTimers() + + try { + const parentLoaderPromise = createControlledPromise<{ auth: string }>() + const unexpectedParentPreloadPromise = createControlledPromise<{ + auth: string + }>() + const parentLoader = vi.fn(({ preload }) => { + return preload ? unexpectedParentPreloadPromise : parentLoaderPromise + }) + let childLoaderSettled = false + const childLoader = vi.fn(async ({ parentMatchPromise }) => { + const parentMatch = (await parentMatchPromise) as any + childLoaderSettled = true + return parentMatch.loaderData + }) + + const rootRoute = new BaseRootRoute({}) + const parentRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/parent', + loader: parentLoader, + }) + const childRoute = new BaseRoute({ + getParentRoute: () => parentRoute, + path: '/child', + loader: childLoader, + }) + + const router = createTestRouter({ + routeTree: rootRoute.addChildren([ + parentRoute.addChildren([childRoute]), + ]), + history: createMemoryHistory(), + }) + + const navigation = router.navigate({ to: '/parent' }) + await vi.waitFor(() => expect(parentLoader).toHaveBeenCalledTimes(1)) + + const preload = router.preloadRoute({ to: '/parent/child' }) + await vi.advanceTimersByTimeAsync(5) + expect(parentLoader).toHaveBeenCalledTimes(1) + expect(childLoader).toHaveBeenCalledTimes(1) + expect(childLoaderSettled).toBe(false) + + parentLoaderPromise.resolve({ auth: 'ok' }) + await navigation + await preload + + expect(parentLoader).toHaveBeenCalledTimes(1) + expect(childLoaderSettled).toBe(true) + await expect(childLoader.mock.results[0]!.value).resolves.toEqual({ + auth: 'ok', + }) + } finally { + vi.useRealTimers() + } + }) + + test('preload does not settle descendant loader when joined active loader owner exits before settling', async () => { + vi.useFakeTimers() + + try { + const parentLoaderPromise = createControlledPromise<{ auth: string }>() + const parentLoader = vi.fn(() => parentLoaderPromise) + let childLoaderSettled = false + const childLoader = vi.fn(async ({ parentMatchPromise }) => { + await parentMatchPromise + childLoaderSettled = true + }) + const childOnError = vi.fn() + + const rootRoute = new BaseRootRoute({}) + const indexRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/', + }) + const parentRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/parent', + loader: parentLoader, + pendingMs: 1, + pendingComponent: {}, + }) + const childRoute = new BaseRoute({ + getParentRoute: () => parentRoute, + path: '/child', + loader: childLoader, + onError: childOnError, + }) + const otherRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/other', + }) + + const router = createTestRouter({ + routeTree: rootRoute.addChildren([ + indexRoute, + parentRoute.addChildren([childRoute]), + otherRoute, + ]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + + await router.load() + + const parentNavigation = router.navigate({ to: '/parent' }) + await vi.waitFor(() => expect(parentLoader).toHaveBeenCalledTimes(1)) + + const preload = router.preloadRoute({ to: '/parent/child' }) + await vi.advanceTimersByTimeAsync(5) + expect(parentLoader).toHaveBeenCalledTimes(1) + expect(childLoader).toHaveBeenCalledTimes(1) + expect(childLoaderSettled).toBe(false) + + const childCachedMatch = router.stores.cachedMatches + .get() + .find((match) => match.routeId === childRoute.id)! + const childLoadPromise = childCachedMatch._nonReactive.loadPromise + const childLoaderPromise = childCachedMatch._nonReactive.loaderPromise + expect(childLoadPromise?.status).toBe('pending') + expect(childLoaderPromise?.status).toBe('pending') + + await router.navigate({ to: '/other' }) + + parentLoaderPromise.resolve({ auth: 'late' }) + await Promise.all([parentNavigation, preload]) + + expect(router.state.location.pathname).toBe('/other') + expect(childLoaderSettled).toBe(false) + expect(childOnError).not.toHaveBeenCalled() + expect( + router.stores.cachedMatches + .get() + .some((match) => match.routeId === childRoute.id), + ).toBe(false) + expect(childLoadPromise?.status).toBe('resolved') + expect(childLoaderPromise?.status).toBe('resolved') + } finally { + vi.useRealTimers() + } + }) + + test('preload clears independently completed descendant when joined active loader owner exits', async () => { + vi.useFakeTimers() + + try { + const parentLoaderPromise = createControlledPromise<{ auth: string }>() + const parentLoader = vi.fn(() => parentLoaderPromise) + const childLoader = vi.fn(() => ({ child: 'preloaded' })) + + const rootRoute = new BaseRootRoute({}) + const indexRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/', + }) + const parentRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/parent', + loader: parentLoader, + pendingMs: 1, + pendingComponent: {}, + }) + const childRoute = new BaseRoute({ + getParentRoute: () => parentRoute, + path: '/child', + loader: childLoader, + }) + const otherRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/other', + }) + + const router = createTestRouter({ + routeTree: rootRoute.addChildren([ + indexRoute, + parentRoute.addChildren([childRoute]), + otherRoute, + ]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + + await router.load() + + const parentNavigation = router.navigate({ to: '/parent' }) + await vi.waitFor(() => expect(parentLoader).toHaveBeenCalledTimes(1)) + + const preload = router.preloadRoute({ to: '/parent/child' }) + await vi.waitFor(() => expect(childLoader).toHaveBeenCalledTimes(1)) + await vi.waitFor(() => + expect( + router.stores.cachedMatches + .get() + .some( + (match) => + match.routeId === childRoute.id && match.status === 'success', + ), + ).toBe(true), + ) + + await router.navigate({ to: '/other' }) + + parentLoaderPromise.resolve({ auth: 'late' }) + await Promise.all([parentNavigation, preload]) + + expect(router.state.location.pathname).toBe('/other') + expect( + router.stores.cachedMatches + .get() + .some((match) => match.routeId === childRoute.id), + ).toBe(false) + } finally { + vi.useRealTimers() + } + }) + + test('preload resolves when joined active loader owner exits with a never-settling descendant loader', async () => { + vi.useFakeTimers() + + try { + const parentLoaderPromise = createControlledPromise<{ auth: string }>() + const childLoaderPromise = createControlledPromise() + const parentLoader = vi.fn(() => parentLoaderPromise) + const childLoader = vi.fn(() => childLoaderPromise) + + const rootRoute = new BaseRootRoute({}) + const indexRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/', + }) + const parentRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/parent', + loader: parentLoader, + pendingMs: 1, + pendingComponent: {}, + }) + const childRoute = new BaseRoute({ + getParentRoute: () => parentRoute, + path: '/child', + loader: childLoader, + }) + const otherRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/other', + }) + + const router = createTestRouter({ + routeTree: rootRoute.addChildren([ + indexRoute, + parentRoute.addChildren([childRoute]), + otherRoute, + ]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + + await router.load() + + const parentNavigation = router.navigate({ to: '/parent' }) + await vi.waitFor(() => expect(parentLoader).toHaveBeenCalledTimes(1)) + + const preloadSettled = vi.fn() + const preload = router.preloadRoute({ to: '/parent/child' }) + preload.then(preloadSettled) + await vi.waitFor(() => expect(childLoader).toHaveBeenCalledTimes(1)) + + await router.navigate({ to: '/other' }) + + parentLoaderPromise.resolve({ auth: 'late' }) + await parentNavigation + await Promise.resolve() + await Promise.resolve() + + expect(preloadSettled).toHaveBeenCalledTimes(1) + expect( + router.stores.cachedMatches + .get() + .some((match) => match.routeId === childRoute.id), + ).toBe(false) + } finally { + vi.useRealTimers() + } + }) + + test('preload resolves when joined active loader owner exits with a never-settling descendant beforeLoad', async () => { + vi.useFakeTimers() + + try { + const parentLoaderPromise = createControlledPromise<{ auth: string }>() + const childBeforeLoadPromise = createControlledPromise() + const parentLoader = vi.fn(() => parentLoaderPromise) + const childBeforeLoad = vi.fn(() => childBeforeLoadPromise) + + const rootRoute = new BaseRootRoute({}) + const indexRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/', + }) + const parentRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/parent', + loader: parentLoader, + pendingMs: 1, + pendingComponent: {}, + }) + const childRoute = new BaseRoute({ + getParentRoute: () => parentRoute, + path: '/child', + beforeLoad: childBeforeLoad, + }) + const otherRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/other', + }) + + const router = createTestRouter({ + routeTree: rootRoute.addChildren([ + indexRoute, + parentRoute.addChildren([childRoute]), + otherRoute, + ]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + + await router.load() + + const parentNavigation = router.navigate({ to: '/parent' }) + await vi.waitFor(() => expect(parentLoader).toHaveBeenCalledTimes(1)) + + const preloadSettled = vi.fn() + const preload = router.preloadRoute({ to: '/parent/child' }) + preload.then(preloadSettled) + await vi.waitFor(() => expect(childBeforeLoad).toHaveBeenCalledTimes(1)) + + await router.navigate({ to: '/other' }) + await vi.waitFor(() => expect(preloadSettled).toHaveBeenCalledTimes(1)) + + expect( + router.stores.cachedMatches + .get() + .some((match) => match.routeId === childRoute.id), + ).toBe(false) + + parentLoaderPromise.resolve({ auth: 'late' }) + childBeforeLoadPromise.resolve() + await Promise.all([parentNavigation, preload]) + } finally { + vi.useRealTimers() + } + }) + + test.each([ + { + name: 'without a never-settling descendant', + withNeverSettlingDescendant: false, + }, + { + name: 'with a never-settling descendant', + withNeverSettlingDescendant: true, + }, + ])( + 'preload cancellation wins after earlier redirect rejection $name', + async ({ withNeverSettlingDescendant }) => { + vi.useFakeTimers() + const consoleError = vi + .spyOn(console, 'error') + .mockImplementation(() => undefined) + + try { + const parentLoaderPromise = createControlledPromise<{ auth: string }>() + const hangLoaderPromise = createControlledPromise() + const parentLoader = vi.fn(() => parentLoaderPromise) + const childLoader = vi.fn(() => { + throw redirect({ to: '/target' }) + }) + const hangLoader = vi.fn(() => hangLoaderPromise) + const targetLoader = vi.fn(() => undefined) + + const rootRoute = new BaseRootRoute({}) + const indexRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/', + }) + const parentRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/parent', + loader: parentLoader, + pendingMs: 1, + pendingComponent: {}, + }) + const childRoute = new BaseRoute({ + getParentRoute: () => parentRoute, + path: '/child', + loader: childLoader, + }) + const hangRoute = new BaseRoute({ + getParentRoute: () => childRoute, + path: '/hang', + loader: hangLoader, + }) + const targetRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/target', + loader: targetLoader, + }) + const otherRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/other', + }) + + const router = createTestRouter({ + routeTree: rootRoute.addChildren([ + indexRoute, + parentRoute.addChildren([childRoute.addChildren([hangRoute])]), + targetRoute, + otherRoute, + ]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + + await router.load() + + const parentNavigation = router.navigate({ to: '/parent' }) + await vi.waitFor(() => expect(parentLoader).toHaveBeenCalledTimes(1)) + + const preloadSettled = vi.fn() + const preload = router.preloadRoute({ + to: withNeverSettlingDescendant + ? '/parent/child/hang' + : '/parent/child', + }) + preload.then(preloadSettled) + + await vi.waitFor(() => expect(childLoader).toHaveBeenCalledTimes(1)) + if (withNeverSettlingDescendant) { + await vi.waitFor(() => expect(hangLoader).toHaveBeenCalledTimes(1)) + } + + await router.navigate({ to: '/other' }) + + parentLoaderPromise.resolve({ auth: 'late' }) + await parentNavigation + await vi.waitFor(() => expect(preloadSettled).toHaveBeenCalledTimes(1)) + + expect(router.state.location.pathname).toBe('/other') + expect(targetLoader).not.toHaveBeenCalled() + expect(consoleError).not.toHaveBeenCalled() + } finally { + consoleError.mockRestore() + vi.useRealTimers() + } + }, + ) + + test('preload from onBeforeLoad waits for active parent loader data', async () => { + vi.useFakeTimers() + + try { + const parentLoaderPromise = createControlledPromise<{ auth: string }>() + const unexpectedParentPreloadPromise = createControlledPromise<{ + auth: string + }>() + const parentLoader = vi.fn(({ preload }) => { + return preload ? unexpectedParentPreloadPromise : parentLoaderPromise + }) + let childLoaderSettled = false + const childLoader = vi.fn(async ({ parentMatchPromise }) => { + const parentMatch = (await parentMatchPromise) as any + childLoaderSettled = true + return parentMatch.loaderData + }) + + const rootRoute = new BaseRootRoute({}) + const parentRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/parent', + loader: parentLoader, + }) + const childRoute = new BaseRoute({ + getParentRoute: () => parentRoute, + path: '/child', + loader: childLoader, + }) + + const router = createTestRouter({ + routeTree: rootRoute.addChildren([ + parentRoute.addChildren([childRoute]), + ]), + history: createMemoryHistory(), + }) + + let preload: ReturnType | undefined + const unsubscribe = router.subscribe('onBeforeLoad', (event) => { + if (!preload && event.toLocation.pathname === '/parent') { + preload = router.preloadRoute({ to: '/parent/child' }) + } + }) + + try { + const navigation = router.navigate({ to: '/parent' }) + await vi.advanceTimersByTimeAsync(5) + + expect(parentLoader).toHaveBeenCalledTimes(1) + expect(childLoader).toHaveBeenCalledTimes(1) + expect(childLoaderSettled).toBe(false) + + parentLoaderPromise.resolve({ auth: 'ok' }) + await navigation + await preload + + expect(parentLoader).toHaveBeenCalledTimes(1) + expect(childLoaderSettled).toBe(true) + await expect(childLoader.mock.results[0]!.value).resolves.toEqual({ + auth: 'ok', + }) + } finally { + unsubscribe() + } + } finally { + vi.useRealTimers() + } + }) + + test('executes head when loader throws notFound during preload', async () => { + const loader = vi.fn(({ preload }) => { + if (preload) { + throw notFound() + } + }) + const head = vi.fn(() => ({ meta: [{ title: 'Foo' }] })) + + const rootRoute = new BaseRootRoute({}) + const fooRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/foo', + loader, + head, + notFoundComponent: () => null, + }) + + const router = createTestRouter({ + routeTree: rootRoute.addChildren([fooRoute]), + history: createMemoryHistory(), + }) + + await router.preloadRoute({ to: '/foo' }) + + expect(loader).toHaveBeenCalledTimes(1) + expect(head).toHaveBeenCalledTimes(1) + }) + + test('executes head when beforeLoad throws notFound during preload', async () => { + const beforeLoad = vi.fn(({ preload }) => { + if (preload) { + throw notFound() + } + }) + const head = vi.fn(() => ({ meta: [{ title: 'Foo' }] })) + + const rootRoute = new BaseRootRoute({}) + const fooRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/foo', + beforeLoad, + head, + notFoundComponent: () => null, + }) + + const router = createTestRouter({ + routeTree: rootRoute.addChildren([fooRoute]), + history: createMemoryHistory(), + }) + + await router.preloadRoute({ to: '/foo' }) + + expect(beforeLoad).toHaveBeenCalledTimes(1) + expect(head).toHaveBeenCalledTimes(1) + }) + + test('exec if pending preload (error)', async () => { + const beforeLoad = vi.fn(async ({ preload }) => { + await sleep(100) + if (preload) throw new Error('error') + }) + const router = setup({ + beforeLoad, + }) + router.preloadRoute({ to: '/foo' }) + await Promise.resolve() + await router.navigate({ to: '/foo' }) + + expect(beforeLoad).toHaveBeenCalledTimes(2) + }) +}) + +describe('loader skip or exec', () => { + const setup = ({ + loader, + staleTime, + defaultStaleReloadMode, + }: { + loader?: Loader + staleTime?: number + defaultStaleReloadMode?: LoaderStaleReloadMode + }) => { + const rootRoute = new BaseRootRoute({}) + + const fooRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/foo', + loader, + staleTime, + gcTime: staleTime, + }) + + const barRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/bar', + }) + + const routeTree = rootRoute.addChildren([fooRoute, barRoute]) + + const router = createTestRouter({ + routeTree, + defaultStaleReloadMode, + history: createMemoryHistory(), + }) + + return router + } + + test('baseline', async () => { + const loader = vi.fn() + const router = setup({ loader }) + await router.load() + expect(loader).toHaveBeenCalledTimes(0) + }) + + test('does not call shouldReload on initial pending load', async () => { + const loader = vi.fn() + const shouldReload = vi.fn(() => false) + + const rootRoute = new BaseRootRoute({}) + const fooRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/foo', + loader, + shouldReload, + }) + + const router = createTestRouter({ + routeTree: rootRoute.addChildren([fooRoute]), + history: createMemoryHistory({ initialEntries: ['/foo'] }), + }) + + await router.load() + + expect(loader).toHaveBeenCalledTimes(1) + expect(shouldReload).not.toHaveBeenCalled() + }) + + test('active preload joins active match instead of cached duplicate with same id', async () => { + const loader = vi.fn(() => ({ source: 'active' })) + const router = setup({ loader }) + + await router.navigate({ to: '/foo' }) + + const activeMatch = router.state.matches.find((match) => + match.id.endsWith('/foo'), + )! + + router.stores.setCached([ + ...router.stores.cachedMatches.get(), + { + ...activeMatch, + loaderData: { source: 'cached' }, + preload: true, + }, + ]) + + const matches = await router.preloadRoute({ to: '/foo' }) + const preloadedMatch = matches?.find((match) => match.id === activeMatch.id) + + expect(loader).toHaveBeenCalledTimes(1) + expect(preloadedMatch?.loaderData).toEqual({ source: 'active' }) + }) + + test('active preload does not execute active head hooks', async () => { + const loader = vi.fn(() => ({ source: 'active' })) + const head = vi.fn(() => ({ meta: [{ title: 'Foo' }] })) + + const rootRoute = new BaseRootRoute({}) + const fooRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/foo', + loader, + head, + }) + + const router = createTestRouter({ + routeTree: rootRoute.addChildren([fooRoute]), + history: createMemoryHistory(), + }) + + await router.navigate({ to: '/foo' }) + expect(head).toHaveBeenCalledTimes(1) + + await router.preloadRoute({ to: '/foo' }) + + expect(loader).toHaveBeenCalledTimes(1) + expect(head).toHaveBeenCalledTimes(1) + }) + + test('preloadRoute returns cache-owned matches with loaderData after load', async () => { + const loader = vi.fn(() => ({ source: 'preload' })) + const router = setup({ loader }) + + const matches = await router.preloadRoute({ to: '/foo' }) + const match = matches?.find((d) => d.id === '/foo/foo') + + expect(loader).toHaveBeenCalledTimes(1) + expect(match?.loaderData).toEqual({ source: 'preload' }) + }) + + test('head assetContext.matches sees lane-updated loaderData', async () => { + const parentLoader = vi.fn(() => ({ parent: 'data' })) + const seenParentLoaderData: Array = [] + + const rootRoute = new BaseRootRoute({}) + const parentRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/parent', + loader: parentLoader, + }) + const childRoute = new BaseRoute({ + getParentRoute: () => parentRoute, + path: '/child', + head: ({ matches }) => { + seenParentLoaderData.push( + matches.find((match) => match.routeId === parentRoute.id)?.loaderData, + ) + return { meta: [{ title: 'Child' }] } + }, + }) + + const router = createTestRouter({ + routeTree: rootRoute.addChildren([parentRoute.addChildren([childRoute])]), + history: createMemoryHistory(), + }) + + await router.preloadRoute({ to: '/parent/child' }) + + expect(parentLoader).toHaveBeenCalledTimes(1) + expect(seenParentLoaderData).toEqual([{ parent: 'data' }]) + }) + + test('same-location load prefers active match over cached duplicate with same id', async () => { + const loader = vi.fn(() => ({ source: 'active' })) + const router = setup({ loader, staleTime: Infinity }) + + await router.navigate({ to: '/foo' }) + + const activeMatch = router.state.matches.find((match) => + match.id.endsWith('/foo'), + )! + + router.stores.setCached([ + ...router.stores.cachedMatches.get(), + { + ...activeMatch, + loaderData: { source: 'cached' }, + preload: true, + }, + ]) + + await router.load() + + const loadedMatch = router.state.matches.find( + (match) => match.id === activeMatch.id, + ) + + expect(loader).toHaveBeenCalledTimes(1) + expect(loadedMatch?.loaderData).toEqual({ source: 'active' }) + }) + + test('preload child context uses active parent over cached duplicate with same id', async () => { + const seenParentContext: Array = [] + + const rootRoute = new BaseRootRoute({}) + const parentRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/parent', + context: () => ({ source: 'active' }), + }) + const childRoute = new BaseRoute({ + getParentRoute: () => parentRoute, + path: '/child', + context: ({ context }) => { + seenParentContext.push(context.source) + return {} + }, + }) + + const router = createTestRouter({ + routeTree: rootRoute.addChildren([parentRoute.addChildren([childRoute])]), + history: createMemoryHistory({ initialEntries: ['/parent'] }), + }) + + await router.load() + + const activeParent = router.state.matches.find( + (match) => match.routeId === parentRoute.id, + )! + + router.stores.setCached([ + ...router.stores.cachedMatches.get(), + { + ...activeParent, + __routeContext: { source: 'cached' }, + context: { source: 'cached' }, + preload: true, + }, + ]) + + await router.preloadRoute({ to: '/parent/child' }) + + expect(seenParentContext).toEqual(['active']) + }) + + test('active redirect ignores cached duplicate ownership by id', async () => { + const rootRoute = new BaseRootRoute({}) + const fooRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/foo', + loader: () => redirect({ to: '/bar' }), + }) + const barRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/bar', + }) + + const router = createTestRouter({ + routeTree: rootRoute.addChildren([fooRoute, barRoute]), + history: createMemoryHistory({ initialEntries: ['/foo'] }), + }) + + const location = router.latestLocation + const matches = router.matchRoutes(location) + const fooMatch = matches.find((match) => match.routeId === fooRoute.id)! + const activeLoadPromise = fooMatch._nonReactive.loadPromise + + router.stores.setPending(matches) + router.stores.setCached([ + ...router.stores.cachedMatches.get(), + { + ...fooMatch, + preload: true, + status: 'success', + }, + ]) + + await expect( + loadMatches({ + router, + location, + matches, + }), + ).rejects.toMatchObject({ + options: expect.objectContaining({ to: '/bar' }), + }) + + expect(activeLoadPromise?.status).toBe('pending') + }) + + test('exec on regular nav', async () => { + const loader = vi.fn(() => Promise.resolve({ hello: 'world' })) + const router = setup({ loader }) + const navigation = router.navigate({ to: '/foo' }) + expect(loader).toHaveBeenCalledTimes(1) + expect(router.stores.pendingMatches.get()).toEqual( + expect.arrayContaining([expect.objectContaining({ id: '/foo/foo' })]), + ) + await navigation + expect(router.state.location.pathname).toBe('/foo') + expect(router.state.matches).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + id: '/foo/foo', + loaderData: { + hello: 'world', + }, + }), + ]), + ) + expect(loader).toHaveBeenCalledTimes(1) + }) + + test.each([false, true])( + 'handles %s async returned redirects from loader', + async (asyncReturn) => { + const loader = vi.fn(() => { + const result = redirect({ to: '/bar' }) + return asyncReturn ? Promise.resolve(result) : result + }) + const router = setup({ loader }) + + await router.navigate({ to: '/foo' }) + + expect(router.state.location.pathname).toBe('/bar') + expect(loader).toHaveBeenCalledTimes(1) + }, + ) + + test.each([false, true])( + 'handles %s async returned notFounds from loader', + async (asyncReturn) => { + const rootRoute = new BaseRootRoute({}) + const fooRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/foo', + loader: () => { + const result = notFound() + return asyncReturn ? Promise.resolve(result) : result + }, + notFoundComponent: () => null, + }) + + const routeTree = rootRoute.addChildren([fooRoute]) + const router = createTestRouter({ + routeTree, + history: createMemoryHistory(), + }) + + await router.navigate({ to: '/foo' }) + + const match = router.state.matches.find((m) => m.routeId === fooRoute.id) + expect(match?.status).toBe('notFound') + expect(router.state.statusCode).toBe(404) + }, + ) + + test('settles descendant match when notFound renders an ancestor boundary', async () => { + const rootRoute = new BaseRootRoute({}) + const parentRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/parent', + notFoundComponent: () => null, + }) + const childRoute = new BaseRoute({ + getParentRoute: () => parentRoute, + path: '/child', + loader: () => notFound({ routeId: parentRoute.id }), + }) + + const routeTree = rootRoute.addChildren([ + parentRoute.addChildren([childRoute]), + ]) + const router = createTestRouter({ + routeTree, + history: createMemoryHistory(), + }) + + await router.navigate({ to: '/parent/child' }) + + const parentMatch = router.state.matches.find( + (m) => m.routeId === parentRoute.id, + ) + const childMatch = router.state.matches.find( + (m) => m.routeId === childRoute.id, + ) + expect(parentMatch?.status).toBe('notFound') + expect(childMatch).toMatchObject({ + status: 'notFound', + isFetching: false, + error: expect.objectContaining({ isNotFound: true }), + }) + }) + + test('preload notFound targeting active parent does not mutate borrowed parent boundary', async () => { + const rootRoute = new BaseRootRoute({}) + const parentRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/parent', + notFoundComponent: () => null, + }) + const childRoute = new BaseRoute({ + getParentRoute: () => parentRoute, + path: '/child', + loader: () => notFound({ routeId: parentRoute.id }), + }) + + const routeTree = rootRoute.addChildren([ + parentRoute.addChildren([childRoute]), + ]) + const router = createTestRouter({ + routeTree, + history: createMemoryHistory(), + }) + + await router.navigate({ to: '/parent' }) + + const activeParentBefore = router.state.matches.find( + (match) => match.routeId === parentRoute.id, ) - await navigation - expect(router.state.location.pathname).toBe('/foo') - expect(router.state.matches).toEqual( - expect.arrayContaining([ - expect.objectContaining({ - id: '/foo/foo', - loaderData: { - hello: 'world', - }, - }), - ]), + expect(activeParentBefore?.status).toBe('success') + + await router.preloadRoute({ to: '/parent/child' }) + + const activeParentAfter = router.state.matches.find( + (match) => match.routeId === parentRoute.id, ) - expect(loader).toHaveBeenCalledTimes(1) + expect(activeParentAfter?.status).toBe('success') + expect(activeParentAfter?.error).toBeUndefined() }) test('exec if resolved preload (success)', async () => { @@ -520,14 +1907,14 @@ describe('loader skip or exec', () => { }) await router.preloadRoute({ to: '/foo' }) expect( - router.stores.cachedMatches.get().some((d) => d.status === 'redirected'), + router.stores.cachedMatches.get().some((d) => d.id === '/foo/foo'), ).toBe(false) await sleep(10) await router.navigate({ to: '/foo' }) expect(router.state.location.pathname).toBe('/foo') expect( - router.stores.cachedMatches.get().some((d) => d.status === 'redirected'), + router.stores.cachedMatches.get().some((d) => d.id === '/foo/foo'), ).toBe(false) expect(loader).toHaveBeenCalledTimes(2) }) @@ -542,19 +1929,163 @@ describe('loader skip or exec', () => { }) router.preloadRoute({ to: '/foo' }) await Promise.resolve() - expect( - router.stores.cachedMatches.get().some((d) => d.status === 'redirected'), - ).toBe(false) await router.navigate({ to: '/foo' }) expect(router.state.location.pathname).toBe('/bar') expect( - router.stores.cachedMatches.get().some((d) => d.status === 'redirected'), + router.stores.cachedMatches.get().some((d) => d.id === '/foo/foo'), ).toBe(false) expect(loader).toHaveBeenCalledTimes(1) }) - test('updateMatch removes redirected matches from cachedMatches', async () => { + test('keeps active pending match renderable when an older preload redirects', async () => { + vi.useFakeTimers() + + try { + let rejectFoo!: (error: unknown) => void + let resolveBar!: () => void + const rootRoute = new BaseRootRoute({}) + const indexRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/', + }) + const fooRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/foo', + pendingMs: 1, + pendingComponent: {}, + loader: () => + new Promise((_resolve, reject) => { + rejectFoo = reject + }), + }) + const barRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/bar', + loader: () => + new Promise((resolve) => { + resolveBar = resolve + }), + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([indexRoute, fooRoute, barRoute]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + + await router.load() + + const preload = router.preloadRoute({ to: '/foo' }) + await vi.waitFor(() => expect(rejectFoo).toBeTypeOf('function')) + + const navigation = router.navigate({ to: '/foo' }) + await vi.advanceTimersByTimeAsync(1) + await vi.waitFor(() => + expect( + router.state.matches.some( + (match) => match.id === '/foo/foo' && match.status === 'pending', + ), + ).toBe(true), + ) + + rejectFoo(redirect({ to: '/bar' })) + await vi.waitFor(() => + expect( + router.stores.pendingMatches + .get() + .some((match) => match.id === '/bar/bar'), + ).toBe(true), + ) + + expect( + router.state.matches.find((match) => match.id === '/foo/foo')?.status, + ).toBe('pending') + + resolveBar() + await Promise.all([preload, navigation]) + + expect(router.state.location.pathname).toBe('/bar') + } finally { + vi.useRealTimers() + } + }) + + test('active-join preload rethrows redirect without clearing active owner loadPromise', async () => { + vi.useFakeTimers() + + try { + let rejectFoo!: (error: unknown) => void + let resolveBar!: () => void + const rootRoute = new BaseRootRoute({}) + const indexRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/', + }) + const fooRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/foo', + pendingMs: 1, + pendingComponent: {}, + loader: () => + new Promise((_resolve, reject) => { + rejectFoo = reject + }), + }) + const barRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/bar', + loader: () => + new Promise((resolve) => { + resolveBar = resolve + }), + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([indexRoute, fooRoute, barRoute]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + + await router.load() + + const navigation = router.navigate({ to: '/foo' }) + await vi.waitFor(() => expect(rejectFoo).toBeTypeOf('function')) + await vi.advanceTimersByTimeAsync(1) + await vi.waitFor(() => + expect( + router.state.matches.some( + (match) => match.id === '/foo/foo' && match.status === 'pending', + ), + ).toBe(true), + ) + + const activeFoo = router.state.matches.find( + (match) => match.id === '/foo/foo', + )! + const activeLoadPromise = activeFoo._nonReactive.loadPromise + expect(activeLoadPromise?.status).toBe('pending') + + const preload = router.preloadRoute({ to: '/foo' }) + await Promise.resolve() + + rejectFoo(redirect({ to: '/bar' })) + await vi.waitFor(() => + expect( + router.stores.pendingMatches + .get() + .some((match) => match.id === '/bar/bar'), + ).toBe(true), + ) + + expect(activeLoadPromise?.status).toBe('pending') + + resolveBar() + await Promise.all([navigation, preload]) + + expect(router.state.location.pathname).toBe('/bar') + } finally { + vi.useRealTimers() + } + }) + + test('updateMatch removes failed matches from cachedMatches', async () => { const loader = vi.fn() const router = setup({ loader }) @@ -565,15 +2096,13 @@ describe('loader skip or exec', () => { router.updateMatch('/foo/foo', (prev) => ({ ...prev, - status: 'redirected', + status: 'error', + error: new Error('boom'), })) expect( router.stores.cachedMatches.get().some((d) => d.id === '/foo/foo'), ).toBe(false) - expect( - router.stores.cachedMatches.get().some((d) => d.status === 'redirected'), - ).toBe(false) }) test('exec if rejected preload (error)', async () => { @@ -738,7 +2267,12 @@ describe('stale loader reload triggers', () => { path: '/bar', }) - const routeTree = rootRoute.addChildren([fooRoute, barRoute]) + const bazRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/baz', + }) + + const routeTree = rootRoute.addChildren([fooRoute, barRoute, bazRoute]) return createTestRouter({ routeTree, @@ -1152,6 +2686,133 @@ describe('stale loader reload triggers', () => { resolveStaleReload, ) }) + + test('settles promises and drops cache entry when a background stale reload redirects', async () => { + let rejectStaleReload!: (error: unknown) => void + let loaderCalls = 0 + const loader = vi.fn(() => { + loaderCalls += 1 + if (loaderCalls === 1) { + return { value: 'first' } + } + + return new Promise((_resolve, reject) => { + rejectStaleReload = reject + }) + }) + const router = setup({ loader, staleTime: 0 }) + + await router.navigate({ to: '/foo' }) + expect(loader).toHaveBeenCalledTimes(1) + + await vi.advanceTimersByTimeAsync(1) + await router.load() + await vi.waitFor(() => expect(loader).toHaveBeenCalledTimes(2)) + + const fooMatch = getMatchById(router, '/foo/foo')! + const backgroundLoaderPromise = fooMatch._nonReactive.loaderPromise + const backgroundLoadPromise = fooMatch._nonReactive.loadPromise + + expect(backgroundLoaderPromise?.status).toBe('pending') + expect(backgroundLoadPromise?.status).toBe('pending') + + rejectStaleReload(redirect({ to: '/bar' })) + await backgroundLoaderPromise + await vi.waitFor(() => expect(router.state.location.pathname).toBe('/bar')) + + expect(backgroundLoadPromise?.status).toBe('resolved') + expect(fooMatch._nonReactive.loaderPromise).toBeUndefined() + expect(fooMatch._nonReactive.loadPromise).toBeUndefined() + expect( + router.stores.cachedMatches + .get() + .some((match) => match.id === '/foo/foo'), + ).toBe(false) + }) + + test('settles promises and drops cache entry when a cached background stale reload redirects', async () => { + let rejectStaleReload!: (error: unknown) => void + let loaderCalls = 0 + const loader = vi.fn(() => { + loaderCalls += 1 + if (loaderCalls === 1) { + return { value: 'first' } + } + + return new Promise((_resolve, reject) => { + rejectStaleReload = reject + }) + }) + const router = setup({ loader, staleTime: 0 }) + + await router.navigate({ to: '/foo' }) + await vi.advanceTimersByTimeAsync(1) + await router.load() + await vi.waitFor(() => expect(loader).toHaveBeenCalledTimes(2)) + + const fooMatch = getMatchById(router, '/foo/foo')! + const backgroundLoaderPromise = fooMatch._nonReactive.loaderPromise + const backgroundLoadPromise = fooMatch._nonReactive.loadPromise + + expect(backgroundLoaderPromise?.status).toBe('pending') + expect(backgroundLoadPromise?.status).toBe('pending') + + await router.navigate({ to: '/bar' }) + expect(router.state.location.pathname).toBe('/bar') + expect( + router.stores.cachedMatches + .get() + .some((match) => match.id === '/foo/foo'), + ).toBe(true) + + rejectStaleReload(redirect({ to: '/baz' })) + await backgroundLoaderPromise + await vi.waitFor(() => expect(router.state.location.pathname).toBe('/baz')) + await vi.waitFor(() => + expect( + router.stores.cachedMatches + .get() + .some((match) => match.id === '/foo/foo'), + ).toBe(false), + ) + + expect(backgroundLoadPromise?.status).toBe('resolved') + expect(fooMatch._nonReactive.loaderPromise).toBeUndefined() + expect(fooMatch._nonReactive.loadPromise).toBeUndefined() + }) + + test('settles promises and drops cache entry when a cached pending preload errors', async () => { + let rejectPreload!: (error: unknown) => void + const loader = vi.fn(() => { + return new Promise((_resolve, reject) => { + rejectPreload = reject + }) + }) + const router = setup({ loader }) + + const preload = router.preloadRoute({ to: '/foo' }) + await vi.waitFor(() => expect(loader).toHaveBeenCalledTimes(1)) + + const fooMatch = getMatchById(router, '/foo/foo')! + const loaderPromise = fooMatch._nonReactive.loaderPromise + const loadPromise = fooMatch._nonReactive.loadPromise + + expect(loaderPromise?.status).toBe('pending') + expect(loadPromise?.status).toBe('pending') + + rejectPreload(new Error('preload failed')) + await preload + + expect(loaderPromise?.status).toBe('resolved') + expect(loadPromise?.status).toBe('resolved') + expect(fooMatch._nonReactive.loaderPromise).toBeUndefined() + expect(fooMatch._nonReactive.loadPromise).toBeUndefined() + expect( + router.stores.cachedMatches + .get() + .some((match) => match.id === '/foo/foo'), + ).toBe(false) + }) }) test('cancelMatches after pending timeout', async () => { @@ -1183,24 +2844,290 @@ test('cancelMatches after pending timeout', async () => { const routeTree = rootRoute.addChildren([fooRoute, barRoute]) const router = createTestRouter({ routeTree, history: createMemoryHistory() }) - await router.load() - router.navigate({ to: '/foo' }) - await sleep(WAIT_TIME * 30) + await router.load() + router.navigate({ to: '/foo' }) + await sleep(WAIT_TIME * 30) + + // At this point, pending timeout should have triggered + const fooMatch = router.getMatch('/foo/foo') + expect(fooMatch).toBeDefined() + + // Navigate away, which should cancel the pending match + await router.navigate({ to: '/bar' }) + await router.latestLoadPromise + + expect(router.state.location.pathname).toBe('/bar') + + // Verify that abort was called and pending timeout was cleared + expect(onAbortMock).toHaveBeenCalled() + const cancelledFooMatch = router.getMatch('/foo/foo') + expect(cancelledFooMatch?._nonReactive.pendingTimeout).toBeUndefined() +}) + +test('pending timeout clears itself so a later load pass can re-arm it', async () => { + vi.useFakeTimers() + + try { + const WAIT_TIME = 5 + let resolveLoader!: () => void + const loader = vi.fn( + () => + new Promise((resolve) => { + resolveLoader = resolve + }), + ) + + const rootRoute = new BaseRootRoute({}) + const fooRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/foo', + pendingMs: WAIT_TIME, + loader, + pendingComponent: {}, + }) + const routeTree = rootRoute.addChildren([fooRoute]) + const router = createTestRouter({ + routeTree, + history: createMemoryHistory(), + }) + + await router.load() + const navigation = router.navigate({ to: '/foo' }) + await vi.advanceTimersByTimeAsync(WAIT_TIME * 2) + + const firstPendingMatch = router.getMatch('/foo/foo') + expect(firstPendingMatch?._nonReactive.pendingTimeout).toBeUndefined() + + const joinedLoad = router.load() + await Promise.resolve() + + const rearmedMatch = router.getMatch('/foo/foo') + expect(rearmedMatch?._nonReactive.pendingTimeout).toBeDefined() + + await vi.advanceTimersByTimeAsync(WAIT_TIME * 2) + expect(rearmedMatch?._nonReactive.pendingTimeout).toBeUndefined() + + resolveLoader() + await Promise.all([navigation, joinedLoad]) + expect(loader).toHaveBeenCalledTimes(1) + } finally { + vi.useRealTimers() + } +}) + +test('settles load promise for pending-visible match that redirects after exiting', async () => { + vi.useFakeTimers() + + try { + let rejectLoader!: (error: unknown) => void + const rootRoute = new BaseRootRoute({}) + const indexRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/', + }) + const fromRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/from', + pendingMs: 1, + pendingComponent: {}, + loader: () => + new Promise((_resolve, reject) => { + rejectLoader = reject + }), + }) + const toRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/to', + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([indexRoute, fromRoute, toRoute]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + + await router.load() + + const navigation = router.navigate({ to: '/from' }) + await vi.waitFor(() => expect(router.state.status).toBe('pending')) + await vi.advanceTimersByTimeAsync(1) + await vi.waitFor(() => + expect( + router.state.matches.some( + (match) => match.id === '/from/from' && match.status === 'pending', + ), + ).toBe(true), + ) + + const fromMatch = router.state.matches.find( + (match) => match.id === '/from/from', + )! + const loadPromise = fromMatch._nonReactive.loadPromise + + expect(loadPromise?.status).toBe('pending') + + rejectLoader(redirect({ to: '/to' })) + await navigation + + expect(router.state.location.pathname).toBe('/to') + expect(loadPromise?.status).toBe('resolved') + expect(fromMatch._nonReactive.loadPromise).toBeUndefined() + expect( + router.stores.cachedMatches + .get() + .some((match) => match.id === '/from/from'), + ).toBe(false) + } finally { + vi.useRealTimers() + } +}) + +test('ignores late loader resolution after pending-visible match exits', async () => { + vi.useFakeTimers() + + try { + let resolveLoader!: () => void + const rootRoute = new BaseRootRoute({}) + const indexRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/', + }) + const fromRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/from', + pendingMs: 1, + pendingComponent: {}, + loader: () => + new Promise((resolve) => { + resolveLoader = resolve + }), + }) + const toRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/to', + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([indexRoute, fromRoute, toRoute]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + + await router.load() + + const fromNavigation = router.navigate({ to: '/from' }) + await vi.waitFor(() => expect(router.state.status).toBe('pending')) + await vi.advanceTimersByTimeAsync(1) + await vi.waitFor(() => + expect( + router.state.matches.some( + (match) => match.id === '/from/from' && match.status === 'pending', + ), + ).toBe(true), + ) + + const fromMatch = router.state.matches.find( + (match) => match.id === '/from/from', + )! + const minPendingPromise = createControlledPromise() + fromMatch._nonReactive.minPendingPromise = minPendingPromise + const loaderPromise = fromMatch._nonReactive.loaderPromise + const loadPromise = fromMatch._nonReactive.loadPromise + + expect(minPendingPromise.status).toBe('pending') + expect(loaderPromise?.status).toBe('pending') + expect(loadPromise?.status).toBe('pending') + + await router.navigate({ to: '/to' }) + + expect(router.state.location.pathname).toBe('/to') + expect(minPendingPromise.status).toBe('resolved') + expect(fromMatch._nonReactive.minPendingPromise).toBeUndefined() + expect(loaderPromise?.status).toBe('resolved') + expect(loadPromise?.status).toBe('resolved') + expect(fromMatch._nonReactive.loaderPromise).toBeUndefined() + expect(fromMatch._nonReactive.loadPromise).toBeUndefined() + + resolveLoader() + await fromNavigation + + expect(router.state.location.pathname).toBe('/to') + expect( + router.stores.cachedMatches + .get() + .some((match) => match.id === '/from/from'), + ).toBe(false) + } finally { + vi.useRealTimers() + } +}) + +test('settles promises for pending-visible match whose loader rejects AbortError after exiting', async () => { + vi.useFakeTimers() - // At this point, pending timeout should have triggered - const fooMatch = router.getMatch('/foo/foo') - expect(fooMatch).toBeDefined() + try { + const rootRoute = new BaseRootRoute({}) + const indexRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/', + }) + const fromRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/from', + pendingMs: 1, + pendingComponent: {}, + loader: ({ abortController }) => + new Promise((_resolve, reject) => { + abortController.signal.addEventListener('abort', () => { + const abortError = new Error('aborted') + abortError.name = 'AbortError' + reject(abortError) + }) + }), + }) + const toRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/to', + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([indexRoute, fromRoute, toRoute]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) - // Navigate away, which should cancel the pending match - await router.navigate({ to: '/bar' }) - await router.latestLoadPromise + await router.load() - expect(router.state.location.pathname).toBe('/bar') + const fromNavigation = router.navigate({ to: '/from' }) + await vi.waitFor(() => expect(router.state.status).toBe('pending')) + await vi.advanceTimersByTimeAsync(1) + await vi.waitFor(() => + expect( + router.state.matches.some( + (match) => match.id === '/from/from' && match.status === 'pending', + ), + ).toBe(true), + ) - // Verify that abort was called and pending timeout was cleared - expect(onAbortMock).toHaveBeenCalled() - const cancelledFooMatch = router.getMatch('/foo/foo') - expect(cancelledFooMatch?._nonReactive.pendingTimeout).toBeUndefined() + const fromMatch = router.state.matches.find( + (match) => match.id === '/from/from', + )! + const loaderPromise = fromMatch._nonReactive.loaderPromise + const loadPromise = fromMatch._nonReactive.loadPromise + + expect(loaderPromise?.status).toBe('pending') + expect(loadPromise?.status).toBe('pending') + + await router.navigate({ to: '/to' }) + await fromNavigation + + expect(router.state.location.pathname).toBe('/to') + expect(loaderPromise?.status).toBe('resolved') + expect(loadPromise?.status).toBe('resolved') + expect(fromMatch._nonReactive.loaderPromise).toBeUndefined() + expect(fromMatch._nonReactive.loadPromise).toBeUndefined() + expect( + router.stores.cachedMatches + .get() + .some((match) => match.id === '/from/from'), + ).toBe(false) + } finally { + vi.useRealTimers() + } }) describe('head execution', () => { @@ -1405,7 +3332,6 @@ describe('head execution', () => { router, location, matches, - updateMatch: router.updateMatch, }), ).rejects.toBe(beforeLoadError) @@ -1417,6 +3343,45 @@ describe('head execution', () => { expect(childHead).toHaveBeenCalledTimes(1) }) + test('clears force pending when beforeLoad throws non-notFound error', async () => { + const beforeLoadError = new Error('beforeLoad-sync-error') + const rootRoute = new BaseRootRoute({}) + + const childRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/test', + beforeLoad: () => { + throw beforeLoadError + }, + }) + + const routeTree = rootRoute.addChildren([childRoute]) + const router = createTestRouter({ + routeTree, + history: createMemoryHistory({ initialEntries: ['/test'] }), + }) + + const location = router.latestLocation + const matches = router.matchRoutes(location) + const childMatch = matches[1]! + childMatch._forcePending = true + childMatch._nonReactive.minPendingPromise = createControlledPromise() + router.stores.setPending(matches) + + await expect( + loadMatches({ + router, + location, + matches, + }), + ).rejects.toBe(beforeLoadError) + + const updatedMatch = router.getMatch(childMatch.id) + expect(updatedMatch?.status).toBe('error') + expect(updatedMatch?._forcePending).toBeUndefined() + expect(updatedMatch?._nonReactive.minPendingPromise).toBeUndefined() + }) + test('propagates async beforeLoad non-notFound error running ancestor loaders and heads', async () => { const beforeLoadError = new Error('beforeLoad-async-error') const rootLoader = vi.fn(() => ({ level: 0 })) @@ -1456,7 +3421,6 @@ describe('head execution', () => { router, location, matches, - updateMatch: router.updateMatch, }), ).rejects.toBe(beforeLoadError) @@ -1470,13 +3434,13 @@ describe('head execution', () => { describe('beforeLoad notFound parent loader outcomes', () => { type ThrowAtIndex = 1 | 2 | 3 - type ParentFailure = 'notFound' | 'redirect' | 'error' + type ParentFailure = 'notFound' | 'redirect' type ParentFailureMap = Partial> type Scenario = { name: string throwAtIndex: ThrowAtIndex parentFailures: ParentFailureMap - expectedErrorKind: 'notFound' | 'redirect' | 'error' + expectedErrorKind: 'notFound' | 'redirect' expectedErrorSource?: string expectedErrorRouteIndex?: 0 | 1 | 2 | 3 expectedLoaderMaxIndex: number @@ -1552,14 +3516,6 @@ describe('head execution', () => { const routes = [rootRoute, level1Route, level2Route, level3Route] as const - ;([0, 1, 2] as const).forEach((index) => { - if (parentFailures[index] === 'error') { - ;(routes[index].options as any).shouldReload = () => { - throw new Error(`loader-${index}-error`) - } - } - }) - const throwRoute = routes[throwAtIndex]! throwRoute.options.beforeLoad = () => { const beforeLoadNotFound = beforeLoadNotFoundFactory @@ -1603,7 +3559,6 @@ describe('head execution', () => { router, location, matches, - updateMatch: router.updateMatch, }) return { error: undefined, matches } } catch (error) { @@ -1725,15 +3680,6 @@ describe('head execution', () => { expectedLoaderMaxIndex: 2, expectedRenderedHeadMaxIndex: -1, }, - { - name: 'propagates regular loader error when mixed with loader notFound in settled loaders', - throwAtIndex: 3 as const, - parentFailures: { 1: 'notFound', 2: 'error' } as ParentFailureMap, - expectedErrorKind: 'error' as const, - expectedErrorSource: 'loader-2-error', - expectedLoaderMaxIndex: 1, - expectedRenderedHeadMaxIndex: -1, - }, ] satisfies Array test.each(scenarios)('$name', async (scenario) => { @@ -1770,12 +3716,6 @@ describe('head execution', () => { return } - if (scenario.expectedErrorKind === 'error') { - expect(error).toBeInstanceOf(Error) - expect((error as Error).message).toBe(scenario.expectedErrorSource) - return - } - expect(error).toEqual( expect.objectContaining({ isNotFound: true, @@ -1903,7 +3843,6 @@ describe('head execution', () => { router, location, matches, - updateMatch: router.updateMatch, }), ).resolves.toBe(matches) @@ -1916,6 +3855,49 @@ describe('head execution', () => { expect(rootMatch?.globalNotFound).toBe(false) expect(rootMatch?.error).toBeUndefined() }) + + test('keeps root globalNotFound from overlapping stale initial load', async () => { + const rootRoute = new BaseRootRoute({ + notFoundComponent: () => null, + }) + const indexRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/', + }) + const postsRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/posts', + }) + + const router = createTestRouter({ + routeTree: rootRoute.addChildren([indexRoute, postsRoute]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + + const matchResult = router.getMatchedRoutes('/non-existent') + expect(matchResult.foundRoute).toBeUndefined() + expect(matchResult.matchedRoutes.map((route) => route.id)).toEqual([ + rootRoute.id, + ]) + + const initialLoad = router.load() + const notFoundNavigation = router.navigate({ + to: '/non-existent' as never, + }) + + await Promise.all([initialLoad, notFoundNavigation]) + + expect(router.state.location.pathname).toBe('/non-existent') + expect(router.state.statusCode).toBe(404) + expect(router.state.matches).toHaveLength(1) + expect(router.state.matches[0]).toEqual( + expect.objectContaining({ + routeId: rootRoute.id, + status: 'success', + globalNotFound: true, + }), + ) + }) }) }) @@ -2176,6 +4158,341 @@ describe('routeId in context options', () => { }) }) +describe('beforeLoad context lifecycle', () => { + test('cached preload reload commits fresh beforeLoad context to returned match context', async () => { + let token = 'one' + const beforeLoad = vi.fn(() => ({ token })) + + const rootRoute = new BaseRootRoute({}) + const fooRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/foo', + beforeLoad, + preloadStaleTime: Infinity, + }) + + const router = createTestRouter({ + routeTree: rootRoute.addChildren([fooRoute]), + history: createMemoryHistory(), + }) + + const first = await router.preloadRoute({ to: '/foo' }) + const firstMatch = first?.find((match) => match.routeId === fooRoute.id) + + expect(firstMatch?.__beforeLoadContext).toEqual({ token: 'one' }) + expect(firstMatch?.context).toMatchObject({ token: 'one' }) + + token = 'two' + + const second = await router.preloadRoute({ to: '/foo' }) + const secondMatch = second?.find((match) => match.routeId === fooRoute.id) + + expect(beforeLoad).toHaveBeenCalledTimes(2) + expect(secondMatch?.__beforeLoadContext).toEqual({ token: 'two' }) + expect(secondMatch?.context).toMatchObject({ token: 'two' }) + }) + + test('clears stale beforeLoad context when a later run returns undefined', async () => { + let returnContext = true + const seenContexts: Array> = [] + + const rootRoute = new BaseRootRoute({ + beforeLoad: () => { + return returnContext ? { token: 'one' } : undefined + }, + }) + const childRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/child', + staleTime: 0, + loader: ({ context }) => { + seenContexts.push(context) + }, + }) + + const router = createTestRouter({ + routeTree: rootRoute.addChildren([childRoute]), + history: createMemoryHistory({ initialEntries: ['/child'] }), + }) + + await router.load() + expect(seenContexts.at(-1)).toMatchObject({ token: 'one' }) + + returnContext = false + await router.invalidate({ sync: true }) + + expect(seenContexts.at(-1)).not.toHaveProperty('token') + expect(router.state.matches[0]?.__beforeLoadContext).toBeUndefined() + }) +}) + +describe('loadRouteChunk', () => { + test('partial notFoundComponent preload does not mark all components loaded', async () => { + const componentPreload = vi.fn() + const errorPreload = vi.fn() + const notFoundPreload = vi.fn() + const rootRoute = new BaseRootRoute({}) + const route = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/chunked', + component: { preload: componentPreload } as any, + errorComponent: { preload: errorPreload } as any, + notFoundComponent: { preload: notFoundPreload } as any, + }) + + await loadRouteChunk(route, ['notFoundComponent']) + + expect(notFoundPreload).toHaveBeenCalledTimes(1) + expect(componentPreload).not.toHaveBeenCalled() + expect(errorPreload).not.toHaveBeenCalled() + expect((route as any)._componentsLoaded).not.toBe(true) + + await loadRouteChunk(route) + + expect(componentPreload).toHaveBeenCalledTimes(1) + expect(errorPreload).toHaveBeenCalledTimes(1) + expect(notFoundPreload).toHaveBeenCalledTimes(2) + expect((route as any)._componentsLoaded).toBe(true) + + await loadRouteChunk(route) + + expect(componentPreload).toHaveBeenCalledTimes(1) + expect(errorPreload).toHaveBeenCalledTimes(1) + expect(notFoundPreload).toHaveBeenCalledTimes(2) + }) + + test('dedupes concurrent full component preloads', async () => { + let resolveComponent!: () => void + const componentPreload = vi.fn( + () => + new Promise((resolve) => { + resolveComponent = resolve + }), + ) + const rootRoute = new BaseRootRoute({}) + const route = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/chunked', + component: { preload: componentPreload } as any, + }) + + const first = loadRouteChunk(route) + const second = loadRouteChunk(route) + + expect(componentPreload).toHaveBeenCalledTimes(1) + + resolveComponent() + await Promise.all([first, second]) + + expect((route as any)._componentsLoaded).toBe(true) + + await loadRouteChunk(route) + + expect(componentPreload).toHaveBeenCalledTimes(1) + }) +}) + +describe('settle errors do not leak across load generations', () => { + test('clearCache settles promises for evicted cached matches', async () => { + const rootRoute = new BaseRootRoute({}) + const cachedRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/cached', + loader: () => undefined, + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([cachedRoute]), + history: createMemoryHistory(), + }) + await router.load() + await router.preloadRoute({ to: '/cached' }) + + const match = router.stores.cachedMatches.get()[0]! + const beforeLoadPromise = createControlledPromise() + const loaderPromise = createControlledPromise() + const loadPromise = createControlledPromise() + const minPendingPromise = createControlledPromise() + + match._nonReactive.beforeLoadPromise = beforeLoadPromise + match._nonReactive.loaderPromise = loaderPromise + match._nonReactive.loadPromise = loadPromise + match._nonReactive.minPendingPromise = minPendingPromise + + router.clearCache() + + expect(router.stores.cachedMatches.get()).toEqual([]) + expect(beforeLoadPromise.status).toBe('resolved') + expect(loaderPromise.status).toBe('resolved') + expect(loadPromise.status).toBe('resolved') + expect(minPendingPromise.status).toBe('resolved') + }) + + test('a stale redirect resolving after a newer navigation does not navigate or update redirect state', async () => { + const slowBeforeLoadStarted = vi.fn() + const slowBeforeLoadGate = createControlledPromise() + + const rootRoute = new BaseRootRoute({}) + const indexRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/', + }) + const slowRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/slow', + beforeLoad: async () => { + slowBeforeLoadStarted() + await slowBeforeLoadGate + throw redirect({ to: '/redirected' }) + }, + }) + const safeRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/safe', + }) + const redirectedRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/redirected', + }) + + const router = createTestRouter({ + routeTree: rootRoute.addChildren([ + indexRoute, + slowRoute, + safeRoute, + redirectedRoute, + ]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + await router.load() + + const staleNavigation = router.navigate({ to: '/slow' }) + await vi.waitFor(() => expect(slowBeforeLoadStarted).toHaveBeenCalled()) + + await router.navigate({ to: '/safe' }) + expect(router.state.location.pathname).toBe('/safe') + + slowBeforeLoadGate.resolve() + await staleNavigation + + expect(router.state.location.pathname).toBe('/safe') + expect(router.state.redirect).toBeUndefined() + }) + + test('a notFound stored by a previous preload is not replayed onto a load pass that joins a newer in-flight load', async () => { + let loaderCalls = 0 + let releaseLoader!: () => void + const loaderGate = new Promise((resolve) => { + releaseLoader = resolve + }) + + const loader = vi.fn(async () => { + loaderCalls++ + if (loaderCalls === 1) { + // the preload generation settles with notFound + throw notFound() + } + // the navigation generation succeeds, but slowly + await loaderGate + return { value: 'fresh' } + }) + + const rootRoute = new BaseRootRoute({}) + const staleRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/stale', + loader, + staleTime: 0, + gcTime: 60_000, + }) + const routeTree = rootRoute.addChildren([staleRoute]) + const router = createTestRouter({ + routeTree, + history: createMemoryHistory(), + }) + await router.load() + + // generation 1: the preload stores the notFound settle error on the + // cached match + await router.preloadRoute({ to: '/stale' }) + expect(loader).toHaveBeenCalledTimes(1) + + // generation 2: navigating reuses the cached match and starts the slow + // loader + const navigatePromise = router.navigate({ to: '/stale' }) + await vi.waitFor(() => expect(loader).toHaveBeenCalledTimes(2)) + + // generation 3: a load pass joins the in-flight generation 2 loader. + // It must observe generation 2's result, not the stale notFound settle + // error stored by generation 1. + const joinPromise = router.load() + await sleep(5) + + releaseLoader() + await Promise.all([navigatePromise, joinPromise]) + + const match = router.state.matches.find((m) => m.routeId === staleRoute.id)! + expect(match.status).toBe('success') + expect(match.loaderData).toEqual({ value: 'fresh' }) + }) + + test('a redirect stored by a previous preload is not replayed onto a load pass that joins a newer in-flight load', async () => { + let loaderCalls = 0 + let releaseLoader!: () => void + const loaderGate = new Promise((resolve) => { + releaseLoader = resolve + }) + + const loader = vi.fn(async () => { + loaderCalls++ + if (loaderCalls === 1) { + throw redirect({ to: '/other' }) + } + await loaderGate + return { value: 'fresh' } + }) + + const rootRoute = new BaseRootRoute({}) + const staleRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/stale', + loader, + staleTime: 0, + gcTime: 60_000, + }) + const otherRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/other', + }) + const routeTree = rootRoute.addChildren([staleRoute, otherRoute]) + const router = createTestRouter({ + routeTree, + history: createMemoryHistory(), + }) + await router.load() + + await router.preloadRoute({ to: '/stale' }) + expect(loader).toHaveBeenCalledTimes(1) + expect( + router.stores.cachedMatches + .get() + .some((match) => match.id === '/stale/stale'), + ).toBe(false) + + const navigatePromise = router.navigate({ to: '/stale' }) + await vi.waitFor(() => expect(loader).toHaveBeenCalledTimes(2)) + const joinPromise = router.load() + await sleep(5) + + releaseLoader() + await Promise.all([navigatePromise, joinPromise]) + + expect(router.state.location.pathname).toBe('/stale') + const match = router.state.matches.find((m) => m.routeId === staleRoute.id)! + expect(match.status).toBe('success') + expect(match.loaderData).toEqual({ value: 'fresh' }) + }) +}) + function sleep(ms: number) { return new Promise((resolve) => setTimeout(resolve, ms)) } diff --git a/packages/router-devtools-core/src/useStyles.tsx b/packages/router-devtools-core/src/useStyles.tsx index 5524ed0ae8..32908c524d 100644 --- a/packages/router-devtools-core/src/useStyles.tsx +++ b/packages/router-devtools-core/src/useStyles.tsx @@ -428,7 +428,7 @@ const stylesFactory = (shadowDOMTarget?: ShadowRoot) => { line-height: ${tokens.font.lineHeight.sm}; `, matchStatus: ( - status: 'pending' | 'success' | 'error' | 'notFound' | 'redirected', + status: 'pending' | 'success' | 'error' | 'notFound', isFetching: false | 'beforeLoad' | 'loader', ) => { const colorMap = { @@ -436,7 +436,6 @@ const stylesFactory = (shadowDOMTarget?: ShadowRoot) => { success: 'green', error: 'red', notFound: 'purple', - redirected: 'gray', } as const const color = diff --git a/packages/router-devtools-core/src/utils.tsx b/packages/router-devtools-core/src/utils.tsx index c14bfd80be..962cdaf0f3 100644 --- a/packages/router-devtools-core/src/utils.tsx +++ b/packages/router-devtools-core/src/utils.tsx @@ -25,7 +25,6 @@ export function getStatusColor(match: AnyRouteMatch) { success: 'green', error: 'red', notFound: 'purple', - redirected: 'gray', } as const return match.isFetching && match.status === 'success' diff --git a/packages/router-plugin/src/core/hmr/handle-route-update.ts b/packages/router-plugin/src/core/hmr/handle-route-update.ts index c0325b06e9..8731b19ac5 100644 --- a/packages/router-plugin/src/core/hmr/handle-route-update.ts +++ b/packages/router-plugin/src/core/hmr/handle-route-update.ts @@ -124,9 +124,8 @@ function handleRouteUpdate( // match from the store (via ...existingMatch spread) and the stale // loaderData / __beforeLoadContext survives the reload cycle. // - // We must update the store directly (not via router.updateMatch) because - // updateMatch wraps in startTransition which may defer the state update, - // and we need the clear to be visible before invalidate reads the store. + // We update the store directly so the clear is visible before invalidate + // reads the store and rematches the route. if (removedKeys.has('loader') || removedKeys.has('beforeLoad')) { const matchIds = [ activeMatch?.id, diff --git a/packages/solid-router/src/Match.tsx b/packages/solid-router/src/Match.tsx index 205e778689..577a5b2eee 100644 --- a/packages/solid-router/src/Match.tsx +++ b/packages/solid-router/src/Match.tsx @@ -4,7 +4,6 @@ import { getLocationChangeInfo, invariant, isNotFound, - isRedirect, rootRouteId, } from '@tanstack/router-core' import { isServer } from '@tanstack/router-core/isServer' @@ -256,6 +255,7 @@ export const MatchInner = (): any => { error: currentMatch.error, _forcePending: currentMatch._forcePending ?? false, _displayPending: currentMatch._displayPending ?? false, + _nonReactive: currentMatch._nonReactive, }, } }) @@ -270,6 +270,20 @@ export const MatchInner = (): any => { const componentKey = () => currentMatchState().key ?? currentMatchState().match.id + const PendingComponent = () => + route().options.pendingComponent ?? + router.options.defaultPendingComponent + + const pendingReplacement = () => { + const pendingMatch = router.stores.pendingMatches + .get() + .find((pending) => pending.routeId === currentMatchState().routeId) + return ( + pendingMatch?.status === 'pending' && + pendingMatch.id !== currentMatch().id + ) + } + const out = () => { const Comp = route().options.component ?? router.options.defaultComponent @@ -279,22 +293,6 @@ export const MatchInner = (): any => { return } - const getLoadPromise = ( - matchId: string, - fallbackMatch: - | { - _nonReactive: { - loadPromise?: Promise - } - } - | undefined, - ) => { - return ( - router.getMatch(matchId)?._nonReactive.loadPromise ?? - fallbackMatch?._nonReactive.loadPromise - ) - } - const keyedOut = () => ( {(_key) => out()} @@ -306,9 +304,7 @@ export const MatchInner = (): any => { {(_) => { const [displayPendingResult] = Solid.createResource( - () => - router.getMatch(currentMatch().id)?._nonReactive - .displayPendingPromise, + () => currentMatch()._nonReactive.displayPendingPromise, ) return <>{displayPendingResult()} @@ -317,37 +313,39 @@ export const MatchInner = (): any => { {(_) => { const [minPendingResult] = Solid.createResource( - () => - router.getMatch(currentMatch().id)?._nonReactive - .minPendingPromise, + () => currentMatch()._nonReactive.minPendingPromise, ) return <>{minPendingResult()} }} + + {(_) => { + const FallbackComponent = PendingComponent() + return FallbackComponent ? ( + + ) : null + }} + {(_) => { const pendingMinMs = route().options.pendingMinMs ?? router.options.defaultPendingMinMs + const matchBucket = currentMatch()._nonReactive if (pendingMinMs) { - const routerMatch = router.getMatch(currentMatch().id) - if ( - routerMatch && - !routerMatch._nonReactive.minPendingPromise - ) { + if (!matchBucket.minPendingPromise) { // Create a promise that will resolve after the minPendingMs if (!(isServer ?? router.isServer)) { const minPendingPromise = createControlledPromise() - routerMatch._nonReactive.minPendingPromise = - minPendingPromise + matchBucket.minPendingPromise = minPendingPromise setTimeout(() => { minPendingPromise.resolve() // We've handled the minPendingPromise, so we can delete it - routerMatch._nonReactive.minPendingPromise = undefined + matchBucket.minPendingPromise = undefined }, pendingMinMs) } } @@ -355,13 +353,10 @@ export const MatchInner = (): any => { const [loaderResult] = Solid.createResource(async () => { await Promise.resolve() - return router.getMatch(currentMatch().id)?._nonReactive - .loadPromise + return matchBucket.loadPromise }) - const FallbackComponent = - route().options.pendingComponent ?? - router.options.defaultPendingComponent + const FallbackComponent = PendingComponent() return ( <> @@ -395,29 +390,6 @@ export const MatchInner = (): any => { ) }} - - {(_) => { - const matchId = currentMatch().id - const routerMatch = router.getMatch(matchId) - - if (!isRedirect(currentMatch().error)) { - if (process.env.NODE_ENV !== 'production') { - throw new Error( - 'Invariant failed: Expected a redirect error', - ) - } - - invariant() - } - - const [loaderResult] = Solid.createResource(async () => { - await Promise.resolve() - return getLoadPromise(matchId, routerMatch) - }) - - return <>{loaderResult()} - }} - {(_) => { if (isServer ?? router.isServer) { @@ -469,14 +441,7 @@ export const Outlet = () => { : undefined }) - const childMatchStatus = Solid.createMemo(() => { - const id = childMatchId() - if (!id) return undefined - return router.stores.matchStores.get(id)?.get().status - }) - - const shouldShowNotFound = () => - childMatchStatus() !== 'redirected' && parentGlobalNotFound() + const shouldShowNotFound = () => parentGlobalNotFound() const childRouteKey = Solid.createMemo(() => { if (shouldShowNotFound()) return undefined diff --git a/packages/solid-router/tests/routeContext.test.tsx b/packages/solid-router/tests/routeContext.test.tsx index b7e0a902a9..aa5ec20ad7 100644 --- a/packages/solid-router/tests/routeContext.test.tsx +++ b/packages/solid-router/tests/routeContext.test.tsx @@ -2451,6 +2451,82 @@ describe('useRouteContext in the component', () => { expect(allContextsValid).toBe(true) }) + test('context value from beforeLoad is propagated when a sub-route is re-entered while its loader reloads in the background', async () => { + let sawUndefinedContext = false + const loaderTime = WAIT_TIME * 3 + + const rootRoute = createRootRoute({ + component: () => , + }) + const homeRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/', + component: () =>
Home page
, + }) + const reloadInFlightRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/reload-in-flight', + beforeLoad: () => ({ number: 42 }), + component: () => , + }) + const reloadInFlightIndexRoute = createRoute({ + getParentRoute: () => reloadInFlightRoute, + path: '/', + staleTime: 0, + loader: async () => { + await sleep(loaderTime) + }, + component: () => { + const context = reloadInFlightIndexRoute.useRouteContext() + const number = () => context().number + sawUndefinedContext ||= number() === undefined + + return ( +
+ number = {String(number())}, saw undefined ={' '} + {String(sawUndefinedContext)} +
+ ) + }, + }) + + const routeTree = rootRoute.addChildren([ + homeRoute, + reloadInFlightRoute.addChildren([reloadInFlightIndexRoute]), + ]) + const router = createRouter({ routeTree, history }) + + render(() => ) + + await router.navigate({ to: '/reload-in-flight' }) + + expect( + await screen.findByText('number = 42, saw undefined = false'), + ).toBeInTheDocument() + + await router.navigate({ to: '/' }) + expect(await screen.findByText('Home page')).toBeInTheDocument() + router.history.back() + + expect( + await screen.findByText('number = 42, saw undefined = false'), + ).toBeInTheDocument() + + await router.navigate({ to: '/' }) + expect(await screen.findByText('Home page')).toBeInTheDocument() + router.history.back() + + expect( + await screen.findByText('number = 42, saw undefined = false'), + ).toBeInTheDocument() + + await sleep(loaderTime + 50) + + expect( + await screen.findByText('number = 42, saw undefined = false'), + ).toBeInTheDocument() + }) + test('route context (sleep in loader), present root route', async () => { const rootRoute = createRootRoute({ loader: async () => { diff --git a/packages/solid-router/tests/store-updates-during-navigation.test.tsx b/packages/solid-router/tests/store-updates-during-navigation.test.tsx index 69b570122d..baf66ccafc 100644 --- a/packages/solid-router/tests/store-updates-during-navigation.test.tsx +++ b/packages/solid-router/tests/store-updates-during-navigation.test.tsx @@ -136,7 +136,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // This number should be as small as possible to minimize the amount of work // that needs to be done during a navigation. // Any change that increases this number should be investigated. - expect(updates).toBe(9) + expect(updates).toBe(7) }) test('redirection in preload', async () => { @@ -156,6 +156,11 @@ describe("Store doesn't update *too many* times during navigation", () => { // Any change that increases this number should be investigated. // Note: Solid has different update counts than React due to different reactivity expect(updates).toBe(2) + expect( + router.stores.cachedMatches + .get() + .some((match) => match.pathname === '/posts'), + ).toBe(false) }) test('sync beforeLoad', async () => { @@ -198,7 +203,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // This number should be as small as possible to minimize the amount of work // that needs to be done during a navigation. // Any change that increases this number should be investigated. - expect(updates).toBe(4) + expect(updates).toBe(3) }) test('hover preload, then navigate, w/ async loaders', async () => { diff --git a/packages/vue-router/src/Match.tsx b/packages/vue-router/src/Match.tsx index 44e9aca0cd..b9259c5db0 100644 --- a/packages/vue-router/src/Match.tsx +++ b/packages/vue-router/src/Match.tsx @@ -4,7 +4,6 @@ import { getLocationChangeInfo, invariant, isNotFound, - isRedirect, rootRouteId, } from '@tanstack/router-core' import { isServer } from '@tanstack/router-core/isServer' @@ -349,22 +348,6 @@ export const MatchInner = Vue.defineComponent({ const match = Vue.computed(() => combinedState.value?.match) const remountKey = Vue.computed(() => combinedState.value?.remountKey) - const getMatchPromise = ( - match: { - id: string - _nonReactive: { - displayPendingPromise?: Promise - minPendingPromise?: Promise - loadPromise?: Promise - } - }, - key: 'displayPendingPromise' | 'minPendingPromise' | 'loadPromise', - ) => { - return ( - router.getMatch(match.id)?._nonReactive[key] ?? match._nonReactive[key] - ) - } - return (): VNode | null => { // If match doesn't exist, return null (component is being unmounted or not ready) if (!combinedState.value || !match.value || !route.value) return null @@ -397,17 +380,6 @@ export const MatchInner = Vue.defineComponent({ return renderRouteNotFound(router, route.value, match.value.error) } - if (match.value.status === 'redirected') { - if (!isRedirect(match.value.error)) { - if (process.env.NODE_ENV !== 'production') { - throw new Error('Invariant failed: Expected a redirect error') - } - - invariant() - } - throw getMatchPromise(match.value, 'loadPromise') - } - if (match.value.status === 'error') { // Check if this route or any parent has an error component const RouteErrorComponent = @@ -437,22 +409,18 @@ export const MatchInner = Vue.defineComponent({ const pendingMinMs = route.value.options.pendingMinMs ?? router.options.defaultPendingMinMs - const routerMatch = router.getMatch(match.value.id) - if ( - pendingMinMs && - routerMatch && - !routerMatch._nonReactive.minPendingPromise - ) { + const matchBucket = Vue.toRaw(match.value._nonReactive) + if (pendingMinMs && !matchBucket.minPendingPromise) { // Create a promise that will resolve after the minPendingMs if (!(isServer ?? router.isServer)) { const minPendingPromise = createControlledPromise() - routerMatch._nonReactive.minPendingPromise = minPendingPromise + matchBucket.minPendingPromise = minPendingPromise setTimeout(() => { minPendingPromise.resolve() // We've handled the minPendingPromise, so we can delete it - routerMatch._nonReactive.minPendingPromise = undefined + matchBucket.minPendingPromise = undefined }, pendingMinMs) } } diff --git a/packages/vue-router/src/Transitioner.tsx b/packages/vue-router/src/Transitioner.tsx index b0133d965c..37e04bc374 100644 --- a/packages/vue-router/src/Transitioner.tsx +++ b/packages/vue-router/src/Transitioner.tsx @@ -82,9 +82,9 @@ export function useTransitionerSetup() { // Vue updates DOM asynchronously (next tick). The View Transitions API expects the // update callback promise to resolve only after the DOM has been updated. // Wrap the router-core implementation to await a Vue flush before resolving. - const originalStartViewTransition: - | undefined - | ((fn: () => Promise) => void) = + const originalStartViewTransition: ( + fn: () => Promise, + ) => Promise = (router as any).__tsrOriginalStartViewTransition ?? router.startViewTransition diff --git a/packages/vue-router/tests/store-updates-during-navigation.test.tsx b/packages/vue-router/tests/store-updates-during-navigation.test.tsx index 1c40bf7be7..b6e12e0216 100644 --- a/packages/vue-router/tests/store-updates-during-navigation.test.tsx +++ b/packages/vue-router/tests/store-updates-during-navigation.test.tsx @@ -138,7 +138,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // that needs to be done during a navigation. // Any change that increases this number should be investigated. // Note: Vue has different update counts than React/Solid due to different reactivity - expect(updates).toBe(16) + expect(updates).toBe(11) }) test('redirection in preload', async () => { @@ -157,7 +157,12 @@ describe("Store doesn't update *too many* times during navigation", () => { // that needs to be done during a navigation. // Any change that increases this number should be investigated. // Note: Vue has different update counts than React/Solid due to different reactivity - expect(updates).toBe(5) + expect(updates).toBe(3) + expect( + router.stores.cachedMatches + .get() + .some((match) => match.pathname === '/posts'), + ).toBe(false) }) test('sync beforeLoad', async () => { @@ -174,7 +179,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // that needs to be done during a navigation. // Any change that increases this number should be investigated. // Note: Vue has different update counts than React/Solid due to different reactivity - expect(updates).toBe(12) + expect(updates).toBe(9) }) test('nothing', async () => { @@ -202,7 +207,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // that needs to be done during a navigation. // Any change that increases this number should be investigated. // Note: Vue has different update counts than React/Solid due to different reactivity - expect(updates).toBe(9) + expect(updates).toBe(6) }) test('hover preload, then navigate, w/ async loaders', async () => { @@ -229,7 +234,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // that needs to be done during a navigation. // Any change that increases this number should be investigated. // Note: Vue has different update counts than React/Solid due to different reactivity - expect(updates).toBe(17) + expect(updates).toBe(7) }) test('navigate, w/ preloaded & async loaders', async () => { @@ -246,7 +251,7 @@ describe("Store doesn't update *too many* times during navigation", () => { // that needs to be done during a navigation. // Any change that increases this number should be investigated. // Note: Vue has different update counts than React/Solid due to different reactivity - expect(updates).toBe(10) + expect(updates).toBe(6) }) test('navigate, w/ preloaded & sync loaders', async () => { @@ -299,6 +304,6 @@ describe("Store doesn't update *too many* times during navigation", () => { // that needs to be done during a navigation. // Any change that increases this number should be investigated. // Note: Vue has different update counts than React/Solid due to different reactivity - expect(updates).toBe(2) + expect(updates).toBe(0) }) }) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 66e022dc6c..53ce64e72a 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1148,6 +1148,71 @@ importers: specifier: ^8.0.14 version: 8.0.14(@types/node@25.0.9)(esbuild@0.27.4)(jiti@2.7.0)(sass@1.97.2)(terser@5.37.0)(tsx@4.20.3)(yaml@2.9.0) + e2e/react-router/issue-7120: + dependencies: + '@tanstack/react-router': + specifier: workspace:* + version: link:../../../packages/react-router + react: + specifier: ^19.2.3 + version: 19.2.3 + react-dom: + specifier: ^19.2.3 + version: 19.2.3(react@19.2.3) + devDependencies: + '@playwright/test': + specifier: ^1.57.0 + version: 1.58.0 + '@tanstack/router-e2e-utils': + specifier: workspace:^ + version: link:../../e2e-utils + '@types/react': + specifier: ^19.2.8 + version: 19.2.9 + '@types/react-dom': + specifier: ^19.2.3 + version: 19.2.3(@types/react@19.2.9) + '@vitejs/plugin-react': + specifier: ^6.0.1 + version: 6.0.1(vite@8.0.14(@types/node@25.0.9)(esbuild@0.27.4)(jiti@2.7.0)(sass@1.97.2)(terser@5.37.0)(tsx@4.20.3)(yaml@2.9.0)) + vite: + specifier: ^8.0.14 + version: 8.0.14(@types/node@25.0.9)(esbuild@0.27.4)(jiti@2.7.0)(sass@1.97.2)(terser@5.37.0)(tsx@4.20.3)(yaml@2.9.0) + + e2e/react-router/issue-7457: + dependencies: + '@tanstack/react-router': + specifier: workspace:* + version: link:../../../packages/react-router + '@tanstack/router-plugin': + specifier: workspace:* + version: link:../../../packages/router-plugin + react: + specifier: ^19.2.3 + version: 19.2.3 + react-dom: + specifier: ^19.2.3 + version: 19.2.3(react@19.2.3) + devDependencies: + '@playwright/test': + specifier: ^1.57.0 + version: 1.58.0 + '@tanstack/router-e2e-utils': + specifier: workspace:^ + version: link:../../e2e-utils + '@types/react': + specifier: ^19.2.8 + version: 19.2.9 + '@types/react-dom': + specifier: ^19.2.3 + version: 19.2.3(@types/react@19.2.9) + '@vitejs/plugin-react': + specifier: ^6.0.1 + version: 6.0.1(vite@8.0.14(@types/node@25.0.9)(esbuild@0.27.4)(jiti@2.7.0)(sass@1.97.2)(terser@5.37.0)(tsx@4.20.3)(yaml@2.9.0)) + vite: + specifier: ^8.0.14 + version: 8.0.14(@types/node@25.0.9)(esbuild@0.27.4)(jiti@2.7.0)(sass@1.97.2)(terser@5.37.0)(tsx@4.20.3)(yaml@2.9.0) + e2e/react-router/js-only-file-based: dependencies: '@tailwindcss/vite': diff --git a/skills/bundle-size-optimization/SKILL.md b/skills/bundle-size-optimization/SKILL.md index d24538be69..3cdffc303e 100644 --- a/skills/bundle-size-optimization/SKILL.md +++ b/skills/bundle-size-optimization/SKILL.md @@ -113,6 +113,13 @@ Useful patterns: remove prod-only strings, remove unused exports, flatten wrappe Rolldown removes code only when unused and side-effect-free. Property reads may trigger getters; storage/global access can observe or throw. +### `isServer` DCE + +- `@tanstack/router-core/isServer` is conditionally exported: browser/client builds use `client.ts` (`isServer = false`), server builds use `server.ts` (`isServer = process.env.NODE_ENV === 'test' ? undefined : true`), and development/test builds use `development.ts` (`isServer = undefined`). The `undefined` value intentionally lets code fall back to `router.isServer` in tests and development. +- Do not assume `const onServer = isServer ?? this.isServer` will treeshake client-only code. In production browser bundles it can become a local `const onServer = false`, while guarded branches like `onServer && redirect.headers.set(...)` or `else if (onServer)` may still remain in emitted JS. +- Prefer inlining `isServer ?? this.isServer` at the server-only branch site instead of assigning it to a local alias. The inline form preserves test/development fallback and gives the browser build a better chance to fold the branch away. +- When a server-only branch should disappear from client bundles, inspect emitted JS in `benchmarks/bundle-size/dist//assets/*.js` to confirm it actually disappeared. + | Annotation | Valid | Unsafe | | ----------------------------------------- | --------------------------------------------------------------------------- | ---------------------------------------------------------------------------------- | | `/* @__PURE__ */ call()` | immediately before a call/new expression whose unused result can be dropped | declarations, property reads, setup, storage, DOM/history/listener code |