diff --git a/.changeset/clean-redirect-errors.md b/.changeset/clean-redirect-errors.md new file mode 100644 index 00000000000..1524e29f546 --- /dev/null +++ b/.changeset/clean-redirect-errors.md @@ -0,0 +1,5 @@ +--- +'@tanstack/router-core': patch +--- + +Render route errors when redirect target construction fails and let successor transactions own navigation and HMR presentation. diff --git a/e2e/react-start/hmr/tests/app.spec.ts b/e2e/react-start/hmr/tests/app.spec.ts index 69ce1d20f30..fe6dd99389f 100644 --- a/e2e/react-start/hmr/tests/app.spec.ts +++ b/e2e/react-start/hmr/tests/app.spec.ts @@ -684,7 +684,7 @@ test.describe('react-start hmr', () => { await expect(page.getByTestId('child')).toHaveText('child') }) - test('rolls back a failed route refresh and accepts the next HMR update', async ({ + test('publishes a failed route refresh and accepts the next HMR update', async ({ page, }) => { await page.goto('/child') @@ -712,7 +712,7 @@ test.describe('react-start hmr', () => { }, ) - await expect(page.getByTestId('crumb-/child')).toHaveText('Child') + await expect(page.getByTestId('crumb-/child')).toHaveText('Child Failed') await expect(page.getByTestId('root-message')).toHaveValue( 'preserved through failure', ) diff --git a/packages/react-router/src/Transitioner.tsx b/packages/react-router/src/Transitioner.tsx index cd20673ff16..244b2d021f6 100644 --- a/packages/react-router/src/Transitioner.tsx +++ b/packages/react-router/src/Transitioner.tsx @@ -29,26 +29,12 @@ export function Transitioner({ : undefined router.startTransition = (fn, expected) => - new Promise((resolve, reject) => { + new Promise((resolve) => { settleOwner(acknowledgement, false) acknowledgement.push(expected, resolve) t(router) - React.startTransition(() => { - try { - fn() - } catch (cause) { - if (acknowledgement[1 /* settle */] === resolve) { - acknowledgement.length = 0 - } - reject(cause) - } - }) + React.startTransition(fn) }) - if (process.env.NODE_ENV !== 'production') { - ;( - router as typeof router & { _cancelTransition?: () => void } - )._cancelTransition = () => settleOwner(acknowledgement, false) - } // Subscribe before canonicalizing so the initial URL has exactly one load. useLayoutEffect(() => { diff --git a/packages/react-router/tests/redirect.test.tsx b/packages/react-router/tests/redirect.test.tsx index 0c254fb098a..3cd6a3d88f4 100644 --- a/packages/react-router/tests/redirect.test.tsx +++ b/packages/react-router/tests/redirect.test.tsx @@ -75,6 +75,57 @@ describe('redirect', () => { expect(router.state.status).toBe('idle') }) + test('renders the source error boundary when building a redirect target fails', async () => { + const boom = new Error('redirect search failed') + const rootRoute = createRootRoute({ component: Outlet }) + const indexRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/', + component: () =>
Home
, + }) + const sourceRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/source', + beforeLoad: () => { + throw redirect({ + to: '/target', + search: () => { + throw boom + }, + }) + }, + errorComponent: ({ error }) => ( +
{error.message}
+ ), + }) + const targetRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/target', + component: () =>
Target
, + }) + const router = createRouter({ + routeTree: rootRoute.addChildren([ + indexRoute, + sourceRoute, + targetRoute, + ]), + history, + }) + + render() + expect(await screen.findByText('Home')).toBeInTheDocument() + + await act(() => router.navigate({ to: '/source' })) + + expect(await screen.findByTestId('source-error')).toHaveTextContent( + boom.message, + ) + expect(screen.queryByText('Home')).not.toBeInTheDocument() + expect(screen.queryByText('Target')).not.toBeInTheDocument() + expect(window.location.pathname).toBe('/source') + expect(router.state.status).toBe('idle') + }) + test('renders a root error after too many same-location redirects', async () => { const loader = vi.fn(() => { throw redirect({ to: '/' }) diff --git a/packages/react-router/tests/transitioner-render-ack.test.tsx b/packages/react-router/tests/transitioner-render-ack.test.tsx index 74bbe7d4857..535a1e0e71a 100644 --- a/packages/react-router/tests/transitioner-render-ack.test.tsx +++ b/packages/react-router/tests/transitioner-render-ack.test.tsx @@ -19,68 +19,6 @@ afterEach(() => { } cleanup() vi.useRealTimers() - vi.unstubAllEnvs() -}) - -test('a route lifecycle callback cannot strand a production navigation', async () => { - vi.stubEnv('NODE_ENV', 'production') - expect(process.env.NODE_ENV).toBe('production') - const rootRoute = createRootRoute({ component: Outlet }) - const indexRoute = createRoute({ - getParentRoute: () => rootRoute, - path: '/', - component: () =>
Index
, - }) - const error = new Error('onEnter failed') - const onEnter = vi.fn(() => { - throw error - }) - const nextRoute = createRoute({ - getParentRoute: () => rootRoute, - path: '/next', - onEnter, - component: () =>
Next
, - }) - const router = createRouter({ - routeTree: rootRoute.addChildren([indexRoute, nextRoute]), - history: createMemoryHistory({ initialEntries: ['/'] }), - }) - - render() - expect(await screen.findByText('Index')).toBeInTheDocument() - await waitFor(() => expect(router.state.status).toBe('idle')) - - const globalReport = vi.fn() - const preventGlobalReport = (event: ErrorEvent) => { - if (event.error === error) { - globalReport() - event.preventDefault() - } - } - window.addEventListener('error', preventGlobalReport) - testCleanups.push(() => { - window.removeEventListener('error', preventGlobalReport) - }) - - let settled = false - const navigation = router.navigate({ to: '/next' }).then( - () => { - settled = true - }, - () => { - settled = true - }, - ) - await waitFor(() => expect(settled).toBe(true)) - await navigation - - expect(onEnter).toHaveBeenCalledOnce() - expect(globalReport).not.toHaveBeenCalled() - expect(screen.getByText('Next')).toBeInTheDocument() - expect(router.state.status).toBe('idle') - - await router.navigate({ to: '/' }) - expect(await screen.findByText('Index')).toBeInTheDocument() }) test('same-location invalidation resolves after its refreshed DOM commits', async () => { diff --git a/packages/router-core/INTERNALS.md b/packages/router-core/INTERNALS.md index ec119aa09c3..d39c229768f 100644 --- a/packages/router-core/INTERNALS.md +++ b/packages/router-core/INTERNALS.md @@ -81,7 +81,7 @@ The main authorities are: | Match flight lease | Ownership keeping that loader work alive | | Pending session | One reveal/minimum-visible deadline and its current owner | | React acknowledgement slot | The one requested publication whose render may settle a transition | -| Refresh transaction | Its starting presentation, handoff, and ability to roll back | +| Refresh transaction | Forced rematerialization and an optional hydration handoff | | Request signal | Lifetime of one server request and any accepted SSR stream | | Accepted SSR stream response | Cleanup ownership transferred from the handler to the response body | @@ -400,8 +400,8 @@ lane uses that data or requires a loader generation. A discoverable same-ID flight may satisfy that requirement, including when `shouldReload` returns `true`. -Every eligible successful blocking loader task from an ordinary foreground -navigation or preload attempts cache admission immediately after loader +Every eligible successful blocking loader task from a foreground navigation, +development refresh, or preload attempts cache admission immediately after loader settlement. It does not wait for component readiness, whole-lane reduction, projection, commit, or render acknowledgement. The cache receives a non-terminal copy with merged context removed and an additional flight lease; @@ -413,9 +413,10 @@ normal navigation staleness and GC semantics rather than becoming synthetic preloads. Background loader candidates remain private until their exact transaction and -committed base authorize background publication. Development refresh foreground -generations also skip early cache admission because their candidate publication -may roll back. +committed base authorize background publication. A development refresh may +admit successful loader work while its lane is private, but refresh commit +discards every cache generation instead of preserving stale rematerialization +inputs. It is valid for cached loader data to have been produced under context from an older `beforeLoad` generation. Loaders are the cache boundary; guards are not. @@ -601,6 +602,13 @@ reduction. Returned and thrown redirects/not-founds normalize identically. Only an error invokes route `onError`; if `onError` throws, its value is normalized again and may itself become an error, not-found, or redirect. +Client and server lanes build a route redirect while its source route still owns +the outcome. Target-construction failures therefore become that route's error, +and a successful client target travels with the redirect so navigation does not +run functional updaters twice. +Client loader flights retain the raw redirect because a flight can be shared. +Each consuming lane materializes that redirect against its own location and +policy before exposing the lane-owned task outcome to settlement and reduction. Router cancellation and request abort bypass `onError`. Aborting the `AbortController` exposed to a loader is not by itself proof that the router discarded the work: a still-owned loader may fulfill or reject afterward, and @@ -610,6 +618,11 @@ proves request cancellation, while the already selected failure/control outcome proves that an aborted descendant is obsolete. The client calls a discarded non-result `canceled`, while the server calls it `skipped`; neither is a publishable terminal state. +The transaction controller exposed to client `context` and `beforeLoad` is +router-owned; route code may observe its signal but must not abort it. A canceled +client lane has therefore already lost writer authority to a successor. +Canceled client lanes release only their private work; the transaction that +superseded them owns presentation and completion. The client and server use the same settlement order and renderable-ancestor rules. @@ -828,10 +841,9 @@ session identity. Changing the selected boundary discards the old session. - `false` means core must finish without emitting `onRendered` or starting a pending minimum based on that publication. -A rejected acknowledgement aborts the transaction that owns the exact pending -session. The transaction remains responsible for restoring the committed lane -and releasing resources; stale acknowledgement failures cannot abort a -successor. +Framework transition wrappers are controlled adapters. Supersession settles an +older acknowledgement as `false`; core does not retain a prior presentation or +install a compensating restoration path for wrapper failure. React cannot await `React.startTransition` directly. Its adapter keeps one router-owned acknowledgement tuple, and `Matches` settles that tuple from a @@ -973,19 +985,19 @@ rematches with committed/cache reuse disabled so obsolete params, context, loader data, or projected assets cannot seed the refreshed lane. Selected cache and preload resources are detached before their controllers are aborted. -Unlike an ordinary foreground navigation, a development refresh does not admit -its loader successes to the cache while its candidate lane is private. The -refresh publication checkpoint is created only after lane execution; admitting -the new generation earlier would capture it as rollback cache and allow a failed -or superseded refresh to remain reusable. Acknowledged refresh data becomes -committed normally and may be cached later when displaced. +Before publication, a failed or superseded refresh leaves the painted +presentation alone because its candidate is still private. Once published, the +refresh is the accepted semantic and presentation generation even while exact +framework acknowledgement is pending. Publication uses the ordinary commit +ownership transfer, releases the replaced generation immediately, consumes any +obsolete hydration handoff, and retains no prior cache or presentation +checkpoint. -Refresh does not immediately discard the accepted committed lane or abort the -loader signals it still owns. The previous semantic lane, presentation, and -their resources remain available to the refresh transaction until the new -publication settles or rolls back. Settlement releases the replaced generation; -rollback restores it. The ability to roll back belongs to that refresh -transaction, not to a separate router-global owner. +A rapid successor refresh rematerializes again, settles the older framework +receipt as `false`, and owns convergence and public completion through the same +`_tx` and `awaitCurrent` chain as ordinary navigation. The older transaction +releases only private background work that did not transfer. No refresh has +authority to restore a predecessor or resolve a successor's commit promise. ## Speculative preloading @@ -1267,6 +1279,12 @@ identity check aborts its private work. If the published handoff becomes incompatible, its failed identity check retires the hydration controller and starts normal client loading from a fresh preflight. +Development refresh rematerializes instead of claiming the prefix. Its +transaction carries the existing handoff only until refreshed matches publish; +that one-way publication consumes the handoff and retires the hydration +controller before lifecycle reentrancy. A refresh superseded before publication +leaves the handoff for its rematerializing successor. + Generic framework `RouterClient` components signal streaming hydration completion after the hydration attempt settles, including rejection. This finally-style handoff allows bootstrap globals to be removed once the server @@ -1321,6 +1339,12 @@ algorithm here. - Do error/not-found selection, required ancestor readiness, component chunks, and descendant redirects still reduce to one final outcome? +- Are redirects materialized while their source route owns target-construction + errors, with shared flights retaining only the raw redirect and each lane + building its own target once? +- Does cancellation require both exact signal identity and proof that the signal + is aborted, and does a canceled lane release every private background + candidate without changing presentation? - Does pending or terminal publication retain the complete structural branch while rendering and outputs apply their relevant cutoff, including the selective-SSR hydration asset-prefix exception? diff --git a/packages/router-core/src/load-client.ts b/packages/router-core/src/load-client.ts index ffc6855e2b3..71c97a63ab0 100644 --- a/packages/router-core/src/load-client.ts +++ b/packages/router-core/src/load-client.ts @@ -163,18 +163,30 @@ const NOT_FOUND = 2 // Control outcomes stay contiguous so the hot path can test them together. const REDIRECTED = 3 const CANCELED = 4 +const CANCELED_OUTCOME: [kind: typeof CANCELED] = [CANCELED] -type LoaderOutcome = +type RedirectOutcome = [ + kind: typeof REDIRECTED, + redirect: AnyRedirect, + location?: ParsedLocation, +] + +type NonRedirectOutcome = | [kind: typeof SUCCESS, data: unknown] | [kind: typeof ERROR, error: unknown] | [kind: typeof NOT_FOUND, error: NotFoundError] - | [kind: typeof REDIRECTED, redirect: AnyRedirect] | [kind: typeof CANCELED] +type RawLoaderOutcome = + | NonRedirectOutcome + | [kind: typeof REDIRECTED, redirect: AnyRedirect] + +type LoaderOutcome = NonRedirectOutcome | RedirectOutcome + type IndexedOutcome = [index: number, outcome: LoaderOutcome, boundary?: number] export type LoaderFlight = [ - outcome: Promise, + outcome: Promise, controller: AbortController, leases: number, ] @@ -202,16 +214,11 @@ export type LoadTransaction = [ startedAt: number, done: Promise, /** - * Dev-only HMR refresh mode. Presence is the mode flag; a refresh always - * carries the presentation it started from and its optional hydration - * handoff. While a publication awaits acknowledgement, its rollback lives - * with the transaction that owns the publication. + * Dev-only HMR refresh mode. Presence forces successor rematerialization + * until this publication is acknowledged. The optional hydration handoff is + * retired when the refresh publishes. */ - refresh?: [ - presentation: Array, - handoff: NonNullable | undefined, - rollback?: () => boolean, - ], + refresh?: [handoff: NonNullable | undefined], ] export type PendingSession = [ @@ -228,15 +235,6 @@ type CoordinatorRouter = AnyRouter & { /** Active speculative lanes retained for cancellation, invalidation, and cache clearing. */ _preloads?: Map> _refreshNextLoad?: boolean - _cancelTransition?: () => void -} - -type PublicationCheckpoint = { - previousMatches: Array - previousPresentation: Array - previousCache: Map - commitPromise: CoordinatorRouter['_commitPromise'] - published: boolean } type LoaderTask = [ @@ -256,7 +254,6 @@ type BackgroundLoaderTask = [ type ExecuteLaneOptions = [ controller: AbortController, redirects: number, - isCurrent: () => boolean, base: Array, preload?: boolean, sync?: boolean, @@ -265,9 +262,7 @@ type ExecuteLaneOptions = [ onReady?: () => void, ] -type ControlOutcome = - | [kind: typeof REDIRECTED, redirect: AnyRedirect] - | [kind: typeof CANCELED] +type ControlOutcome = RedirectOutcome | [kind: typeof CANCELED] type LaneResult = ProjectedLane | ControlOutcome @@ -301,7 +296,7 @@ function normalize( value: unknown, rejected: boolean, routeId?: string, -): LoaderOutcome { +): RawLoaderOutcome { if (isRedirect(value)) { return [REDIRECTED, value] } @@ -315,7 +310,7 @@ function normalize( return rejected ? [ERROR, value] : [SUCCESS, value] } -function normalizeError(route: AnyRoute, cause: unknown): LoaderOutcome { +function normalizeError(route: AnyRoute, cause: unknown): RawLoaderOutcome { let outcome = normalize(cause, true, route.id) if (outcome[0 /* kind */] !== ERROR) { return outcome @@ -329,26 +324,22 @@ function normalizeError(route: AnyRoute, cause: unknown): LoaderOutcome { } function normalizeLaneError( + router: AnyRouter, + lane: Lane, route: AnyRoute, cause: unknown, options: ExecuteLaneOptions, ): LoaderOutcome { - if ( - options[0 /* controller */].signal.aborted || - !options[2 /* isCurrent */]() - ) { - options[0 /* controller */].abort() - return [CANCELED] + if (options[0 /* controller */].signal.aborted) { + return CANCELED_OUTCOME } - return normalizeError(route, cause) -} - -export function navigateFrom(router: AnyRouter, location: ParsedLocation) { - return (opts: any) => - router.navigate({ - ...opts, - _fromLocation: location, - }) + return materializeRedirect( + router, + lane, + route, + normalizeError(route, cause), + options, + ) } async function contextualize( @@ -361,8 +352,8 @@ async function contextualize( ): Promise { const [location, matches] = lane const signal = options[0 /* controller */].signal - const preload = !!options[4 /* preload */] - for (let index = options[7 /* resolvedPrefix */] ?? 0; index < end; index++) { + const preload = !!options[3 /* preload */] + for (let index = options[6 /* resolvedPrefix */] ?? 0; index < end; index++) { const match = matches[index]! const route = getRoute(router, match) @@ -374,7 +365,11 @@ async function contextualize( const common = { params: match.params, location, - navigate: navigateFrom(router, location), + navigate: (opts: any) => + router.navigate({ + ...opts, + _fromLocation: location, + }), buildLocation: router.buildLocation, cause: preload ? ('preload' as const) : match.cause, abortController: options[0 /* controller */], @@ -400,16 +395,18 @@ async function contextualize( match.context = context } catch (cause) { releaseFlight(router, match) - return [index, normalizeLaneError(route, cause, options)] + return [index, normalizeLaneError(router, lane, route, cause, options)] } - if (signal.aborted || !options[2 /* isCurrent */]()) { - options[0 /* controller */].abort() - return [index, [CANCELED]] + if (signal.aborted) { + return [index, CANCELED_OUTCOME] } const validationError = match.paramsError ?? match.searchError if (validationError !== undefined) { releaseFlight(router, match) - return [index, normalizeLaneError(route, validationError, options)] + return [ + index, + normalizeLaneError(router, lane, route, validationError, options), + ] } const beforeLoad = route.options.beforeLoad if (!beforeLoad) { @@ -436,16 +433,21 @@ async function contextualize( const previousStatus = match.status if (index >= retainedEnd) { match.status = 'pending' - options[8 /* onReady */]?.() + options[7 /* onReady */]?.() } try { setFetching(router, match, 'beforeLoad', options[0 /* controller */]) const result = await waitFor(beforeLoad(beforeLoadContext), signal) - if (!options[2 /* isCurrent */]()) { - options[0 /* controller */].abort() - return [index, [CANCELED]] + if (signal.aborted) { + return [index, CANCELED_OUTCOME] } - const outcome = normalize(result, false, route.id) + const outcome = materializeRedirect( + router, + lane, + route, + normalize(result, false, route.id), + options, + ) if (outcome[0 /* kind */] !== SUCCESS) { releaseFlight(router, match) return [index, outcome] @@ -456,7 +458,7 @@ async function contextualize( } } catch (cause) { releaseFlight(router, match) - return [index, normalizeLaneError(route, cause, options)] + return [index, normalizeLaneError(router, lane, route, cause, options)] } finally { if (match.status === 'pending') { match.status = previousStatus @@ -483,7 +485,6 @@ function releaseOwnedFlight( if ( current && !current[0 /* controller */].signal.aborted && - !(process.env.NODE_ENV !== 'production' && current[6 /* refresh */]) && !current[3 /* matches */].includes(match) && current[3 /* matches */].some((candidate) => candidate.id === match.id) && current[3 /* matches */].some( @@ -523,9 +524,6 @@ function transferMatchResources( deferSameIdFlight && flight?.[2 /* leases */] === 1 && router._flights?.get(match.id) === flight && - !( - process.env.NODE_ENV !== 'production' && router._tx?.[6 /* refresh */] - ) && next?.some((candidate) => candidate.id === match.id) ) { // The successor has not made its same-ID reload decision yet. @@ -582,7 +580,11 @@ function getLoaderContext( return { params: match.params, location, - navigate: navigateFrom(router, location), + navigate: (opts: any) => + router.navigate({ + ...opts, + _fromLocation: location, + }), cause: preload ? ('preload' as const) : match.cause, abortController: controller, preload, @@ -601,12 +603,12 @@ async function loadResource( route: AnyRoute, loader: RouteLoaderFn | undefined, parentMatchPromise: Promise | undefined, - preload: boolean, - owner: AbortController, + options: ExecuteLaneOptions, ): Promise { + const owner = options[0 /* controller */] const signal = owner.signal if (signal.aborted) { - return [CANCELED] + return CANCELED_OUTCOME } if (!loader) { return [SUCCESS, undefined] @@ -628,7 +630,7 @@ async function loadResource( route, controller, parentMatchPromise, - preload, + !!options[3 /* preload */], ), ), ) @@ -636,7 +638,7 @@ async function loadResource( (value) => normalize(value, false, route.id), (cause) => normalize(cause, true, route.id), ) - .then((result): LoaderOutcome => { + .then((result): RawLoaderOutcome => { // The registry controls discovery; leases keep current consumers // sharing the same terminal outcome. if ( @@ -659,13 +661,19 @@ async function loadResource( } match._flight = flight match.abortController = flight[1 /* controller */] - return await waitFor(flight[0 /* outcome */], signal) + return materializeRedirect( + router, + lane, + route, + await waitFor(flight[0 /* outcome */], signal), + options, + ) } catch (cause) { - if (cause !== signal) { + if (cause !== signal || !signal.aborted) { throw cause } releaseFlight(router, match) - return [CANCELED] + return CANCELED_OUTCOME } finally { setFetching(router, match, false, owner) } @@ -748,7 +756,7 @@ function createLoaderTask( ): Promise { const match = lane[1 /* matches */][index]! const route = getRoute(router, match) - const preload = !!options[4 /* preload */] + const preload = !!options[3 /* preload */] const plannedCacheMatch = router._cache.get(match.id) let configured let reload = false @@ -769,9 +777,8 @@ function createLoaderTask( ), ) } - if (!options[2 /* isCurrent */]()) { - options[0 /* controller */].abort() - reloadFailure = [CANCELED] + if (options[0 /* controller */].signal.aborted) { + reloadFailure = CANCELED_OUTCOME } } if (!reloadFailure) { @@ -779,7 +786,7 @@ function createLoaderTask( reload = true } else { const staleAge = - options[4 /* preload */] || match.preload + options[3 /* preload */] || match.preload ? (route.options.preloadStaleTime ?? router.options.defaultPreloadStaleTime ?? 30_000) @@ -789,9 +796,9 @@ function createLoaderTask( configured || (configured === undefined && Date.now() - match.updatedAt >= staleAge && - (options[6 /* forceStaleReload */] || + (options[5 /* forceStaleReload */] || match.cause === 'enter' || - options[3 /* base */].some( + options[2 /* base */].some( (candidate) => candidate.routeId === match.routeId && candidate.id !== match.id, @@ -802,7 +809,7 @@ function createLoaderTask( } catch (cause) { match.invalid = true releaseFlight(router, match) - reloadFailure = normalizeLaneError(route, cause, options) + reloadFailure = normalizeLaneError(router, lane, route, cause, options) } const routeLoader = route.options.loader const loader = @@ -827,7 +834,7 @@ function createLoaderTask( reload && match.status === 'success' && !preload && - !options[5 /* sync */] && + !options[4 /* sync */] && ((typeof routeLoader === 'function' ? undefined : routeLoader?.staleReloadMode) ?? @@ -836,7 +843,7 @@ function createLoaderTask( const loaded = reload && (!preload || route.options.preload !== false) const blocking = loaded && !background && (match.status !== 'success' || !!routeLoader) - const onReady = index >= retainedEnd ? options[8 /* onReady */] : undefined + const onReady = index >= retainedEnd ? options[7 /* onReady */] : undefined const onLazyReady = route.lazyFn && route._lazy !== true ? onReady : undefined if (loaded && !routeLoader) { match.invalid = false @@ -859,7 +866,7 @@ function createLoaderTask( if (!loaded) { match.isFetching = false } - const rawOutcome = reloadFailure + const loaderOutcome = reloadFailure ? Promise.resolve(reloadFailure) : !blocking ? Promise.resolve([SUCCESS, match.loaderData]) @@ -870,24 +877,15 @@ function createLoaderTask( route, loader, semanticParent, - preload, - options[0 /* controller */], + options, ) - const outcome = rawOutcome.then((result) => { + const outcome = loaderOutcome.then((result) => { if (blocking) { settleInto(match, result, preload) if (result[0 /* kind */] === SUCCESS) { // A settled generation can outlive its lane without keeping unresolved - // navigation work alive. Refresh generations remain private for rollback. - if ( - routeLoader && - !options[0 /* controller */].signal.aborted && - !( - process.env.NODE_ENV !== 'production' && - !preload && - router._tx?.[6 /* refresh */] - ) - ) { + // navigation work alive. + if (routeLoader && !options[0 /* controller */].signal.aborted) { cacheLoaderMatch(router, match, plannedCacheMatch) } // A route is renderable only after both its data and normal component @@ -900,24 +898,30 @@ function createLoaderTask( return result }) - const rawChunkFailure = waitFor( + const chunkOutcome = waitFor( Promise.resolve().then(() => loadRouteChunk(route, undefined, onLazyReady)), options[0 /* controller */].signal, ).then( () => undefined, - (cause): IndexedOutcome => [ - index, - normalizeLaneError(route, cause, options), - ], + (cause): IndexedOutcome | undefined => + lane[1 /* matches */].some( + (candidate, candidateIndex) => + candidateIndex <= index && + (candidate.status === 'error' || + candidate.status === 'notFound' || + candidate._notFound), + ) + ? undefined + : [index, normalizeLaneError(router, lane, route, cause, options)], ) - const chunkFailure = rawChunkFailure.then((failure) => + const chunkFailure = chunkOutcome.then((failure) => outcome.then((result) => { if ( blocking && !failure && result[0 /* kind */] === SUCCESS && match.status === 'pending' && - options[2 /* isCurrent */]() + !options[0 /* controller */].signal.aborted ) { match.status = 'success' onReady?.() @@ -944,8 +948,7 @@ function createLoaderTask( route, loader, semanticParent, - false, - options[0 /* controller */], + options, ).then((result) => { match.isFetching = false settleInto(candidate, result, false) @@ -980,14 +983,14 @@ async function getNotFoundBoundary( } for (let i = index; i >= 0; i--) { const route = getRoute(router, matches[i]!) - const loading = loadRouteChunk(route, false) - if (loading) { - try { + try { + const loading = loadRouteChunk(route, false) + if (loading) { await waitFor(loading, signal) - } catch (cause) { - if (cause === signal) { - throw cause - } + } + } catch (cause) { + if (cause === signal && signal.aborted) { + throw cause } } if (route.options.notFoundComponent) { @@ -1055,12 +1058,50 @@ async function settleTasks( return serialFailure ?? loaderFailure } +function materializeRedirect( + router: AnyRouter, + lane: Lane, + route: AnyRoute, + outcome: RawLoaderOutcome, + options: ExecuteLaneOptions, + failed?: true, +): LoaderOutcome { + while (outcome[0 /* kind */] === REDIRECTED) { + const redirect = outcome[1 /* redirect */] + if ( + redirect.options.reloadDocument + ? options[3 /* preload */] + : options[1 /* redirects */] >= 20 + ) { + return outcome + } + try { + if (redirect.options.href && redirect.options.reloadDocument) { + router.resolveRedirect(redirect) + return outcome + } + return [ + REDIRECTED, + redirect, + router.buildLocation({ + ...redirect.options, + _fromLocation: lane[0 /* location */], + _includeValidateSearch: true, + }), + ] + } catch (cause) { + outcome = failed ? [ERROR, cause] : normalizeError(route, cause) + failed = true + } + } + return outcome +} + async function reduceLane( router: AnyRouter, lane: ContextualizedLane, tasks: Array, controller: AbortController, - redirects: number, settlement: Promise, onReady?: () => void, ): Promise { @@ -1113,7 +1154,7 @@ async function reduceLane( if ( outcome[0 /* kind */] !== REDIRECTED || outcome[1 /* redirect */].options.reloadDocument || - redirects < 20 + outcome[2 /* location */] ) { discardBackground(router, lane) return outcome as ControlOutcome @@ -1169,9 +1210,9 @@ async function reduceLane( controller.signal, ) } catch (cause) { - if (cause === controller.signal) { + if (cause === controller.signal && controller.signal.aborted) { discardBackground(router, lane) - return [CANCELED] + return CANCELED_OUTCOME } } if (!outcome) { @@ -1227,7 +1268,7 @@ export async function projectLane( match.styles = head?.styles match.scripts = scripts } catch (cause) { - if (cause === signal) { + if (cause === signal && signal.aborted) { break } console.error(cause) @@ -1247,111 +1288,112 @@ async function executeClientLane( options: ExecuteLaneOptions, ): Promise { const matched = [location, matches as Array] as MatchedLane - const presented = router.stores.matches.get() - let plannedBoundary = matches.findIndex((match) => match._notFound) - if (router.options.notFoundMode !== 'root' && plannedBoundary >= 0) { - const boundary = await getNotFoundBoundary( - router, - matched[1 /* matches */], - undefined, - options[0 /* controller */].signal, - plannedBoundary, - ) - if (boundary !== plannedBoundary) { - matches[plannedBoundary]!._notFound = undefined - matches[boundary]!._notFound = true - } - plannedBoundary = boundary - } - let end = plannedBoundary < 0 ? matches.length : plannedBoundary + 1 - let retainedEnd = 0 - while (retainedEnd < end && retainedEnd !== plannedBoundary) { - const match = matches[retainedEnd]! - const committed = options[3 /* base */][retainedEnd] - const visible = presented[retainedEnd] - if ( - committed?.id !== match.id || - committed.status !== 'success' || - committed._notFound || - match.preload || - visible?.id !== match.id || - visible.status !== 'success' || - visible._notFound - ) { - break - } - retainedEnd++ - } - const tasks: Array = [] - const start = options[7 /* resolvedPrefix */] ?? 0 - let semanticParent = start - ? Promise.resolve(matched[1 /* matches */][start - 1]!) - : undefined - const planSuccessfulLane = () => { - for (let index = start; index < end; index++) { - if (options[0 /* controller */].signal.aborted) { - break - } - semanticParent = createLoaderTask( - router, - matched as ContextualizedLane, - index, - tasks, - semanticParent, - options, - retainedEnd, - ) - } - } - // From here on `matched` is contextualized: `contextualize` communicates - // through mutation plus a failure return, so the phase brand is asserted at - // the two use sites below rather than granted by a (byte-costing) return. - const failure = await contextualize( - router, - matched, - options, - end, - planSuccessfulLane, - retainedEnd, - ) - if (failure) { - options[5 /* sync */] = true - end = failure[0 /* index */] - if (failure[1 /* outcome */][0 /* kind */] === NOT_FOUND) { - failure[2 /* boundary */] = await getNotFoundBoundary( + const signal = options[0 /* controller */].signal + let reduced: ReducedLane | ControlOutcome + try { + const presented = router.stores.matches.get() + let plannedBoundary = matches.findIndex((match) => match._notFound) + if (router.options.notFoundMode !== 'root' && plannedBoundary >= 0) { + const boundary = await getNotFoundBoundary( router, matched[1 /* matches */], - failure, - options[0 /* controller */].signal, + undefined, + signal, + plannedBoundary, ) - end = Math.min(end, failure[2 /* boundary */] + 1) - } else if (failure[1 /* outcome */][0 /* kind */] >= REDIRECTED) { - end = 0 - } - planSuccessfulLane() - } - if (options[2 /* isCurrent */]() && !options[4 /* preload */]) { - const abort: Array = [] - for (const [id, flight] of router._flights ?? []) { - if (!flight[2 /* leases */]) { - router._flights!.delete(id) - abort.push(flight[1 /* controller */]) + if (boundary !== plannedBoundary) { + matches[plannedBoundary]!._notFound = undefined + matches[boundary]!._notFound = true } + plannedBoundary = boundary + } + let end = plannedBoundary < 0 ? matches.length : plannedBoundary + 1 + let retainedEnd = 0 + while (retainedEnd < end && retainedEnd !== plannedBoundary) { + const match = matches[retainedEnd]! + const committed = options[2 /* base */][retainedEnd] + const visible = presented[retainedEnd] + if ( + committed?.id !== match.id || + committed.status !== 'success' || + committed._notFound || + match.preload || + visible?.id !== match.id || + visible.status !== 'success' || + visible._notFound + ) { + break + } + retainedEnd++ } - for (const controller of abort) { - controller.abort() + const tasks: Array = [] + const start = options[6 /* resolvedPrefix */] ?? 0 + let semanticParent = start + ? Promise.resolve(matched[1 /* matches */][start - 1]!) + : undefined + const planSuccessfulLane = () => { + for (let index = start; index < end; index++) { + if (signal.aborted) { + break + } + semanticParent = createLoaderTask( + router, + matched as ContextualizedLane, + index, + tasks, + semanticParent, + options, + retainedEnd, + ) + } + } + // From here on `matched` is contextualized: `contextualize` communicates + // through mutation plus a failure return, so the phase brand is asserted at + // the two use sites below rather than granted by a (byte-costing) return. + const failure = await contextualize( + router, + matched, + options, + end, + planSuccessfulLane, + retainedEnd, + ) + if (failure) { + options[4 /* sync */] = true + end = failure[0 /* index */] + if (failure[1 /* outcome */][0 /* kind */] === NOT_FOUND) { + const boundary = await getNotFoundBoundary( + router, + matched[1 /* matches */], + failure, + signal, + ) + failure[2 /* boundary */] = boundary + end = Math.min(end, boundary + 1) + } else if (failure[1 /* outcome */][0 /* kind */] >= REDIRECTED) { + end = 0 + } + planSuccessfulLane() + } + if (!signal.aborted && !options[3 /* preload */]) { + const abort: Array = [] + for (const [id, flight] of router._flights ?? []) { + if (!flight[2 /* leases */]) { + router._flights!.delete(id) + abort.push(flight[1 /* controller */]) + } + } + for (const controller of abort) { + controller.abort() + } } - } - let reduced: ReducedLane | ControlOutcome - try { const reduction = reduceLane( router, matched as ContextualizedLane, tasks, options[0 /* controller */], - options[1 /* redirects */], settleTasks(tasks, failure, matched[2 /* background */]), - options[8 /* onReady */], + options[7 /* onReady */], ) if (matched[2 /* background */]?.length) { matched[3 /* backgroundSettlement */] = settleTasks( @@ -1370,6 +1412,9 @@ async function executeClientLane( reduced = await reduction } catch (cause) { discardBackground(router, matched) + if (cause === signal && signal.aborted) { + return CANCELED_OUTCOME + } throw cause } if (isControl(reduced)) { @@ -1378,9 +1423,9 @@ async function executeClientLane( return projectLane( router, reduced, - options[0 /* controller */].signal, - options[7 /* resolvedPrefix */] === reduced[1 /* matches */].length - ? options[7 /* resolvedPrefix */] + signal, + options[6 /* resolvedPrefix */] === reduced[1 /* matches */].length + ? options[6 /* resolvedPrefix */] : 0, ) } @@ -1472,36 +1517,34 @@ function offerPending(router: CoordinatorRouter, tx: LoadTransaction): void { offered[index]!.status = 'pending' const ack = (session[4 /* ack */] = router .startTransition(() => router.stores.setMatches(offered), offered) - .then( - (rendered) => { - if ( - rendered && - router._pending === session && - session![4 /* ack */] === ack && - !session![2 /* deadline */] - ) { - session![2 /* deadline */] = Date.now() + min - } - return rendered - }, - () => { - if (router._pending?.[4 /* ack */] === ack) { - tx[0 /* controller */].abort() - } - return false - }, - )) + .then((rendered) => { + if ( + rendered && + router._pending === session && + session![4 /* ack */] === ack && + !session![2 /* deadline */] + ) { + session![2 /* deadline */] = Date.now() + min + } + return rendered + })) return } } /** - * Cancels pending UI timing when the current load replaces its presentation. - * An obsolete load cannot clear the fallback that remains painted above it. + * Cancels pending UI timing unless the current successor can take over the + * same boundary that remains painted. */ function finishPending(router: CoordinatorRouter, tx: LoadTransaction): void { - if (router._tx === tx) { - clearTimeout(router._pending?.[3 /* revealTimer */]) + const session = router._pending + if ( + router._tx === tx || + !router._tx?.[3 /* matches */].some( + (match) => match.id === session?.[1 /* boundaryId */], + ) + ) { + clearTimeout(session?.[3 /* revealTimer */]) router._pending = undefined } } @@ -1545,11 +1588,6 @@ function publishMatches( router.stores.setMatches(matches) } -function discardLane(router: AnyRouter, lane: ProjectedLane): void { - transferMatchResources(router, lane[1 /* matches */]) - discardBackground(router, lane) -} - function commitMatches( router: CoordinatorRouter, tx: LoadTransaction, @@ -1566,46 +1604,48 @@ function commitMatches( } const cut = _getRenderedMatches(matches).length const cached = new Map() - const now = Date.now() - for (const match of [...previous, ...previousCached.values()]) { - // Rendered-prefix ids and settled successes anywhere in the lane are - // authoritative: retaining an older same-id generation would shadow them - // at the next planning pass. Unsettled beyond-boundary matches are not — - // they must not evict a newer same-id preload. - if ( - match.status !== 'success' || - matches.some( - (candidate, index) => - candidate.id === match.id && - (index < cut || candidate.status === 'success'), + if (process.env.NODE_ENV === 'production' || !tx[6 /* refresh */]) { + const now = Date.now() + for (const match of [...previous, ...previousCached.values()]) { + // Rendered-prefix ids and settled successes anywhere in the lane are + // authoritative: retaining an older same-id generation would shadow them + // at the next planning pass. Unsettled beyond-boundary matches are not — + // they must not evict a newer same-id preload. + if ( + match.status !== 'success' || + matches.some( + (candidate, index) => + candidate.id === match.id && + (index < cut || candidate.status === 'success'), + ) + ) { + continue + } + const work = match as WorkMatch + const route = getRoute(router, work) + if ( + !route.options.loader || + now - match.updatedAt >= + (match.preload + ? (route.options.preloadGcTime ?? + router.options.defaultPreloadGcTime ?? + 300_000) + : (route.options.gcTime ?? router.options.defaultGcTime ?? 300_000)) + ) { + continue + } + cached.set( + match.id, + previousCached.get(match.id) === match + ? match + : ({ + ...match, + _flight: undefined, + isFetching: false, + context: {}, + } as WorkMatch), ) - ) { - continue - } - const work = match as WorkMatch - const route = getRoute(router, work) - if ( - !route.options.loader || - now - match.updatedAt >= - (match.preload - ? (route.options.preloadGcTime ?? - router.options.defaultPreloadGcTime ?? - 300_000) - : (route.options.gcTime ?? router.options.defaultGcTime ?? 300_000)) - ) { - continue } - cached.set( - match.id, - previousCached.get(match.id) === match - ? match - : ({ - ...match, - _flight: undefined, - isFetching: false, - context: {}, - } as WorkMatch), - ) } // The lane becomes committed before publication can synchronously reenter. tx[3 /* matches */] = [] @@ -1616,155 +1656,13 @@ function commitMatches( [...previousCached.values(), ...previous], [...matches, ...cached.values()], ) - runRouteLifecycle(router, previous, matches, () => router._tx === tx) -} - -function commitRefreshMatches( - router: CoordinatorRouter, - tx: LoadTransaction, - matches: LaneMatches<'projected'>, - checkpoint: PublicationCheckpoint, -): void { - const previous = router._committed - const previousCached = router._cache - for (const match of matches) { - match.preload = false - } - const cached = new Map() - // Delay releasing the previous owners until the HMR render is acknowledged. - // Old generations must not become reusable cache entries after refresh. - tx[3 /* matches */] = [] - router._cache = cached - checkpoint.previousMatches = previous - checkpoint.previousCache = previousCached - checkpoint.published = true - publishMatches(router, matches) - if (!checkpoint.published || router._tx !== tx) { - return - } - runRouteLifecycle(router, previous, matches, () => router._tx === tx) -} - -function settlePublication( - router: CoordinatorRouter, - checkpoint: PublicationCheckpoint, -): void { - if (!checkpoint.published) { - return - } - checkpoint.published = false - transferMatchResources( - router, - [...checkpoint.previousCache.values(), ...checkpoint.previousMatches], - [...router._cache.values(), ...router._committed], - ) -} - -function rollbackPublication( - router: CoordinatorRouter, - tx: LoadTransaction, - lane: ProjectedLane, - checkpoint: PublicationCheckpoint, -): boolean { - if ( - !checkpoint.published || - router._tx !== tx || - router._committed !== lane[1 /* matches */] - ) { - settlePublication(router, checkpoint) - return false - } - - const discarded = [...router._cache.values(), ...router._committed] - const restored = [ - ...checkpoint.previousCache.values(), - ...checkpoint.previousMatches, - ] - router._cache = checkpoint.previousCache - router._committed = checkpoint.previousMatches - checkpoint.published = false - - for (const match of discarded as Array) { - if ( - !restored.includes(match) && - match._flight && - router._flights?.get(match.id) === match._flight - ) { - router._flights.delete(match.id) - } - } - - finishPending(router, tx) - router.batch(() => { - router.stores.status.set('idle') - router.stores.setMatches(checkpoint.previousPresentation) - }) - tx[0 /* controller */].abort() - transferMatchResources(router, discarded, restored) - discardBackground(router, lane) - if (router._tx === tx && router._commitPromise === checkpoint.commitPromise) { - router._commitPromise?.resolve() - router._commitPromise = undefined - } - return true -} - -async function transitionRefresh( - router: CoordinatorRouter, - tx: LoadTransaction, - lane: ProjectedLane, - changeInfo: ReturnType, -): Promise { - const refresh = tx[6 /* refresh */]! - const checkpoint: PublicationCheckpoint = { - previousMatches: router._committed, - previousPresentation: refresh[0 /* presentation */], - previousCache: router._cache, - commitPromise: router._commitPromise, - published: false, - } - const commit = () => { - finishPending(router, tx) - refresh[2 /* rollback */] = rollback - commitRefreshMatches(router, tx, lane[1 /* matches */], checkpoint) - if (!checkpoint.published || router._tx !== tx) { - return - } - router.emit({ type: 'onLoad', ...changeInfo }) - if (router._tx === tx) { - router.emit({ type: 'onBeforeRouteMount', ...changeInfo }) - } - } - const rollback = () => { - if (refresh[2 /* rollback */] === rollback) { - refresh[2 /* rollback */] = undefined - } - const restored = rollbackPublication(router, tx, lane, checkpoint) - router._cancelTransition?.() - return restored - } - try { - const rendered = await router.startTransition(commit, lane[1 /* matches */]) - if (refresh[2 /* rollback */] === rollback) { - refresh[2 /* rollback */] = undefined - } - if (checkpoint.published) { - const handoff = refresh[1 /* handoff */] - if (handoff && router._handoff === handoff) { - handoff[1 /* finish */]() - } - if (router._tx === tx) { - tx[6 /* refresh */] = undefined - } - } - settlePublication(router, checkpoint) - return rendered - } catch (cause) { - if (rollback()) { - return + if (process.env.NODE_ENV !== 'production') { + const handoff = tx[6 /* refresh */]?.[0 /* handoff */] + if (handoff && router._handoff === handoff) { + handoff[1 /* finish */]() } - throw cause } + runRouteLifecycle(router, previous, matches, tx) } async function awaitCurrent( @@ -1784,35 +1682,44 @@ async function awaitCurrent( async function followRedirect( router: CoordinatorRouter, tx: LoadTransaction, - redirect: AnyRedirect, + outcome: RedirectOutcome, ): Promise { - await router.navigate({ - ...redirect.options, - replace: true, - ignoreBlocker: true, - _redirects: tx[1 /* redirects */] + 1, - } as any) -} - -function restoreCommitted( - router: CoordinatorRouter, - tx: LoadTransaction, -): void { - finishPending(router, tx) - tx[0 /* controller */].abort() - transferMatchResources(router, tx[3 /* matches */]) - tx[3 /* matches */] = [] - if (router._tx !== tx) { + const redirect = outcome[1 /* redirect */] + const location = outcome[2 /* location */] + if (!location) { + await router.navigate({ + ...redirect.options, + replace: true, + ignoreBlocker: true, + } as any) return } - router.batch(() => { - router.stores.status.set('idle') - router.stores.setMatches(router._committed) - }) - if (router._tx === tx) { - router._commitPromise?.resolve() - router._commitPromise = undefined + if (redirect.options.reloadDocument) { + await router.navigate({ + href: location.publicHref, + reloadDocument: true, + replace: true, + ignoreBlocker: true, + } as any) + return } + ;(location as ParsedLocation & { _redirects?: number })._redirects = + tx[1 /* redirects */] + 1 + router._pendingLocation = location + const committed = router.commitLocation({ + ...location, + viewTransition: redirect.options.viewTransition, + replace: true, + resetScroll: redirect.options.resetScroll, + hashScrollIntoView: redirect.options.hashScrollIntoView, + ignoreBlocker: true, + }) + queueMicrotask(() => { + if (router._pendingLocation === location) { + router._pendingLocation = undefined + } + }) + await committed } async function runBackground( @@ -1838,7 +1745,6 @@ async function runBackground( lane, tasks, tx[0 /* controller */], - tx[1 /* redirects */], settlement, ) } catch (cause) { @@ -1852,7 +1758,7 @@ async function runBackground( router._tx === tx && router._committed === base ) { - await followRedirect(router, tx, reduced[1 /* redirect */]) + await followRedirect(router, tx, reduced) } return } @@ -1887,7 +1793,6 @@ async function runClientTransaction( const options: ExecuteLaneOptions = [ tx[0 /* controller */], tx[1 /* redirects */], - () => router._tx === tx && !!tx[3 /* matches */].length, router._committed, undefined, sync, @@ -1903,32 +1808,38 @@ async function runClientTransaction( ) if (isControl(result)) { - if (result[0 /* kind */] === REDIRECTED && router._tx === tx) { - if (result[1 /* redirect */].options.reloadDocument) { - finishPending(router, tx) - } - transferMatchResources(router, tx[3 /* matches */]) - tx[3 /* matches */] = [] - if (router._tx === tx) { - if (process.env.NODE_ENV !== 'production' && tx[6 /* refresh */]) { - router._refreshNextLoad = true - } - await followRedirect(router, tx, result[1 /* redirect */]) - } - } else { - restoreCommitted(router, tx) + const follow = result[0 /* kind */] === REDIRECTED && router._tx === tx + if (!follow || result[1 /* redirect */].options.reloadDocument) { + finishPending(router, tx) + } + transferMatchResources(router, tx[3 /* matches */]) + tx[3 /* matches */] = [] + if (!follow) { + return + } + if (router._tx !== tx) { + finishPending(router, tx) + return } + if (process.env.NODE_ENV !== 'production' && tx[6 /* refresh */]) { + router._refreshNextLoad = true + } + await followRedirect(router, tx, result) return } if (router._tx !== tx) { - discardLane(router, result) + finishPending(router, tx) + transferMatchResources(router, result[1 /* matches */]) + discardBackground(router, result) return } // Only an acknowledged fallback owns a minimum. Recheck at the commit // boundary because native view transitions can defer their update callback. await awaitPendingMinimum(router, tx) if (router._tx !== tx) { - discardLane(router, result) + finishPending(router, tx) + transferMatchResources(router, result[1 /* matches */]) + discardBackground(router, result) return } const toLocation = tx[2 /* location */] @@ -1939,12 +1850,16 @@ async function runClientTransaction( const background = result[2 /* background */] await router.startViewTransition(async () => { if (router._tx !== tx) { - discardLane(router, result) + finishPending(router, tx) + transferMatchResources(router, result[1 /* matches */]) + discardBackground(router, result) return } await awaitPendingMinimum(router, tx) if (router._tx !== tx) { - discardLane(router, result) + finishPending(router, tx) + transferMatchResources(router, result[1 /* matches */]) + discardBackground(router, result) return } const commit = () => { @@ -1958,24 +1873,20 @@ async function runClientTransaction( router.emit({ type: 'onBeforeRouteMount', ...changeInfo }) } } - const rendered = - process.env.NODE_ENV !== 'production' && tx[6 /* refresh */] - ? await transitionRefresh(router, tx, result, changeInfo) - : await router.startTransition(commit, result[1 /* matches */]) - if ( - process.env.NODE_ENV !== 'production' && - tx[6 /* refresh */] && - rendered === undefined - ) { - return + const rendered = await router.startTransition( + commit, + result[1 /* matches */], + ) + if (process.env.NODE_ENV !== 'production' && tx[6 /* refresh */]) { + tx[6 /* refresh */] = undefined } if (router._tx !== tx) { discardBackground(router, result) return } if (background?.length) { - // Publish refreshes only after the foreground render acknowledgement. - // Otherwise a fast refresh can replace the acknowledged generation + // Publish background matches only after the foreground acknowledgement. + // Otherwise fast work can replace the acknowledged generation // before the framework commits it and strand the navigation. runBackground( router, @@ -2009,12 +1920,8 @@ export async function loadClientRoute( ): Promise { let rematerialize = false if (process.env.NODE_ENV !== 'production') { - router._tx?.[6 /* refresh */]?.[2 /* rollback */]?.() rematerialize = !!router._refreshNextLoad || !!router._tx?.[6 /* refresh */] } - const refreshPresentation = rematerialize - ? router.stores.matches.get() - : undefined const previousOwner = router._tx const resolvedLocation = router.stores.resolvedLocation.get() const previousLocation = resolvedLocation ?? router.stores.location.get() @@ -2054,36 +1961,15 @@ export async function loadClientRoute( return } const sameHref = previousLocation.href === location.href - let matches: Array let controller = preflight - try { - matches = - process.env.NODE_ENV !== 'production' && rematerialize - ? router.matchRoutes(location, { - _controller: preflight, - _rematerialize: true, - }) - : router.matchRoutes(location, { _controller: preflight }) - acquireMatchResources(matches) - } catch (cause) { - preflight.abort() - if (!isRedirect(cause)) { - if (process.env.NODE_ENV !== 'production' && rematerialize) { - router._refreshNextLoad = undefined - } - await awaitCurrent(router) - router._commitPromise?.resolve() - router._commitPromise = undefined - return - } - await router.navigate({ - ...cause.options, - replace: true, - ignoreBlocker: true, - }) - await awaitCurrent(router, previousOwner) - return - } + const matches = + process.env.NODE_ENV !== 'production' && rematerialize + ? router.matchRoutes(location, { + _controller: preflight, + _rematerialize: true, + }) + : router.matchRoutes(location, { _controller: preflight }) + acquireMatchResources(matches) const resolvedPrefix = hydrationController ? handoff![1 /* finish */](matches) : undefined @@ -2116,15 +2002,12 @@ export async function loadClientRoute( resolvedPrefix, ), ) - .catch(() => { - if (router._tx === tx) { - restoreCommitted(router, tx) - } - }), + // Preserve the settlement turn in which immediately completed background + // work can publish before callers resume from `load`. + .then(), ] if (process.env.NODE_ENV !== 'production' && rematerialize) { - // `refreshPresentation` is always captured when `rematerialize` is set. - tx[6 /* refresh */] = [refreshPresentation!, handoff] + tx[6 /* refresh */] = [handoff] router._refreshNextLoad = undefined } router._tx = tx @@ -2163,17 +2046,13 @@ export async function loadClientRoute( ) { offerPending(router, tx) } - try { - await tx[5 /* done */] - } finally { - await awaitCurrent(router, tx) - } + await tx[5 /* done */] + await awaitCurrent(router, tx) } export async function refreshClientRoute( router: CoordinatorRouter, ): Promise { - router._tx?.[6 /* refresh */]?.[2 /* rollback */]?.() const pending = router._tx if ( pending && @@ -2185,7 +2064,7 @@ export async function refreshClientRoute( await awaitCurrent(router, pending) } } - // Existing owners remain alive for rollback but cannot donate stale work. + // Existing owners remain presented but cannot donate stale work. router._flights?.clear() router.clearCache() router._refreshNextLoad = true @@ -2196,17 +2075,15 @@ export async function preloadClientRoute( router: CoordinatorRouter, opts: any, redirects = 0, + builtLocation?: ParsedLocation, ): Promise | undefined> { - if (redirects > 20) { - return - } if ( process.env.NODE_ENV !== 'production' && (router._refreshNextLoad || router._tx?.[6 /* refresh */]) ) { return } - const location = opts._builtLocation ?? router.buildLocation(opts) + const location = builtLocation ?? router.buildLocation(opts) const base = router._committed const controller = new AbortController() let matches: Array @@ -2230,9 +2107,6 @@ export async function preloadClientRoute( result = await executeClientLane(router, location, matches, [ controller, redirects, - // Preload lanes run to completion even when unrelated navigations commit: - // finished work seeds the cache. - () => true, base, true, ]) @@ -2251,11 +2125,9 @@ export async function preloadClientRoute( ) { return preloadClientRoute( router, - { - ...result[1 /* redirect */].options, - _fromLocation: location, - }, + result[1 /* redirect */].options, redirects + 1, + result[2 /* location */], ) } } catch (cause) { @@ -2315,8 +2187,7 @@ export async function hydrate(router: AnyRouter): Promise { const previousPreflight = router._preflight router._preflight = controller previousPreflight?.abort() - // Route context can abort this controller itself. Only a new slot owner - // supersedes hydration. + // Only a new slot owner supersedes hydration. const isCurrent = () => router._preflight === controller let location!: AnyRouter['latestLocation'] @@ -2516,7 +2387,11 @@ export async function hydrate(router: AnyRouter): Promise { params: match.params, context: parentContext, location, - navigate: navigateFrom(router, location), + navigate: (opts: any) => + router.navigate({ + ...opts, + _fromLocation: location, + }), buildLocation: router.buildLocation, cause: match.cause, abortController: controller, diff --git a/packages/router-core/src/load-server.ts b/packages/router-core/src/load-server.ts index 083d7ffb407..7608b0fc65c 100644 --- a/packages/router-core/src/load-server.ts +++ b/packages/router-core/src/load-server.ts @@ -82,17 +82,51 @@ function normalize(value: unknown, rejected: boolean): LoaderOutcome { return rejected ? [ERROR, value] : [SUCCESS, value] } -function normalizeError(route: AnyRoute, cause: unknown): LoaderOutcome { +function normalizeError( + router: AnyRouter, + lane: { location: ParsedLocation }, + route: AnyRoute, + cause: unknown, + signal?: AbortSignal, + notify = true, +): LoaderOutcome { + signal?.throwIfAborted() let outcome = normalize(cause, true) if (outcome[0] !== ERROR) { - return outcome + return materializeRedirect(router, lane, route, outcome, signal, notify) } try { route.options.onError?.(outcome[1]) } catch (onErrorCause) { outcome = normalize(onErrorCause, true) } - return outcome + signal?.throwIfAborted() + return materializeRedirect(router, lane, route, outcome, signal, notify) +} + +function materializeRedirect( + router: AnyRouter, + lane: { location: ParsedLocation }, + route: AnyRoute, + outcome: LoaderOutcome, + signal?: AbortSignal, + notify = true, +): LoaderOutcome { + if (outcome[0] !== REDIRECTED) { + return outcome + } + signal?.throwIfAborted() + try { + outcome[1].options._fromLocation = lane.location + router.resolveRedirect(outcome[1]) + signal?.throwIfAborted() + return outcome + } catch (cause) { + signal?.throwIfAborted() + return notify + ? normalizeError(router, lane, route, cause, signal, false) + : [ERROR, cause] + } } function maybe( @@ -197,7 +231,13 @@ async function contextualize( match.ssr = await resolveSsr(router, lane, index) } catch (cause) { signal?.throwIfAborted() - failure = [index, stampNotFound(match, normalizeError(route, cause))] + failure = [ + index, + stampNotFound( + match, + normalizeError(router, lane, route, cause, signal), + ), + ] end = index } signal?.throwIfAborted() @@ -239,7 +279,13 @@ async function contextualize( } catch (cause) { signal?.throwIfAborted() if (!failure) { - failure = [index, stampNotFound(match, normalizeError(route, cause))] + failure = [ + index, + stampNotFound( + match, + normalizeError(router, lane, route, cause, signal), + ), + ] } end = index break @@ -252,7 +298,10 @@ async function contextualize( if (validationError !== undefined) { failure = [ index, - stampNotFound(match, normalizeError(route, validationError)), + stampNotFound( + match, + normalizeError(router, lane, route, validationError, signal), + ), ] end = index break @@ -293,7 +342,16 @@ async function contextualize( try { const beforeLoadContext = await route.options.beforeLoad(options) signal?.throwIfAborted() - const outcome = stampNotFound(match, normalize(beforeLoadContext, false)) + const outcome = stampNotFound( + match, + materializeRedirect( + router, + lane, + route, + normalize(beforeLoadContext, false), + signal, + ), + ) if (outcome[0] !== SUCCESS) { failure = [index, outcome] end = index @@ -307,7 +365,13 @@ async function contextualize( parentContext = match.context } catch (cause) { signal?.throwIfAborted() - failure = [index, stampNotFound(match, normalizeError(route, cause))] + failure = [ + index, + stampNotFound( + match, + normalizeError(router, lane, route, cause, signal), + ), + ] end = index break } @@ -373,14 +437,13 @@ function createLoaderTask( (cause) => normalize(cause, true), ) .then((result): LoaderOutcome => { - if ( - result[0] !== REDIRECTED && - (signal?.aborted || match.abortController.signal.reason === lane) - ) { + if (signal?.aborted || match.abortController.signal.reason === lane) { return [SKIPPED] } if (result[0] === ERROR) { - result = normalizeError(route, result[1]) + result = normalizeError(router, lane, route, result[1], signal) + } else { + result = materializeRedirect(router, lane, route, result, signal) } return stampNotFound(match, result) }) @@ -424,13 +487,13 @@ async function getNotFoundBoundary( } for (let candidate = index; candidate >= 0; candidate--) { const route = getRoute(router, matches[candidate]!) - const loading = loadRouteChunk(route, false) - if (loading) { - try { + try { + const loading = loadRouteChunk(route, false) + if (loading) { await loading - } catch { - signal?.throwIfAborted() } + } catch { + signal?.throwIfAborted() } signal?.throwIfAborted() if (route.options.notFoundComponent) { @@ -450,15 +513,6 @@ function abortMatches( } } -function resolveServerRedirect( - router: AnyRouter, - location: ParsedLocation, - value: AnyRedirect, -): ServerLoadResult { - value.options._fromLocation = location - return { type: 'redirect', redirect: router.resolveRedirect(value) } -} - async function applyFailure( router: AnyRouter, lane: ContextualizedLane, @@ -530,7 +584,10 @@ async function loadNormalChunks( signal?.throwIfAborted() return [ index, - stampNotFound(match, normalizeError(route, cause)), + stampNotFound( + match, + normalizeError(router, lane, route, cause, signal), + ), ] as IndexedOutcome }, ) @@ -540,7 +597,13 @@ async function loadNormalChunks( } } catch (cause) { signal?.throwIfAborted() - chunks.push([index, stampNotFound(match, normalizeError(route, cause))]) + chunks.push([ + index, + stampNotFound( + match, + normalizeError(router, lane, route, cause, signal), + ), + ]) } } for (const chunk of chunks) { @@ -698,7 +761,7 @@ async function executeServerLane( if (control?.[1][0] === REDIRECTED) { abortMatches(lane.matches, 0, lane) - return resolveServerRedirect(router, location, control[1][1]) + return { type: 'redirect', redirect: control[1][1] } } let failure = lane.failure ?? loaderFailure @@ -742,7 +805,7 @@ async function executeServerLane( if (requiredFailure) { if (requiredFailure[1][0] === REDIRECTED) { abortMatches(lane.matches) - return resolveServerRedirect(router, location, requiredFailure[1][1]) + return { type: 'redirect', redirect: requiredFailure[1][1] } } failure = requiredFailure } @@ -808,9 +871,7 @@ export async function loadServerRoute( }) if (next.publicHref !== canonical.publicHref) { const href = canonical.publicHref || '/' - throw canonical.external - ? redirect({ href }) - : redirect({ href, _builtLocation: canonical }) + throw redirect({ href }) } const fromLocation = router.stores.resolvedLocation.get() @@ -828,7 +889,8 @@ export async function loadServerRoute( if (!isRedirect(cause)) { throw cause } - result = resolveServerRedirect(router, next, cause) + cause.options._fromLocation = next + result = { type: 'redirect', redirect: router.resolveRedirect(cause) } } router._serverResult = result diff --git a/packages/router-core/src/redirect.ts b/packages/router-core/src/redirect.ts index 2ac387daeb2..4485bc8adf9 100644 --- a/packages/router-core/src/redirect.ts +++ b/packages/router-core/src/redirect.ts @@ -1,6 +1,5 @@ import type { NavigateOptions } from './link' import type { AnyRouter, RegisteredRouter } from './router' -import type { ParsedLocation } from './location' export type AnyRedirect = Redirect @@ -14,13 +13,7 @@ export type Redirect< TMaskFrom extends string = TFrom, TMaskTo extends string = '.', > = Response & { - options: NavigateOptions & { - /** - * @internal - * A **trusted** built location that can be used to redirect to. - */ - _builtLocation?: ParsedLocation - } + options: NavigateOptions } export type RedirectOptions< @@ -50,11 +43,6 @@ export type RedirectOptions< * @link [API Docs](https://tanstack.com/router/latest/docs/framework/react/api/router/RedirectType#headers-property) */ headers?: HeadersInit - /** - * @internal - * A **trusted** built location that can be used to redirect to. - */ - _builtLocation?: ParsedLocation } & NavigateOptions export type ResolvedRedirect< @@ -122,11 +110,7 @@ export function redirect< ): Redirect { opts.statusCode = opts.statusCode || opts.code || 307 - if ( - !opts._builtLocation && - !opts.reloadDocument && - typeof opts.href === 'string' - ) { + if (!opts.reloadDocument && typeof opts.href === 'string') { try { new URL(opts.href) opts.reloadDocument = true diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index fde737da59f..b473339c70a 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -692,13 +692,7 @@ export type PreloadRouteFn< TTo, TMaskFrom, TMaskTo - > & { - /** - * @internal - * A **trusted** built location that can be used to redirect to. - */ - _builtLocation?: ParsedLocation - }, + >, ) => Promise | undefined> export type MatchRouteFn< @@ -937,10 +931,10 @@ export function runRouteLifecycle( router: AnyRouter, previous: Array, matches: Array, - isCurrent?: () => boolean, + owner?: LoadTransaction, ): void { for (const match of previous) { - if (isCurrent?.() === false) { + if (owner && router._tx !== owner) { return } if (!matches.some((candidate) => candidate.routeId === match.routeId)) { @@ -950,7 +944,7 @@ export function runRouteLifecycle( } } for (const match of matches) { - if (isCurrent?.() === false) { + if (owner && router._tx !== owner) { return } const route = (router.routesById as Record)[ @@ -1846,6 +1840,19 @@ export class RouterCore< unmaskOnReload?: boolean } = {}, ): ParsedLocation => { + if (dest.href) { + const parsed = parseHref(dest.href, {} as ParsedHistoryState) + dest = { + ...dest, + to: executeRewriteInput( + this.rewrite, + new URL(parsed.pathname, this.origin), + ).pathname, + search: this.options.parseSearch(parsed.search), + hash: parsed.hash.slice(1), + } + } + // We allow the caller to override the current location const currentLocation = dest._fromLocation || this._pendingLocation || this.latestLocation @@ -2216,38 +2223,12 @@ export class RouterCore< hashScrollIntoView, viewTransition, ignoreBlocker, - _redirects, - href, ...rest - }: BuildNextOptions & - CommitLocationOptions & { _redirects?: number } = {}) => { - if (href) { - const currentIndex = this.history.location.state.__TSR_index - - const parsed = parseHref(href, { - __TSR_index: replace ? currentIndex : currentIndex + 1, - }) - - // If the href contains the basepath, we need to strip it before setting `to` - // because `buildLocation` will add the basepath back when creating the final URL. - // Without this, hrefs like '/app/about' would become '/app/app/about'. - const hrefUrl = new URL(parsed.pathname, this.origin) - const rewrittenUrl = executeRewriteInput(this.rewrite, hrefUrl) - - rest.to = rewrittenUrl.pathname - rest.search = this.options.parseSearch(parsed.search) - // remove the leading `#` from the hash - rest.hash = parsed.hash.slice(1) - } - + }: BuildNextOptions & CommitLocationOptions = {}) => { const location = this.buildLocation({ ...(rest as any), _includeValidateSearch: true, }) - if (_redirects) { - ;(location as typeof location & { _redirects?: number })._redirects = - _redirects - } this._pendingLocation = location as ParsedLocation< FullSearchSchema @@ -2514,9 +2495,8 @@ export class RouterCore< resolveRedirect = (redirect: AnyRedirect): AnyRedirect => { const locationHeader = redirect.headers.get('Location') - if (!redirect.options.href || redirect.options._builtLocation) { - const location = - redirect.options._builtLocation ?? this.buildLocation(redirect.options) + if (!redirect.options.href) { + const location = this.buildLocation(redirect.options) const href = location.publicHref || '/' redirect.options.href = href redirect.headers.set('Location', href) @@ -2535,7 +2515,6 @@ export class RouterCore< if ( redirect.options.href && - !redirect.options._builtLocation && // Check for dangerous protocols before processing the redirect isDangerousProtocol(redirect.options.href, this.protocolAllowlist) ) { @@ -2611,7 +2590,8 @@ export class RouterCore< TTrailingSlashOption, TDefaultStructuralSharingOption, TRouterHistory - > = (opts) => preloadClientRoute(this, opts) + > = (opts: any, builtLocation?: ParsedLocation) => + preloadClientRoute(this, opts, 0, builtLocation) matchRoute: MatchRouteFn< TRouteTree, diff --git a/packages/router-core/tests/boundary-component-chunk.test.ts b/packages/router-core/tests/boundary-component-chunk.test.ts index b8e8322ca6a..e1302003d27 100644 --- a/packages/router-core/tests/boundary-component-chunk.test.ts +++ b/packages/router-core/tests/boundary-component-chunk.test.ts @@ -138,8 +138,60 @@ describe('route boundary component preloads', () => { expect(match?.error).toBe(routeError) }) + test('an ancestor error boundary ignores a hidden descendant component failure', async () => { + const componentGate = createControlledPromise() + const routeError = new Error('parent loader failed') + const componentError = new Error('child component chunk failed') + const childOnError = vi.fn() + pendingGates.push(componentGate) + + const HiddenComponent = Object.assign(() => null, { + preload: () => componentGate, + }) + + const rootRoute = new BaseRootRoute({}) + const parentRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/parent', + loader: () => { + throw routeError + }, + errorComponent: () => null, + }) + const childRoute = new BaseRoute({ + getParentRoute: () => parentRoute, + path: '/child', + component: HiddenComponent as any, + onError: childOnError, + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([parentRoute.addChildren([childRoute])]), + history: createMemoryHistory({ initialEntries: ['/parent/child'] }), + }) + + const loading = router.load() + pendingLoads.push(loading) + await loading + + expect(router.state.matches[1]).toMatchObject({ + routeId: parentRoute.id, + status: 'error', + error: routeError, + }) + expect(componentGate.status).toBe('pending') + + componentGate.reject(componentError) + await expect(componentGate).rejects.toBe(componentError) + await new Promise((resolve) => setTimeout(resolve, 0)) + + expect(childOnError).not.toHaveBeenCalled() + expect(router.state.matches[1]?.error).toBe(routeError) + }) + test('global notFound does not wait for component chunks below its boundary', async () => { const hiddenComponentGate = createControlledPromise() + const componentError = new Error('hidden component chunk failed') + const hiddenOnError = vi.fn() const notFoundPreload = vi.fn(() => Promise.resolve()) pendingGates.push(hiddenComponentGate) @@ -160,6 +212,7 @@ describe('route boundary component preloads', () => { getParentRoute: () => layoutRoute, path: '/child', component: HiddenComponent as any, + onError: hiddenOnError, }) const router = createTestRouter({ routeTree: rootRoute.addChildren([layoutRoute.addChildren([childRoute])]), @@ -178,6 +231,61 @@ describe('route boundary component preloads', () => { expect(router.state.matches.find((match) => match._notFound)?.routeId).toBe( layoutRoute.id, ) + + hiddenComponentGate.reject(componentError) + await expect(hiddenComponentGate).rejects.toBe(componentError) + await new Promise((resolve) => setTimeout(resolve, 0)) + + expect(hiddenOnError).not.toHaveBeenCalled() + }) + + test('a root not-found boundary ignores a late root component failure', async () => { + const componentGate = createControlledPromise() + const notFoundGate = createControlledPromise() + const componentError = new Error('root component chunk failed') + const onError = vi.fn() + const notFoundPreload = vi.fn(() => notFoundGate) + pendingGates.push(componentGate, notFoundGate) + + const RootComponent = Object.assign(() => null, { + preload: () => componentGate, + }) + const NotFoundBoundary = Object.assign(() => null, { + preload: notFoundPreload, + }) + const rootRoute = new BaseRootRoute({ + component: RootComponent as any, + notFoundComponent: NotFoundBoundary as any, + onError, + }) + const childRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/child', + beforeLoad: () => { + throw notFound({ routeId: rootRoute.id }) + }, + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([childRoute]), + history: createMemoryHistory({ initialEntries: ['/child'] }), + }) + + const loading = router.load() + pendingLoads.push(loading) + await vi.waitFor(() => expect(notFoundPreload).toHaveBeenCalledOnce()) + notFoundGate.resolve() + await loading + + expect(router.state.matches[0]).toMatchObject({ + status: 'success', + _notFound: true, + }) + + componentGate.reject(componentError) + await expect(componentGate).rejects.toBe(componentError) + await new Promise((resolve) => setTimeout(resolve, 0)) + + expect(onError).not.toHaveBeenCalled() }) test('a late normal component chunk cannot replace a selected not-found boundary', async () => { diff --git a/packages/router-core/tests/build-location.test.ts b/packages/router-core/tests/build-location.test.ts index c35937259c5..14ab661ff6a 100644 --- a/packages/router-core/tests/build-location.test.ts +++ b/packages/router-core/tests/build-location.test.ts @@ -1961,6 +1961,28 @@ describe('buildLocation - location output structure', () => { expect(location.searchStr).toBe('') expect(location.href).toBe('/posts') }) + + test('href options are not mutated', async () => { + const rootRoute = new BaseRootRoute({}) + const postsRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/posts', + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([postsRoute]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + const options = Object.freeze({ href: '/posts?page=1#section' }) + + const location = router.buildLocation(options as any) + + expect(location).toMatchObject({ + pathname: '/posts', + search: { page: 1 }, + hash: 'section', + }) + expect(options).toEqual({ href: '/posts?page=1#section' }) + }) }) describe('buildLocation - optional params', () => { diff --git a/packages/router-core/tests/client-lane-adversarial.test.ts b/packages/router-core/tests/client-lane-adversarial.test.ts index acfe3fede49..bcd75475951 100644 --- a/packages/router-core/tests/client-lane-adversarial.test.ts +++ b/packages/router-core/tests/client-lane-adversarial.test.ts @@ -173,6 +173,196 @@ describe('adversarial client lane ownership', () => { expect(cOnEnter).toHaveBeenCalledTimes(1) }) + test('supersession cancels lazy not-found boundary lookup without leaking pending ownership', async () => { + const lazyStarted = createControlledPromise() + const lazyGate = createControlledPromise() + const safeHeadStarted = createControlledPromise() + const safeHeadGate = createControlledPromise() + + const rootRoute = new BaseRootRoute({}) + const indexRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/', + }) + const missingRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/missing', + pendingMs: 0, + pendingMinMs: 0, + pendingComponent: () => null, + beforeLoad: () => { + throw notFound() + }, + }).lazy(() => { + lazyStarted.resolve() + return lazyGate + }) + const safeRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/safe', + head: async () => { + safeHeadStarted.resolve() + await safeHeadGate + return {} + }, + }) + const history = createMemoryHistory({ initialEntries: ['/'] }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([indexRoute, missingRoute, safeRoute]), + history, + }) + + await router.load() + history.push('/missing') + const supersededLoad = router.load() + await lazyStarted + const supersededTx = router._tx! + expect(router._pending?.[0]).toBe(supersededTx) + + let supersededOutcome: unknown + const observedSupersededLoad = supersededLoad.then( + () => { + supersededOutcome = 'resolved' + }, + (cause) => { + supersededOutcome = cause + }, + ) + history.push('/safe') + const replacementLoad = router.load() + await safeHeadStarted + await new Promise((resolve) => setTimeout(resolve, 0)) + + expect(supersededOutcome).toBeUndefined() + expect(safeHeadGate.status).toBe('pending') + expect(router._pending).toBeUndefined() + expect(router.state.matches.at(-1)?.routeId).toBe(missingRoute.id) + + safeHeadGate.resolve() + lazyGate.resolve({ options: { notFoundComponent: () => null } }) + await Promise.all([observedSupersededLoad, replacementLoad]) + + expect(supersededOutcome).toBe('resolved') + expect(router._pending).toBeUndefined() + expect(router.state).toMatchObject({ + status: 'idle', + location: { pathname: '/safe' }, + }) + expect(router.state.matches.at(-1)?.routeId).toBe(safeRoute.id) + history.destroy() + }) + + test('an unrelated third load clears a pending session awaiting same-boundary takeover', async () => { + const pageStarted = createControlledPromise() + const pageGate = createControlledPromise() + const retainedStarted = createControlledPromise() + const retainedGate = createControlledPromise() + const safeHeadStarted = createControlledPromise() + const safeHeadGate = createControlledPromise() + + const rootRoute = new BaseRootRoute({ + validateSearch: (search: Record) => ({ + revision: Number(search.revision) || 0, + }), + beforeLoad: ({ search }) => { + if (search.revision === 2) { + retainedStarted.resolve() + return retainedGate.then(() => ({})) + } + return {} + }, + }) + const indexRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/', + }) + const pageRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/page', + pendingMs: 0, + pendingMinMs: 0, + pendingComponent: () => null, + beforeLoad: ({ search }) => { + if (search.revision === 1) { + pageStarted.resolve() + return pageGate.then(() => ({})) + } + return {} + }, + }) + const safeRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/safe', + head: async () => { + safeHeadStarted.resolve() + await safeHeadGate + return {} + }, + }) + const history = createMemoryHistory({ initialEntries: ['/'] }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([indexRoute, pageRoute, safeRoute]), + history, + }) + + await router.load() + const loads: Array> = [] + try { + history.push('/page?revision=1') + const firstLoad = router.load() + loads.push(firstLoad) + await pageStarted + const firstTx = router._tx! + await vi.waitFor(() => expect(router._pending?.[0]).toBe(firstTx)) + + history.push('/page?revision=2') + const secondLoad = router.load() + loads.push(secondLoad) + await retainedStarted + await new Promise((resolve) => setTimeout(resolve, 0)) + expect(router._pending?.[0]).toBe(firstTx) + + history.push('/safe') + const thirdLoad = router.load() + loads.push(thirdLoad) + await safeHeadStarted + await new Promise((resolve) => setTimeout(resolve, 0)) + + expect(router._pending).toBeUndefined() + } finally { + pageGate.resolve() + retainedGate.resolve() + safeHeadGate.resolve() + await Promise.allSettled(loads) + history.destroy() + } + }) + + test('a live signal rejected by lazy not-found lookup is not treated as cancellation', async () => { + let laneSignal: AbortSignal | undefined + const rootRoute = new BaseRootRoute({}) + const missingRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/missing', + beforeLoad: ({ abortController }) => { + laneSignal = abortController.signal + throw notFound() + }, + }).lazy(() => Promise.reject(laneSignal)) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([missingRoute]), + history: createMemoryHistory({ initialEntries: ['/missing'] }), + }) + + await router.load() + + expect(laneSignal?.aborted).toBe(false) + expect(router.state.status).toBe('idle') + expect(router.state.location.pathname).toBe('/missing') + expect(router.state.resolvedLocation?.pathname).toBe('/missing') + expect(router.state.matches.at(-1)?.status).not.toBe('pending') + }) + test('keeps a hidden child matched without projecting it below a parent error', async () => { const parentError = new Error('parent failed') const childHead = vi.fn(() => ({ @@ -256,6 +446,57 @@ describe('adversarial client lane ownership', () => { expect(redirectSignal?.aborted).toBe(true) }) + test('a navigation started while releasing a redirect lane supersedes that redirect', async () => { + let successorNavigation: Promise | undefined + + const rootRoute = new BaseRootRoute({}) + const indexRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/', + }) + const sourceRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/source', + loader: ({ abortController }) => { + abortController.signal.addEventListener( + 'abort', + () => { + successorNavigation = router.navigate({ to: '/successor' }) + }, + { once: true }, + ) + throw redirect({ to: '/redirect-target' }) + }, + }) + const redirectTargetRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/redirect-target', + }) + const successorRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/successor', + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([ + indexRoute, + sourceRoute, + redirectTargetRoute, + successorRoute, + ]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + + await router.load() + await router.navigate({ to: '/source' }) + await successorNavigation + + expect(router.state.location.pathname).toBe('/successor') + expect(router.state.matches.at(-1)).toMatchObject({ + routeId: successorRoute.id, + status: 'success', + }) + }) + test('keeps successful descendant data behind an ancestor boundary', async () => { const parentError = new Error('parent failed') const childData = { value: 'child data' } diff --git a/packages/router-core/tests/fatal-load-rejection.test.ts b/packages/router-core/tests/fatal-load-rejection.test.ts deleted file mode 100644 index bac78891b0c..00000000000 --- a/packages/router-core/tests/fatal-load-rejection.test.ts +++ /dev/null @@ -1,86 +0,0 @@ -import { describe, expect, test, vi } from 'vitest' -import { createMemoryHistory } from '@tanstack/history' -import { - BaseRootRoute, - BaseRoute, - createControlledPromise, - redirect, -} from '../src' -import { createTestRouter } from './routerTestUtils' - -/** - * A genuinely fatal router rejection (not a route outcome — for example, - * redirect resolution throwing while finalizing a serial beforeLoad - * redirect) must settle without publishing matches whose load - * promises never settled: the serial redirect capped the loader prefix at 0, - * so the loader phase never ran for ancestor matches and their pending - * loadPromise would hang Suspense forever. - */ - -describe('fatal load rejection', () => { - test('fatal rejection during a serial-redirect-capped pass settles the lane', async () => { - const boom = new Error('resolveRedirect failed') - const errorComponentGate = createControlledPromise() - const errorComponentStarted = createControlledPromise() - const errorComponentPreload = vi.fn(() => { - errorComponentStarted.resolve() - return errorComponentGate - }) - const ErrorComponent = Object.assign(() => null, { - preload: errorComponentPreload, - }) - - const rootRoute = new BaseRootRoute({ - // Never runs: the serial redirect caps the loader prefix at 0. Its - // loadPromise (created for the beforeLoad phase) must still settle. - loader: () => 'root data', - errorComponent: ErrorComponent, - }) - const badRoute = new BaseRoute({ - getParentRoute: () => rootRoute, - path: '/bad', - beforeLoad: () => { - throw redirect({ - to: '/bad', - search: () => { - throw boom - }, - }) - }, - }) - const safeLoader = vi.fn(() => 'safe data') - const safeRoute = new BaseRoute({ - getParentRoute: () => rootRoute, - path: '/safe', - loader: safeLoader, - }) - - const router = createTestRouter({ - routeTree: rootRoute.addChildren([badRoute, safeRoute]), - history: createMemoryHistory({ initialEntries: ['/bad'] }), - }) - - const load = router.load() - const outcome = await Promise.race([ - load.then(() => 'load-settled' as const), - errorComponentStarted.then(() => 'error-preload-started' as const), - ]) - if (outcome === 'error-preload-started') { - errorComponentGate.resolve() - } - expect(outcome).toBe('load-settled') - await load - expect(errorComponentPreload).not.toHaveBeenCalled() - expect(router.state.status).toBe('idle') - - errorComponentGate.resolve() - await router.navigate({ to: '/safe' }) - expect(router.state.location.pathname).toBe('/safe') - expect(router.state.matches.at(-1)).toMatchObject({ - routeId: safeRoute.id, - status: 'success', - loaderData: 'safe data', - }) - expect(safeLoader).toHaveBeenCalledTimes(1) - }) -}) diff --git a/packages/router-core/tests/hmr-refresh-lifecycle.test.ts b/packages/router-core/tests/hmr-refresh-lifecycle.test.ts index ad7c743bf38..c115b8fff7e 100644 --- a/packages/router-core/tests/hmr-refresh-lifecycle.test.ts +++ b/packages/router-core/tests/hmr-refresh-lifecycle.test.ts @@ -193,13 +193,22 @@ describe('HMR route refresh', () => { expect(router.state.matches[0]?.loaderData).toBe(2) }) - test('restores the committed presentation when publication fails', async () => { + test('publishes refresh route errors without restoring the previous generation', async () => { let generation = 1 + const boom = new Error('refreshed loader failed') + const onError = vi.fn() const rootRoute = new BaseRootRoute({}) const pageRoute = new BaseRoute({ getParentRoute: () => rootRoute, path: '/page', - loader: () => generation, + loader: () => { + if (generation === 2) { + throw boom + } + return generation + }, + onError, + errorComponent: () => null, }) const router = createTestRouter({ routeTree: rootRoute.addChildren([pageRoute]), @@ -207,28 +216,19 @@ describe('HMR route refresh', () => { }) await router.load() - const previousMatches = router.state.matches generation = 2 - const startTransition = router.startTransition - router.startTransition = async (fn) => { - fn() - router.clearCache() - throw new Error('render failed') - } - await router._refreshRoute!() + expect(onError).toHaveBeenCalledWith(boom) expect(router.state.status).toBe('idle') - expect(router.state.matches).toHaveLength(previousMatches.length) - expect(router.state.matches.at(-1)?.loaderData).toBe(1) - - router.startTransition = startTransition - generation = 3 - await router._refreshRoute!() - expect(router.state.matches.at(-1)?.loaderData).toBe(3) + expect(router.state.matches.at(-1)).toMatchObject({ + routeId: pageRoute.id, + status: 'error', + error: boom, + }) }) - test('keeps the rendered generation alive until refresh is acknowledged', async () => { + test('rapid refreshes release obsolete generations and converge on the successor', async () => { let generation = 1 const signals: Array = [] const rootRoute = new BaseRootRoute({}) @@ -246,70 +246,44 @@ describe('HMR route refresh', () => { }) await router.load() - const renderedSignal = signals[0]! - generation = 2 - const startTransition = router.startTransition - router.startTransition = async (fn) => { - fn() - throw new Error('render failed') - } - - await router._refreshRoute!() - - expect(renderedSignal.aborted).toBe(false) - expect(signals[1]?.aborted).toBe(true) - expect(router.state.matches.at(-1)?.loaderData).toBe(1) - - router.startTransition = startTransition - generation = 3 - await router._refreshRoute!() - expect(renderedSignal.aborted).toBe(true) - expect(router.state.matches.at(-1)?.loaderData).toBe(3) - }) - - test('rolls overlapping refreshes back to the last acknowledged generation', async () => { - let generation = 1 - const rootRoute = new BaseRootRoute({}) - const pageRoute = new BaseRoute({ - getParentRoute: () => rootRoute, - path: '/page', - loader: () => generation, - }) - const router = createTestRouter({ - routeTree: rootRoute.addChildren([pageRoute]), - history: createMemoryHistory({ initialEntries: ['/page'] }), - }) - - await router.load() - let transitionCount = 0 - let supersedeFirst!: () => void + const firstAck = createControlledPromise() + const firstPublished = createControlledPromise() + let transitions = 0 router.startTransition = (fn) => { - transitionCount++ - fn() - if (transitionCount === 1) { - return new Promise((resolve) => { - supersedeFirst = () => resolve(false) - }) + transitions++ + if (transitions === 1) { + fn() + firstPublished.resolve() + return firstAck } - return Promise.reject(new Error('replacement render failed')) + firstAck.resolve(false) + fn() + return Promise.resolve(true) } - ;( - router as typeof router & { _cancelTransition?: () => void } - )._cancelTransition = () => supersedeFirst() generation = 2 - const firstRefresh = router._refreshRoute!() - await vi.waitFor(() => expect(transitionCount).toBe(1)) + const firstSettled = vi.fn() + const firstRefresh = router._refreshRoute!().then(firstSettled) + await firstPublished + + expect(firstSettled).not.toHaveBeenCalled() + expect(signals[0]?.aborted).toBe(true) + expect(signals[1]?.aborted).toBe(false) + expect(router.state.matches.at(-1)?.loaderData).toBe(2) generation = 3 const secondRefresh = router._refreshRoute!() await Promise.all([firstRefresh, secondRefresh]) + expect(firstSettled).toHaveBeenCalledOnce() + expect(signals[1]?.aborted).toBe(true) + expect(signals[2]?.aborted).toBe(false) + expect(router.state.matches.at(-1)?.loaderData).toBe(3) expect(router.state.status).toBe('idle') - expect(router.state.matches.at(-1)?.loaderData).toBe(1) + expect(router._cache.size).toBe(0) }) - test('does not resolve a superseding navigation promise during rollback', async () => { + test('a published refresh waits for the navigation that supersedes it', async () => { let generation = 1 const destinationGate = createControlledPromise() const rootRoute = new BaseRootRoute({}) @@ -329,25 +303,25 @@ describe('HMR route refresh', () => { }) await router.load() + const refreshAck = createControlledPromise() + const refreshPublished = createControlledPromise() let transitionCount = 0 - let cancelRefresh!: () => void router.startTransition = (fn) => { transitionCount++ - fn() if (transitionCount === 1) { - return new Promise((resolve) => { - cancelRefresh = () => resolve(false) - }) + fn() + refreshPublished.resolve() + return refreshAck } + refreshAck.resolve(false) + fn() return Promise.resolve(true) } - ;( - router as typeof router & { _cancelTransition?: () => void } - )._cancelTransition = () => cancelRefresh() generation = 2 - const refresh = router._refreshRoute!() - await vi.waitFor(() => expect(transitionCount).toBe(1)) + const refreshSettled = vi.fn() + const refresh = router._refreshRoute!().then(refreshSettled) + await refreshPublished const navigationSettled = vi.fn() const navigation = router @@ -356,16 +330,19 @@ describe('HMR route refresh', () => { await vi.waitFor(() => expect(router.state.location.pathname).toBe('/destination'), ) + expect(refreshSettled).not.toHaveBeenCalled() expect(navigationSettled).not.toHaveBeenCalled() destinationGate.resolve() await Promise.all([refresh, navigation]) + expect(refreshSettled).toHaveBeenCalledOnce() expect(navigationSettled).toHaveBeenCalledOnce() expect(router.state.matches.at(-1)?.routeId).toBe(destinationRoute.id) }) test('waits for an ordinary pending navigation before refreshing', async () => { const destinationGate = createControlledPromise() + const destinationLoader = vi.fn(() => destinationGate) const rootRoute = new BaseRootRoute({}) const pageRoute = new BaseRoute({ getParentRoute: () => rootRoute, @@ -377,7 +354,7 @@ describe('HMR route refresh', () => { pendingMs: 0, pendingMinMs: 0, pendingComponent: () => null, - loader: () => destinationGate, + loader: destinationLoader, }) const router = createTestRouter({ routeTree: rootRoute.addChildren([pageRoute, destinationRoute]), @@ -390,9 +367,6 @@ describe('HMR route refresh', () => { expect(router.state.status).toBe('pending') expect(router.state.matches.at(-1)?.routeId).toBe(destinationRoute.id) }) - destinationRoute.options.onStay = () => { - throw new Error('refreshed destination failed') - } const refreshSettled = vi.fn() const refresh = router._refreshRoute!().then(refreshSettled) await Promise.resolve() @@ -406,39 +380,10 @@ describe('HMR route refresh', () => { routeId: destinationRoute.id, status: 'success', }) + expect(destinationLoader).toHaveBeenCalledTimes(2) expect(refreshSettled).toHaveBeenCalledOnce() }) - test('rolls back when an HMR lifecycle callback throws', async () => { - let generation = 1 - const rootRoute = new BaseRootRoute({}) - const pageRoute = new BaseRoute({ - getParentRoute: () => rootRoute, - path: '/page', - loader: () => generation, - }) - const router = createTestRouter({ - routeTree: rootRoute.addChildren([pageRoute]), - history: createMemoryHistory({ initialEntries: ['/page'] }), - }) - - await router.load() - generation = 2 - pageRoute.options.onStay = () => { - throw new Error('lifecycle failed') - } - - await router._refreshRoute!() - - expect(router.state.status).toBe('idle') - expect(router.state.matches.at(-1)?.loaderData).toBe(1) - - pageRoute.options.onStay = undefined - generation = 3 - await router._refreshRoute!() - expect(router.state.matches.at(-1)?.loaderData).toBe(3) - }) - test('does not adopt a preload created by HMR preflight hooks', async () => { let generation = 1 const rootRoute = new BaseRootRoute({}) diff --git a/packages/router-core/tests/hydration-currentness.test.ts b/packages/router-core/tests/hydration-currentness.test.ts index 3257b0e2016..a9d52f0c97b 100644 --- a/packages/router-core/tests/hydration-currentness.test.ts +++ b/packages/router-core/tests/hydration-currentness.test.ts @@ -434,9 +434,10 @@ describe('hydration asset currentness', () => { expect(childLoader).toHaveBeenCalledTimes(1) }) - test('failed HMR restores a partial hydration presentation and handoff', async () => { + test('published HMR consumes a partial hydration handoff before its successor converges', async () => { let hydrationController: AbortController | undefined - const childLoader = vi.fn(() => 'client child data') + let generation = 0 + const childLoader = vi.fn(() => `client child data ${++generation}`) const rootRoute = new BaseRootRoute({ context: ({ abortController }: { abortController: AbortController }) => { hydrationController ??= abortController @@ -465,30 +466,40 @@ describe('hydration asset currentness', () => { await hydrate(router) const handoff = router._handoff - const startTransition = router.startTransition - router.startTransition = async (fn) => { + const firstAck = createControlledPromise() + const firstPublished = createControlledPromise() + let transitions = 0 + router.startTransition = (fn) => { + transitions++ + if (transitions === 1) { + fn() + firstPublished.resolve() + return firstAck + } + firstAck.resolve(false) fn() - throw new Error('HMR render failed') + return Promise.resolve(true) } - await router._refreshRoute!() + const firstRefresh = router._refreshRoute!() + await firstPublished - expect(router.state.matches.map((match) => match.routeId)).toEqual([ - rootRoute.id, - childRoute.id, - ]) + expect(handoff).toBeDefined() + expect(router._handoff).toBeUndefined() + expect(hydrationController?.signal.aborted).toBe(true) expect(router.state.matches[1]).toMatchObject({ - status: 'pending', + status: 'success', ssr: false, + loaderData: 'client child data 1', }) - expect(router._handoff).toBe(handoff) - expect(hydrationController?.signal.aborted).toBe(false) - router.startTransition = startTransition - await router.load() + const secondRefresh = router._refreshRoute!() + await Promise.all([firstRefresh, secondRefresh]) + + expect(router._handoff).toBeUndefined() expect(router.state.matches[1]).toMatchObject({ status: 'success', - loaderData: 'client child data', + loaderData: 'client child data 2', }) expect(childLoader).toHaveBeenCalledTimes(2) }) diff --git a/packages/router-core/tests/public-client-loading-contract.test.ts b/packages/router-core/tests/public-client-loading-contract.test.ts index d9c9f9366d6..169f9e89dc2 100644 --- a/packages/router-core/tests/public-client-loading-contract.test.ts +++ b/packages/router-core/tests/public-client-loading-contract.test.ts @@ -125,76 +125,6 @@ describe('public client loading contracts', () => { } }) - test('a rejected pending publication restores the committed lane', async () => { - const loaderStarted = createControlledPromise() - const loaderGate = createControlledPromise() - let loaderSignal: AbortSignal | undefined - const publicationError = new Error('pending publication failed') - - const rootRoute = new BaseRootRoute({}) - const indexRoute = new BaseRoute({ - getParentRoute: () => rootRoute, - path: '/', - }) - const targetRoute = new BaseRoute({ - getParentRoute: () => rootRoute, - path: '/target', - pendingMs: 0, - pendingComponent: () => null, - loader: ({ abortController }) => { - loaderSignal = abortController.signal - loaderStarted.resolve() - return loaderGate - }, - }) - const recoveryRoute = new BaseRoute({ - getParentRoute: () => rootRoute, - path: '/recovery', - }) - const router = createTestRouter({ - routeTree: rootRoute.addChildren([ - indexRoute, - targetRoute, - recoveryRoute, - ]), - history: createMemoryHistory({ initialEntries: ['/'] }), - }) - - await router.load() - const startTransition = router.startTransition - let rejectPending = true - router.startTransition = (fn, expected) => { - if ( - rejectPending && - expected?.some((match) => match.status === 'pending') - ) { - rejectPending = false - fn() - return Promise.reject(publicationError) - } - return startTransition(fn, expected) - } - - try { - const navigation = router.navigate({ to: '/target' }) - await loaderStarted - await navigation - - expect(loaderSignal?.aborted).toBe(true) - expect(router.state.status).toBe('idle') - expect(router.state.matches.at(-1)?.routeId).toBe(indexRoute.id) - - await router.navigate({ to: '/recovery' }) - expect(router.state.matches.at(-1)).toMatchObject({ - routeId: recoveryRoute.id, - status: 'success', - }) - } finally { - loaderGate.resolve('late target data') - router.startTransition = startTransition - } - }) - test('background loading is observable while retaining committed data', async () => { const reloadGate = createControlledPromise<{ generation: number }>() let loaderCalls = 0 diff --git a/packages/router-core/tests/redirect-target-error.test.ts b/packages/router-core/tests/redirect-target-error.test.ts new file mode 100644 index 00000000000..bf1ce75b0d7 --- /dev/null +++ b/packages/router-core/tests/redirect-target-error.test.ts @@ -0,0 +1,477 @@ +import { describe, expect, test, vi } from 'vitest' +import { createMemoryHistory } from '@tanstack/history' +import { + BaseRootRoute, + BaseRoute, + createControlledPromise, + redirect, +} from '../src' +import { createTestRouter, loadServerResponse } from './routerTestUtils' + +describe('redirect target errors', () => { + test('a client redirect target error becomes the originating route error', async () => { + const boom = new Error('resolveRedirect failed') + const errorComponentGate = createControlledPromise() + const errorComponentStarted = createControlledPromise() + const errorComponentPreload = vi.fn(() => { + errorComponentStarted.resolve() + return errorComponentGate + }) + const ErrorComponent = Object.assign(() => null, { + preload: errorComponentPreload, + }) + + const rootLoader = vi.fn(() => 'root data') + const onError = vi.fn() + const rootRoute = new BaseRootRoute({ loader: rootLoader }) + const badRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/bad', + beforeLoad: () => { + throw redirect({ + to: '/bad', + search: () => { + throw boom + }, + }) + }, + onError, + errorComponent: ErrorComponent, + }) + const safeLoader = vi.fn(() => 'safe data') + const safeRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/safe', + loader: safeLoader, + }) + + const router = createTestRouter({ + routeTree: rootRoute.addChildren([badRoute, safeRoute]), + history: createMemoryHistory({ initialEntries: ['/bad'] }), + }) + + const load = router.load() + const outcome = await Promise.race([ + load.then(() => 'load-settled' as const), + errorComponentStarted.then(() => 'error-preload-started' as const), + ]) + expect(outcome).toBe('error-preload-started') + expect(rootLoader).toHaveBeenCalledOnce() + expect(onError).toHaveBeenCalledWith(boom) + + errorComponentGate.resolve() + await load + expect(errorComponentPreload).toHaveBeenCalledOnce() + expect(router.state.status).toBe('idle') + expect(router.state.location.pathname).toBe('/bad') + expect(router.state.matches.at(-1)).toMatchObject({ + routeId: badRoute.id, + status: 'error', + error: boom, + }) + + await router.navigate({ to: '/safe' }) + expect(router.state.location.pathname).toBe('/safe') + expect(router.state.matches.at(-1)).toMatchObject({ + routeId: safeRoute.id, + status: 'success', + loaderData: 'safe data', + }) + expect(safeLoader).toHaveBeenCalledTimes(1) + }) + + test('a server redirect target error becomes the originating route error', async () => { + const boom = new Error('resolveRedirect failed') + const onError = vi.fn() + const rootRoute = new BaseRootRoute({}) + const badRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/bad', + beforeLoad: () => { + throw redirect({ + to: '/bad', + search: () => { + throw boom + }, + }) + }, + onError, + errorComponent: () => null, + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([badRoute]), + history: createMemoryHistory({ initialEntries: ['/bad'] }), + }) + + const response = await loadServerResponse(router, '/bad') + + expect(response.status).toBe(500) + expect(response.headers.get('Location')).toBeNull() + expect(onError).toHaveBeenCalledWith(boom) + expect(router.state.matches.at(-1)).toMatchObject({ + routeId: badRoute.id, + status: 'error', + error: boom, + }) + }) + + test('a server loader redirect target error becomes the route error', async () => { + const boom = new Error('server loader redirect failed') + const onError = vi.fn() + const rootRoute = new BaseRootRoute({}) + const badRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/bad', + loader: () => + redirect({ + to: '/target', + hash: () => { + throw boom + }, + }), + onError, + errorComponent: () => null, + }) + const targetRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/target', + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([badRoute, targetRoute]), + history: createMemoryHistory({ initialEntries: ['/bad'] }), + }) + + const response = await loadServerResponse(router, '/bad') + + expect(response.status).toBe(500) + expect(onError).toHaveBeenCalledWith(boom) + expect(router.state.matches.at(-1)).toMatchObject({ + routeId: badRoute.id, + status: 'error', + error: boom, + }) + }) + + test('a loader redirect target error becomes the originating route error', async () => { + const boom = new Error('redirect hash failed') + const onError = vi.fn() + const rootRoute = new BaseRootRoute({}) + const badRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/bad', + loader: () => + redirect({ + to: '/target', + hash: () => { + throw boom + }, + }), + onError, + errorComponent: () => null, + }) + const targetRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/target', + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([badRoute, targetRoute]), + history: createMemoryHistory({ initialEntries: ['/bad'] }), + }) + + await router.load() + + expect(onError).toHaveBeenCalledWith(boom) + expect(router.state.location.pathname).toBe('/bad') + expect(router.state.matches.at(-1)).toMatchObject({ + routeId: badRoute.id, + status: 'error', + error: boom, + }) + }) + + test('an internal href parse error becomes the originating route error', async () => { + const boom = new Error('redirect search parse failed') + const onError = vi.fn() + const rootRoute = new BaseRootRoute({}) + const sourceRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/source', + beforeLoad: () => { + throw redirect({ href: '/target?bad' }) + }, + onError, + errorComponent: () => null, + }) + const targetRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/target', + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([sourceRoute, targetRoute]), + history: createMemoryHistory({ initialEntries: ['/source'] }), + parseSearch: (search) => { + if (search) { + throw boom + } + return {} + }, + }) + + await router.load() + + expect(onError).toHaveBeenCalledWith(boom) + expect(router.state.location.pathname).toBe('/source') + expect(router.state.matches.at(-1)).toMatchObject({ + routeId: sourceRoute.id, + status: 'error', + error: boom, + }) + }) + + test('an unsafe document href becomes the originating route error', async () => { + const onError = vi.fn() + const rootRoute = new BaseRootRoute({}) + const sourceRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/source', + loader: () => redirect({ href: 'javascript:alert(1)' }), + onError, + errorComponent: () => null, + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([sourceRoute]), + history: createMemoryHistory({ initialEntries: ['/source'] }), + }) + + await router.load() + + const error = onError.mock.calls[0]?.[0] + expect(error).toEqual( + expect.objectContaining({ + message: expect.stringContaining('unsafe protocol'), + }), + ) + expect(router.state.matches.at(-1)).toMatchObject({ + routeId: sourceRoute.id, + status: 'error', + error, + }) + }) + + test('a preload does not build a document redirect target', async () => { + const search = vi.fn(() => ({ redirected: true })) + const rootRoute = new BaseRootRoute({}) + const sourceRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/source', + beforeLoad: () => + redirect({ + to: '/target', + search, + reloadDocument: true, + }), + }) + const targetRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/target', + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([sourceRoute, targetRoute]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + + const matches = await router.preloadRoute({ to: '/source' }) + + expect(matches).toBeUndefined() + expect(search).not.toHaveBeenCalled() + }) + + test('request cancellation discards a late redirect without finalizing it', async () => { + const cancellation = new Error('request disconnected') + const boom = new Error('late redirect target failed') + const loaderStarted = createControlledPromise() + const loaderGate = createControlledPromise>() + const onError = vi.fn() + const rootRoute = new BaseRootRoute({}) + const sourceRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/source', + loader: () => { + loaderStarted.resolve() + return loaderGate + }, + onError, + }) + const targetRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/target', + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([sourceRoute, targetRoute]), + history: createMemoryHistory({ initialEntries: ['/source'] }), + isServer: true, + }) + const controller = new AbortController() + const load = loadServerResponse(router, '/source', controller.signal) + await loaderStarted + + controller.abort(cancellation) + await expect(load).rejects.toBe(cancellation) + loaderGate.resolve( + redirect({ + to: '/target', + search: () => { + throw boom + }, + }), + ) + await new Promise((resolve) => setTimeout(resolve, 0)) + + expect(onError).not.toHaveBeenCalled() + }) + + test('a background redirect target error publishes after foreground commit', async () => { + const boom = new Error('background redirect target failed') + const reloadStarted = createControlledPromise() + const reloadGate = createControlledPromise>() + const onError = vi.fn() + let loads = 0 + const rootRoute = new BaseRootRoute({}) + const pageRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/page', + loader: () => { + if (++loads === 1) { + return 'initial data' + } + reloadStarted.resolve() + return reloadGate + }, + onError, + errorComponent: () => null, + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([pageRoute]), + history: createMemoryHistory({ initialEntries: ['/page'] }), + }) + + await router.load() + const invalidation = router.invalidate() + await reloadStarted + await invalidation + expect(router.state.matches.at(-1)).toMatchObject({ + routeId: pageRoute.id, + status: 'success', + loaderData: 'initial data', + }) + + reloadGate.resolve( + redirect({ + to: '/page', + search: () => { + throw boom + }, + }), + ) + + await vi.waitFor(() => expect(onError).toHaveBeenCalledWith(boom)) + await vi.waitFor(() => + expect(router.state.matches.at(-1)).toMatchObject({ + routeId: pageRoute.id, + status: 'error', + error: boom, + }), + ) + }) + + test('a successful redirect target is built once', async () => { + const search = vi.fn(() => ({ redirected: true })) + const rootRoute = new BaseRootRoute({}) + const sourceRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/source', + loader: () => redirect({ to: '/target', search }), + }) + const targetRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/target', + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([sourceRoute, targetRoute]), + history: createMemoryHistory({ initialEntries: ['/source'] }), + }) + + await router.load() + + expect(search).toHaveBeenCalledOnce() + expect(router.state.location).toMatchObject({ + pathname: '/target', + search: { redirected: true }, + }) + expect(router.state.matches.at(-1)).toMatchObject({ + routeId: targetRoute.id, + status: 'success', + }) + }) + + test('a shared loader redirect is materialized for each preload lane', async () => { + const loaderStarted = createControlledPromise() + const loaderGate = createControlledPromise>() + const sourceLoader = vi.fn(() => { + loaderStarted.resolve() + return loaderGate + }) + const searchUpdater = vi.fn((search: { version?: number }) => ({ + version: search.version, + })) + const rootRoute = new BaseRootRoute({}) + const sourceRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/source', + validateSearch: (search: Record) => ({ + version: Number(search.version), + }), + loader: sourceLoader, + }) + const targetRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/target', + validateSearch: (search: Record) => ({ + version: Number(search.version), + }), + loaderDeps: ({ search }) => ({ version: search.version }), + loader: ({ deps }) => deps.version, + }) + const router = createTestRouter({ + routeTree: rootRoute.addChildren([sourceRoute, targetRoute]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + + const first = router.preloadRoute({ + to: '/source', + search: { version: 1 }, + }) + await loaderStarted + const second = router.preloadRoute({ + to: '/source', + search: { version: 2 }, + }) + await vi.waitFor(() => expect(sourceLoader).toHaveBeenCalledOnce()) + + loaderGate.resolve( + redirect({ to: '/target', search: searchUpdater } as any), + ) + const [firstMatches, secondMatches] = await Promise.all([first, second]) + + expect(sourceLoader).toHaveBeenCalledOnce() + expect(searchUpdater).toHaveBeenCalledTimes(2) + expect(firstMatches?.at(-1)).toMatchObject({ + routeId: targetRoute.id, + loaderData: 1, + }) + expect(secondMatches?.at(-1)).toMatchObject({ + routeId: targetRoute.id, + loaderData: 2, + }) + }) +}) diff --git a/packages/solid-router/src/link.tsx b/packages/solid-router/src/link.tsx index e24e32fd79a..635d4da81e8 100644 --- a/packages/solid-router/src/link.tsx +++ b/packages/solid-router/src/link.tsx @@ -251,12 +251,15 @@ export function useLinkProps< }) const doPreload = () => - router - .preloadRoute({ ...options, _builtLocation: next() } as any) - .catch((err: any) => { - console.warn(err) - console.warn(preloadWarning) - }) + ( + router.preloadRoute as ( + opts: typeof options, + builtLocation: ReturnType, + ) => ReturnType + )(options, next()).catch((err: any) => { + console.warn(err) + console.warn(preloadWarning) + }) const [ref, setRef] = Solid.createSignal(null) diff --git a/packages/solid-router/tests/link.test.tsx b/packages/solid-router/tests/link.test.tsx index 968d2642736..c1c602775c9 100644 --- a/packages/solid-router/tests/link.test.tsx +++ b/packages/solid-router/tests/link.test.tsx @@ -5221,6 +5221,7 @@ describe('Link', () => { ) test('Link.preload="intent" should preload on focus, hover, and touchstart', async () => { + const updateSearch = vi.fn((search) => search) const rootRoute = createRootRoute() const indexRoute = createRoute({ getParentRoute: () => rootRoute, @@ -5228,7 +5229,7 @@ describe('Link', () => { component: () => ( <>

Index Heading

- + About Link @@ -5255,6 +5256,8 @@ describe('Link', () => { expect(aboutLink).toBeInTheDocument() const baselineCalls = preloadRouteSpy.mock.calls.length + const baselineSearchCalls = updateSearch.mock.calls.length + expect(baselineSearchCalls).toBeGreaterThan(0) fireEvent.focus(aboutLink) await waitFor(() => @@ -5270,6 +5273,7 @@ describe('Link', () => { await waitFor(() => expect(preloadRouteSpy).toHaveBeenCalledTimes(baselineCalls + 3), ) + expect(updateSearch).toHaveBeenCalledTimes(baselineSearchCalls) }) test('Router.preload="intent", pendingComponent renders during unresolved route loader', async () => { diff --git a/packages/vue-router/src/link.tsx b/packages/vue-router/src/link.tsx index fb4b1ad37fc..a4aab312cfc 100644 --- a/packages/vue-router/src/link.tsx +++ b/packages/vue-router/src/link.tsx @@ -231,12 +231,15 @@ function useLinkPropsImpl( const doPreload = () => { const options = getOptions() - return router - .preloadRoute({ ...options, _builtLocation: next.value } as any) - .catch((err: any) => { - console.warn(err) - console.warn(preloadWarning) - }) + return ( + router.preloadRoute as ( + opts: typeof options, + builtLocation: ReturnType, + ) => ReturnType + )(options, next.value).catch((err: any) => { + console.warn(err) + console.warn(preloadWarning) + }) } let pendingPreload: 'intent' | 'viewport' | undefined diff --git a/packages/vue-router/tests/link.test.tsx b/packages/vue-router/tests/link.test.tsx index 1669682cb2d..1a2dd98bbe8 100644 --- a/packages/vue-router/tests/link.test.tsx +++ b/packages/vue-router/tests/link.test.tsx @@ -5661,6 +5661,7 @@ describe('Link', () => { ) test('Link.preload="intent" should preload on focus, hover, and touchstart', async () => { + const updateSearch = vi.fn((search) => search) const rootRoute = createRootRoute() const indexRoute = createRoute({ getParentRoute: () => rootRoute, @@ -5668,7 +5669,7 @@ describe('Link', () => { component: () => ( <>

Index Heading

- + About Link @@ -5695,6 +5696,8 @@ describe('Link', () => { expect(aboutLink).toBeInTheDocument() const baselineCalls = preloadRouteSpy.mock.calls.length + const baselineSearchCalls = updateSearch.mock.calls.length + expect(baselineSearchCalls).toBeGreaterThan(0) fireEvent.focus(aboutLink) await waitFor(() => @@ -5710,6 +5713,7 @@ describe('Link', () => { await waitFor(() => expect(preloadRouteSpy).toHaveBeenCalledTimes(baselineCalls + 3), ) + expect(updateSearch).toHaveBeenCalledTimes(baselineSearchCalls) }) test('Router.preload="intent", pendingComponent renders during unresolved route loader', async () => {