Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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: 5 additions & 0 deletions .changeset/eight-feet-lose.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@tanstack/router-core': patch
---

Fixed aborted loader executions clearing promises owned by a newer load of the same route match.
94 changes: 94 additions & 0 deletions packages/react-router/tests/loaders.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -940,3 +940,97 @@ test('reproducer for #6388 - rapid navigation between parameterized routes shoul
expect(paramPage).toHaveTextContent('Param Component 1 Done')
expect(loaderCompleteMock).toHaveBeenCalled()
})

test('aborting a reused parent match does not clear the replacement load promise', async () => {
const rootAbortMock = vi.fn()
const rootLoaderMock = vi.fn()
const indexAbortMock = vi.fn()
const errorComponentMock = vi.fn()

const abortableDelay = (
abortController: AbortController,
onAbort: () => void,
) =>
new Promise<void>((resolve, reject) => {
const timer = setTimeout(resolve, WAIT_TIME)
abortController.signal.addEventListener('abort', () => {
clearTimeout(timer)
onAbort()
reject(
new DOMException('signal is aborted without reason', 'AbortError'),
)
})
})

const rootRoute = createRootRoute({
loader: async ({ abortController }) => {
rootLoaderMock()
await abortableDelay(abortController, rootAbortMock)
return 'root loaded'
},
shouldReload: true,
component: Outlet,
errorComponent: ({ error }) => {
errorComponentMock(error)
return <div data-testid="route-error">{error.message}</div>
},
})

const indexRoute = createRoute({
getParentRoute: () => rootRoute,
path: '/',
validateSearch: (search): { filter: string } => ({
filter: typeof search.filter === 'string' ? search.filter : '',
}),
loaderDeps: ({ search }) => ({ filter: search.filter }),
loader: async ({ deps, abortController }) => {
await abortableDelay(abortController, indexAbortMock)
return deps.filter
},
component: () => {
const data = indexRoute.useLoaderData()
const search = indexRoute.useSearch()
const navigate = indexRoute.useNavigate()
return (
<div data-testid="index-page">
<div data-testid="index-loader-data">{data}</div>
<input
data-testid="filter-input"
value={search.filter}
onChange={(event) => {
void navigate({
to: '/',
search: { filter: event.target.value },
})
}}
/>
</div>
)
},
})

const router = createRouter({
routeTree: rootRoute.addChildren([indexRoute]),
history,
})

render(<RouterProvider router={router} />)
await act(() => router.latestLoadPromise)

const input = await screen.findByTestId('filter-input')
for (const filter of ['a', 'ab', 'abc', 'abcd', 'abcde', 'abcdef']) {
fireEvent.change(input, { target: { value: filter } })
}
await act(() => router.latestLoadPromise)

expect(rootLoaderMock.mock.calls.length).toBeGreaterThan(1)
expect(rootAbortMock).toHaveBeenCalled()
expect(indexAbortMock).toHaveBeenCalled()
expect(errorComponentMock).not.toHaveBeenCalled()
Comment thread
LadyBluenotes marked this conversation as resolved.
expect(screen.queryByTestId('route-error')).not.toBeInTheDocument()
expect(await screen.findByTestId('index-page')).toBeInTheDocument()
expect(await screen.findByTestId('index-loader-data')).toHaveTextContent(
'abcdef',
)
expect(router.state.location.search).toEqual({ filter: 'abcdef' })
})
44 changes: 30 additions & 14 deletions packages/router-core/src/load-matches.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import type {
} from './route'
import type { AnyRouteMatch, MakeRouteMatch } from './Matches'
import type { AnyRouter, SSROption, UpdateMatchFn } from './router'
import type { ControlledPromise } from './utils'

