Skip to content

Commit 70ec1da

Browse files
fix(react-router): clear stale route errors on navigation (#7136)
Co-authored-by: schiller-manuel <6340397+schiller-manuel@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
1 parent 540d221 commit 70ec1da

3 files changed

Lines changed: 107 additions & 21 deletions

File tree

.changeset/ten-cougars-jump.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@tanstack/react-router': patch
3+
---
4+
5+
Fix a stale route error boundary state issue that could briefly render the next route's `errorComponent` after navigating away from a failed route.

packages/react-router/src/CatchBoundary.tsx

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -36,40 +36,34 @@ class CatchBoundaryImpl extends React.Component<{
3636
}) => React.ReactNode
3737
onCatch?: (error: Error, errorInfo: ErrorInfo) => void
3838
}> {
39-
state = { error: null } as { error: Error | null; resetKey: string }
40-
static getDerivedStateFromProps(props: any) {
41-
return { resetKey: props.getResetKey() }
39+
state = { error: null } as { error: Error | null; resetKey?: string | number }
40+
41+
static getDerivedStateFromProps(
42+
props: { getResetKey: () => string | number },
43+
state: { resetKey?: string | number; error: Error | null },
44+
) {
45+
const resetKey = props.getResetKey()
46+
47+
if (state.error && state.resetKey !== resetKey) {
48+
return { resetKey, error: null }
49+
}
50+
51+
return { resetKey }
4252
}
4353
static getDerivedStateFromError(error: Error) {
4454
return { error }
4555
}
4656
reset() {
4757
this.setState({ error: null })
4858
}
49-
componentDidUpdate(
50-
prevProps: Readonly<{
51-
getResetKey: () => string
52-
children: (props: { error: any; reset: () => void }) => any
53-
onCatch?: ((error: any, info: any) => void) | undefined
54-
}>,
55-
prevState: any,
56-
): void {
57-
if (prevState.error && prevState.resetKey !== this.state.resetKey) {
58-
this.reset()
59-
}
60-
}
6159
componentDidCatch(error: Error, errorInfo: ErrorInfo) {
6260
if (this.props.onCatch) {
6361
this.props.onCatch(error, errorInfo)
6462
}
6563
}
6664
render() {
67-
// If the resetKey has changed, don't render the error
6865
return this.props.children({
69-
error:
70-
this.state.resetKey !== this.props.getResetKey()
71-
? null
72-
: this.state.error,
66+
error: this.state.error,
7367
reset: () => {
7468
this.reset()
7569
},

packages/react-router/tests/errorComponent.test.tsx

Lines changed: 88 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'
2-
import { cleanup, fireEvent, render, screen } from '@testing-library/react'
2+
import { act, cleanup, fireEvent, render, screen } from '@testing-library/react'
33

44
import {
55
Link,
@@ -169,5 +169,92 @@ describe.each([true, false])(
169169
})
170170
},
171171
)
172+
173+
describe('stale route errors do not leak after navigation', () => {
174+
test.each([
175+
{
176+
caller: 'loader' as const,
177+
routeOptions: {
178+
loader: throwFn,
179+
},
180+
},
181+
{
182+
caller: 'render' as const,
183+
routeOptions: {
184+
component: function RenderErrorComponent() {
185+
throwFn()
186+
},
187+
},
188+
},
189+
])(
190+
'navigating away from a $caller error does not call the next route errorComponent',
191+
async ({ routeOptions }) => {
192+
const rootRoute = createRootRoute()
193+
const indexErrorComponent = vi.fn(() => <div>Index error</div>)
194+
195+
const indexRoute = createRoute({
196+
getParentRoute: () => rootRoute,
197+
path: '/',
198+
component: function Home() {
199+
return (
200+
<div>
201+
<div>Index route content</div>
202+
<Link to="/about">link to about</Link>
203+
</div>
204+
)
205+
},
206+
errorComponent: indexErrorComponent,
207+
})
208+
209+
const aboutRoute = createRoute({
210+
getParentRoute: () => rootRoute,
211+
path: '/about',
212+
component: function About() {
213+
return <div>About route content</div>
214+
},
215+
errorComponent: isUsingLazyError ? undefined : MyErrorComponent,
216+
...routeOptions,
217+
})
218+
219+
if (isUsingLazyError) {
220+
aboutRoute.lazy(() =>
221+
Promise.resolve(
222+
createLazyRoute('/about')({
223+
errorComponent: MyErrorComponent,
224+
}),
225+
),
226+
)
227+
}
228+
229+
const routeTree = rootRoute.addChildren([indexRoute, aboutRoute])
230+
231+
const router = createRouter({
232+
routeTree,
233+
history,
234+
})
235+
236+
render(<RouterProvider router={router} />)
237+
238+
const linkToAbout = await screen.findByRole('link', {
239+
name: 'link to about',
240+
})
241+
242+
await act(() => fireEvent.click(linkToAbout))
243+
244+
expect(
245+
await screen.findByText('Error: error thrown', undefined, {
246+
timeout: 1500,
247+
}),
248+
).toBeInTheDocument()
249+
250+
await act(() => router.navigate({ to: '/' }))
251+
252+
expect(
253+
await screen.findByText('Index route content'),
254+
).toBeInTheDocument()
255+
expect(indexErrorComponent).not.toHaveBeenCalled()
256+
},
257+
)
258+
})
172259
},
173260
)

0 commit comments

Comments
 (0)