Skip to content

fix: ensure child routes receive context returned from parent routes' beforeLoad functions, even if invalidate() is called during another load #4306

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jamesknelson
Copy link

@jamesknelson jamesknelson commented Jun 3, 2025

Possibly related:

I've been struggling with an issue where a parent route's context, as returned by beforeLoad, is missing (as in completely empty) in a child route, in certain circumstances, particular immediately after logging in. The types suggest this should be impossible, but there are circumstances in where it happens.

This PR adds a failing e2e test that reproduces the issue, and a tentative fix to the router itself.

Details

To add authentication to the router context, I'm passing a context prop into <RouterProvider>. This context is stored in React state, contains the current user, and is updated after login.

Also, in order to get ensure that the routes are re-loaded after log out, I've added a call to router.invalidate() in an effect within the same component.

const [authContext] = useAuth()

useEffect(() => {
  if (notInitialRender) {
    router.invalidate()
  }
}, [authContext])

return (
  <RouterProvider router={router} context={authContext} />
)

This works as expected when logging out. However, after logging in, the router is calling the loader() function of the new page without the context injected by its parent route's beforeLoad() function. In my app, this causes an error to be logged to the console on the first run through, and causes the app to crash on the second. This behavior is repeated in the e2e test I've added here.

Adding console.log() statements in various places, I've noticed that the beforeLoad() function is running before the child loader is called, so it's not that the context isn't available, it's just not being passed in.

While debugging, I noticed that invalidate() calls router.load(), even when the router is already loading, e.g. because of a navigate() call being made after login.

invalidate: InvalidateFn<
RouterCore<
TRouteTree,
TTrailingSlashOption,
TDefaultStructuralSharingOption,
TRouterHistory,
TDehydrated
>
> = (opts) => {
const invalidate = (d: MakeRouteMatch<TRouteTree>) => {
if (opts?.filter?.(d as MakeRouteMatchUnion<this>) ?? true) {
return {
...d,
invalid: true,
...(d.status === 'error'
? ({ status: 'pending', error: undefined } as const)
: {}),
}
}
return d
}
this.__store.setState((s) => ({
...s,
matches: s.matches.map(invalidate),
cachedMatches: s.cachedMatches.map(invalidate),
pendingMatches: s.pendingMatches?.map(invalidate),
}))
return this.load({ sync: opts?.sync })
}

I noticed that adding a guard to this router.load(), so that it is only called if the router is not already loading, fixed my issue, but I worry it would introduce further bugs down the track.

So the fix I ended up arriving on is to use the existing latestLoadPromise to wait for previous router.load() calls to complete before starting a new one, which resolved the issue for me, and got the new e2e test that I've added passing.

@jamesknelson jamesknelson force-pushed the avoid-double-load-on-invalidate branch from 1c5614b to e45ac7e Compare June 3, 2025 06:41
@jamesknelson jamesknelson changed the title fix: ensure child routes receive context returned from parent routes' beforeLoad functions, even after invalidate() is called fix: ensure child routes receive context returned from parent routes' beforeLoad functions, even if invalidate() is called during another load Jun 3, 2025
@schiller-manuel
Copy link
Contributor

can we test this in a unit test as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants