Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion docs/router/framework/react/api/router/RouteOptionsType.md
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,9 @@ type loader = (
location: ParsedLocation
params: TAllParams
preload: boolean
parentMatchPromise: Promise<MakeRouteMatchFromRoute<TParentRoute>>
parentMatchPromise:
| Promise<MakeRouteMatchFromRoute<TParentRoute>>
| MakeRouteMatchFromRoute<TParentRoute>
navigate: NavigateFn<AnyRoute> // @deprecated
route: AnyRoute
},
Expand All @@ -144,6 +146,7 @@ type loader = (
- If this function returns a promise, the route will be put into a pending state and cause rendering to suspend until the promise resolves. If this route's pendingMs threshold is reached, the `pendingComponent` will be shown until it resolves. If the promise rejects, the route will be put into an error state and the error will be thrown during render.
- If this function returns a `TLoaderData` object, that object will be stored on the route match until the route match is no longer active. It can be accessed using the `useLoaderData` hook in any component that is a child of the route match before another `<Outlet />` is rendered.
- Deps must be returned by your `loaderDeps` function in order to appear.
- `parentMatchPromise` is a promise _if_ the parent route's `loader` returns a promise as well, otherwise it is the resolved parent match object.

> 🚧 `opts.navigate` has been deprecated and will be removed in the next major release. Use `throw redirect({ to: '/somewhere' })` instead. Read more about the `redirect` function [here](../redirectFunction.md).

Expand Down
235 changes: 129 additions & 106 deletions packages/router-core/src/load-matches.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { createControlledPromise, isPromise } from './utils'
import { isNotFound } from './not-found'
import { rootRouteId } from './root'
import { isRedirect } from './redirect'
import type { Awaitable } from './utils'
import type { NotFoundError } from './not-found'
import type { ParsedLocation } from './location'
import type {
Expand Down Expand Up @@ -34,7 +35,7 @@ type InnerLoadContext = {
onReady?: () => Promise<void>
sync?: boolean
/** mutable state, scoped to a `loadMatches` call */
matchPromises: Array<Promise<AnyRouteMatch>>
matchPromises: Array<Awaitable<AnyRouteMatch>>
}

const triggerOnReady = (inner: InnerLoadContext): void | Promise<void> => {
Expand Down Expand Up @@ -703,10 +704,10 @@ const runLoader = async (
}
}

const loadRouteMatch = async (
const loadRouteMatch = (
inner: InnerLoadContext,
index: number,
): Promise<AnyRouteMatch> => {
): Awaitable<AnyRouteMatch> => {
const { id: matchId, routeId } = inner.matches[index]!
let loaderShouldRunAsync = false
let loaderIsRunningAsync = false
Expand All @@ -716,121 +717,140 @@ const loadRouteMatch = async (
if (inner.router.isServer) {
const headResult = executeHead(inner, matchId, route)
if (headResult) {
const head = await headResult
inner.updateMatch(matchId, (prev) => ({
...prev,
...head,
}))
return headResult.then((head) => {
inner.updateMatch(matchId, (prev) => ({
...prev,
...head,
}))
return inner.router.getMatch(matchId)!
})
}
return inner.router.getMatch(matchId)!
}
} else {
const prevMatch = inner.router.getMatch(matchId)!
// there is a loaderPromise, so we are in the middle of a load
if (prevMatch._nonReactive.loaderPromise) {
// do not block if we already have stale data we can show
// but only if the ongoing load is not a preload since error handling is different for preloads
// and we don't want to swallow errors
if (prevMatch.status === 'success' && !inner.sync && !prevMatch.preload) {
return prevMatch
}
await prevMatch._nonReactive.loaderPromise
return settleLoadRouteMatch()
}

const prevMatch = inner.router.getMatch(matchId)!

// there is a loaderPromise, so we are in the middle of a load
if (prevMatch._nonReactive.loaderPromise) {
// do not block if we already have stale data we can show
// but only if the ongoing load is not a preload since error handling is different for preloads
// and we don't want to swallow errors
if (prevMatch.status === 'success' && !inner.sync && !prevMatch.preload) {
return prevMatch
}
return prevMatch._nonReactive.loaderPromise.then(() => {
const match = inner.router.getMatch(matchId)!
if (match.error) {
handleRedirectAndNotFound(inner, match, match.error)
}
} else {
// This is where all of the stale-while-revalidate magic happens
const age = Date.now() - prevMatch.updatedAt

const preload = resolvePreload(inner, matchId)

const staleAge = preload
? (route.options.preloadStaleTime ??
inner.router.options.defaultPreloadStaleTime ??
30_000) // 30 seconds for preloads by default
: (route.options.staleTime ??
inner.router.options.defaultStaleTime ??
0)

const shouldReloadOption = route.options.shouldReload

// Default to reloading the route all the time
// Allow shouldReload to get the last say,
// if provided.
const shouldReload =
typeof shouldReloadOption === 'function'
? shouldReloadOption(getLoaderContext(inner, matchId, index, route))
: shouldReloadOption

const nextPreload =
!!preload && !inner.router.state.matches.some((d) => d.id === matchId)
const match = inner.router.getMatch(matchId)!
match._nonReactive.loaderPromise = createControlledPromise<void>()
if (nextPreload !== match.preload) {
inner.updateMatch(matchId, (prev) => ({
...prev,
preload: nextPreload,
}))
}
return settleLoadRouteMatch()
})
}

// This is where all of the stale-while-revalidate magic happens
const age = Date.now() - prevMatch.updatedAt

const preload = resolvePreload(inner, matchId)

const staleAge = preload
? (route.options.preloadStaleTime ??
inner.router.options.defaultPreloadStaleTime ??
30_000) // 30 seconds for preloads by default
: (route.options.staleTime ?? inner.router.options.defaultStaleTime ?? 0)

const shouldReloadOption = route.options.shouldReload

// Default to reloading the route all the time
// Allow shouldReload to get the last say,
// if provided.
const shouldReload =
typeof shouldReloadOption === 'function'
? shouldReloadOption(getLoaderContext(inner, matchId, index, route))
: shouldReloadOption

const nextPreload =
!!preload && !inner.router.state.matches.some((d) => d.id === matchId)
const match = inner.router.getMatch(matchId)!
match._nonReactive.loaderPromise = createControlledPromise<void>()
if (nextPreload !== match.preload) {
inner.updateMatch(matchId, (prev) => ({
...prev,
preload: nextPreload,
}))
}

// If the route is successful and still fresh, just resolve
const { status, invalid } = match
loaderShouldRunAsync =
status === 'success' && (invalid || (shouldReload ?? age > staleAge))
if (preload && route.options.preload === false) {
// Do nothing
} else if (loaderShouldRunAsync && !inner.sync) {
loaderIsRunningAsync = true
;(async () => {
try {
await runLoader(inner, matchId, index, route)
const match = inner.router.getMatch(matchId)!
match._nonReactive.loaderPromise?.resolve()
match._nonReactive.loadPromise?.resolve()
match._nonReactive.loaderPromise = undefined
} catch (err) {
if (isRedirect(err)) {
await inner.router.navigate(err.options)
}
}
})()
} else if (status !== 'success' || (loaderShouldRunAsync && inner.sync)) {
if (preload && route.options.preload === false) {
// Do nothing
return settleLoadRouteMatch()
}

// If the route is successful and still fresh, just resolve
const { status, invalid } = match
loaderShouldRunAsync =
status === 'success' && (invalid || (shouldReload ?? age > staleAge))
if (loaderShouldRunAsync && !inner.sync) {
loaderIsRunningAsync = true
;(async () => {
try {
await runLoader(inner, matchId, index, route)
} else {
// if the loader did not run, still update head.
// reason: parent's beforeLoad may have changed the route context
// and only now do we know the route context (and that the loader would not run)
const headResult = executeHead(inner, matchId, route)
if (headResult) {
const head = await headResult
inner.updateMatch(matchId, (prev) => ({
...prev,
...head,
}))
const match = inner.router.getMatch(matchId)!
match._nonReactive.loaderPromise?.resolve()
match._nonReactive.loadPromise?.resolve()
match._nonReactive.loaderPromise = undefined
} catch (err) {
if (isRedirect(err)) {
await inner.router.navigate(err.options)
}
}
}
})()
return settleLoadRouteMatch()
}
Comment on lines +791 to 808
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Avoid unhandled rejections during background redirects.

The async IIFE isn’t observed; awaiting inner.router.navigate(...) can leak unhandled rejections if navigation fails. Fire-and-forget instead.

-      } catch (err) {
-        if (isRedirect(err)) {
-          await inner.router.navigate(err.options)
-        }
-      }
+      } catch (err) {
+        if (isRedirect(err)) {
+          // Detach to avoid unhandled rejections in the background task
+          void inner.router.navigate(err.options)
+        }
+      }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
status === 'success' && (invalid || (shouldReload ?? age > staleAge))
if (loaderShouldRunAsync && !inner.sync) {
loaderIsRunningAsync = true
;(async () => {
try {
await runLoader(inner, matchId, index, route)
} else {
// if the loader did not run, still update head.
// reason: parent's beforeLoad may have changed the route context
// and only now do we know the route context (and that the loader would not run)
const headResult = executeHead(inner, matchId, route)
if (headResult) {
const head = await headResult
inner.updateMatch(matchId, (prev) => ({
...prev,
...head,
}))
const match = inner.router.getMatch(matchId)!
match._nonReactive.loaderPromise?.resolve()
match._nonReactive.loadPromise?.resolve()
match._nonReactive.loaderPromise = undefined
} catch (err) {
if (isRedirect(err)) {
await inner.router.navigate(err.options)
}
}
}
})()
return settleLoadRouteMatch()
}
status === 'success' && (invalid || (shouldReload ?? age > staleAge))
if (loaderShouldRunAsync && !inner.sync) {
loaderIsRunningAsync = true
;(async () => {
try {
await runLoader(inner, matchId, index, route)
const match = inner.router.getMatch(matchId)!
match._nonReactive.loaderPromise?.resolve()
match._nonReactive.loadPromise?.resolve()
match._nonReactive.loaderPromise = undefined
} catch (err) {
if (isRedirect(err)) {
// Detach to avoid unhandled rejections in the background task
void inner.router.navigate(err.options)
}
}
})()
return settleLoadRouteMatch()
}
🤖 Prompt for AI Agents
In packages/router-core/src/load-matches.ts around lines 784 to 801, the async
IIFE currently does "await inner.router.navigate(err.options)" inside the catch
block which can leak unhandled rejections when navigation fails; instead call
navigate in a fire-and-forget manner and ensure any rejection is handled locally
(e.g. invoke inner.router.navigate(err.options).catch(() => {/* optional log
*/}) or drop the await and attach a .catch handler) so the background redirect
cannot create an unhandled rejection.

const match = inner.router.getMatch(matchId)!
if (!loaderIsRunningAsync) {
match._nonReactive.loaderPromise?.resolve()
match._nonReactive.loadPromise?.resolve()

if (status !== 'success' || (loaderShouldRunAsync && inner.sync)) {
return runLoader(inner, matchId, index, route).then(settleLoadRouteMatch)
}

clearTimeout(match._nonReactive.pendingTimeout)
match._nonReactive.pendingTimeout = undefined
if (!loaderIsRunningAsync) match._nonReactive.loaderPromise = undefined
match._nonReactive.dehydrated = undefined
const nextIsFetching = loaderIsRunningAsync ? match.isFetching : false
if (nextIsFetching !== match.isFetching || match.invalid !== false) {
inner.updateMatch(matchId, (prev) => ({
...prev,
isFetching: nextIsFetching,
invalid: false,
}))
return inner.router.getMatch(matchId)!
} else {
// if the loader did not run, still update head.
// reason: parent's beforeLoad may have changed the route context
// and only now do we know the route context (and that the loader would not run)
const headResult = executeHead(inner, matchId, route)
if (headResult) {
return headResult.then((head) => {
inner.updateMatch(matchId, (prev) => ({
...prev,
...head,
}))
return settleLoadRouteMatch()
})
}

return settleLoadRouteMatch()

function settleLoadRouteMatch() {
const match = inner.router.getMatch(matchId)!

if (!loaderIsRunningAsync) {
match._nonReactive.loaderPromise?.resolve()
match._nonReactive.loadPromise?.resolve()
match._nonReactive.loaderPromise = undefined
}

clearTimeout(match._nonReactive.pendingTimeout)
match._nonReactive.pendingTimeout = undefined
match._nonReactive.dehydrated = undefined

const nextIsFetching = loaderIsRunningAsync ? match.isFetching : false
if (nextIsFetching !== match.isFetching || match.invalid !== false) {
inner.updateMatch(matchId, (prev) => ({
...prev,
isFetching: nextIsFetching,
invalid: false,
}))
return inner.router.getMatch(matchId)!
}

return match
}
}
Expand Down Expand Up @@ -866,10 +886,13 @@ export async function loadMatches(arg: {

// Execute all loaders in parallel
const max = inner.firstBadMatchIndex ?? inner.matches.length
let hasPromises = false
for (let i = 0; i < max; i++) {
inner.matchPromises.push(loadRouteMatch(inner, i))
const result = loadRouteMatch(inner, i)
inner.matchPromises.push(result)
if (!hasPromises && isPromise(result)) hasPromises = true
}
await Promise.all(inner.matchPromises)
if (hasPromises) await Promise.all(inner.matchPromises)

const readyPromise = triggerOnReady(inner)
if (isPromise(readyPromise)) await readyPromise
Expand Down
10 changes: 9 additions & 1 deletion packages/router-core/src/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import type {
Expand,
IntersectAssign,
LooseAsyncReturnType,
LooseIsReturnPromise,
LooseReturnType,
NoInfer,
} from './utils'
Expand Down Expand Up @@ -421,6 +422,11 @@ export interface RouteTypes<
children: TChildren
loaderData: ResolveLoaderData<TLoaderFn>
loaderDeps: TLoaderDeps
asyncLoaderFn: unknown extends TLoaderFn
? boolean
: LooseIsReturnPromise<TLoaderFn> extends never
? boolean
: LooseIsReturnPromise<TLoaderFn>
fileRouteTypes: TFileRouteTypes
}

Expand Down Expand Up @@ -1236,7 +1242,9 @@ export interface LoaderFnContext<
// root route does not have a parent match
parentMatchPromise: TId extends RootRouteId
? never
: Promise<MakeRouteMatchFromRoute<TParentRoute>>
: TParentRoute['types']['asyncLoaderFn'] extends true
? Promise<MakeRouteMatchFromRoute<TParentRoute>>
: MakeRouteMatchFromRoute<TParentRoute>
cause: 'preload' | 'enter' | 'stay'
route: AnyRoute
}
Expand Down
8 changes: 8 additions & 0 deletions packages/router-core/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,14 @@ export type LooseAsyncReturnType<T> = T extends (
: TReturn
: never

export type LooseIsReturnPromise<T> = T extends (
...args: Array<any>
) => infer TReturn
? TReturn extends Promise<any>
? true
: false
: never

export function last<T>(arr: Array<T>) {
return arr[arr.length - 1]
}
Expand Down
Loading