/**
* An object of this shape is created when calling `loadMatches`.
Expand All @@ -35,6 +36,7 @@ type InnerLoadContext = {
forceStaleReload?: boolean
onReady?: () => Promise<void>
sync?: boolean
loadPromises: Array<ControlledPromise<void> | undefined>
}

const triggerOnReady = (inner: InnerLoadContext): void | Promise<void> => {
Expand Down Expand Up @@ -395,10 +397,12 @@ const executeBeforeLoad = (

// explicitly capture the previous loadPromise
let prevLoadPromise = match._nonReactive.loadPromise
match._nonReactive.loadPromise = createControlledPromise<void>(() => {
const loadPromise = createControlledPromise<void>(() => {
prevLoadPromise?.resolve()
prevLoadPromise = undefined
})
match._nonReactive.loadPromise = loadPromise
inner.loadPromises[index] = loadPromise

const { paramsError, searchError } = match

Expand Down Expand Up @@ -727,8 +731,7 @@ const runLoader = async (

if ((error as any)?.name === 'AbortError') {
if (match.abortController.signal.aborted) {
match._nonReactive.loaderPromise?.resolve()
match._nonReactive.loaderPromise = undefined
// loadRouteMatch resolves the promises owned by this execution.
return
}
inner.updateMatch(matchId, (prev) => ({
Expand Down Expand Up @@ -835,11 +838,13 @@ const loadRouteMatch = async (
;(async () => {
try {
await runLoader(inner, matchPromises, matchId, index, route)
const match = inner.router.getMatch(matchId)!
match._nonReactive.loaderPromise?.resolve()
match._nonReactive.loadPromise?.resolve()
match._nonReactive.loaderPromise = undefined
match._nonReactive.loadPromise = undefined
loaderPromise?.resolve()
loadPromise?.resolve()
const nonReactive = inner.router.getMatch(matchId)?._nonReactive
if (nonReactive && nonReactive.loadPromise === loadPromise) {
nonReactive.loaderPromise = undefined
nonReactive.loadPromise = undefined
}
} catch (err) {
if (isRedirect(err)) {
await inner.router.navigate(err.options)
Expand All @@ -854,6 +859,8 @@ const loadRouteMatch = async (
}

const { id: matchId, routeId } = inner.matches[index]!
const loadPromise = inner.loadPromises[index]
let loaderPromise: ControlledPromise<void> | undefined
let loaderShouldRunAsync = false
let loaderIsRunningAsync = false
const route = inner.router.looseRoutesById[routeId]!
Expand Down Expand Up @@ -910,6 +917,8 @@ const loadRouteMatch = async (
}

if (match.status === 'pending') {
loaderPromise = createControlledPromise<void>()
match._nonReactive.loaderPromise = loaderPromise
await handleLoader(
preload,
prevMatch,
Expand All @@ -922,7 +931,8 @@ const loadRouteMatch = async (
const nextPreload =
preload && !inner.router.stores.matchStores.has(matchId)
const match = inner.router.getMatch(matchId)!
match._nonReactive.loaderPromise = createControlledPromise<void>()
loaderPromise = createControlledPromise<void>()
match._nonReactive.loaderPromise = loaderPromise
if (nextPreload !== match.preload) {
inner.updateMatch(matchId, (prev) => ({
...prev,
Expand All @@ -935,14 +945,20 @@ const loadRouteMatch = async (
}
const match = inner.router.getMatch(matchId)!
if (!loaderIsRunningAsync) {
match._nonReactive.loaderPromise?.resolve()
match._nonReactive.loadPromise?.resolve()
match._nonReactive.loadPromise = undefined
loaderPromise?.resolve()
loadPromise?.resolve()
}

if (match._nonReactive.loadPromise !== loadPromise) {
return match
}

if (!loaderIsRunningAsync) {
match._nonReactive.loadPromise = undefined
match._nonReactive.loaderPromise = undefined
}
clearTimeout(match._nonReactive.pendingTimeout)
match._nonReactive.pendingTimeout = undefined
if (!loaderIsRunningAsync) match._nonReactive.loaderPromise = undefined
match._nonReactive.dehydrated = undefined

const nextIsFetching = loaderIsRunningAsync ? match.isFetching : false
Expand All @@ -968,7 +984,7 @@ export async function loadMatches(arg: {
updateMatch: UpdateMatchFn
sync?: boolean
}): Promise<Array<MakeRouteMatch>> {
const inner: InnerLoadContext = arg
const inner: InnerLoadContext = { ...arg, loadPromises: [] }
const matchPromises: Array<Promise<AnyRouteMatch>> = []

// make sure the pending component is immediately rendered when hydrating a match that is not SSRed
Expand Down
Loading