TanStack / router

🤖 Fully typesafe Router for React (and friends) w/ built-in caching, 1st class search-param APIs, client-side cache integration and isomorphic rendering.
https://tanstack.com/router
MIT License
7.51k stars 526 forks source link

Context is undefined in weird case. #1751

Closed Arilas closed 1 week ago

Arilas commented 1 month ago

Describe the bug

For some unknown reason context is undefined in loader, but it was present in beforeLoad

Your Example Website or App

https://github.com/Arilas/tanter-bug

Steps to Reproduce the Bug or Issue

  1. Clone
  2. yarn
  3. yarn dev
  4. Open Host
  5. Obserse error in src/routes/_secured.tsx
  6. Refresh the page
  7. Now it's working

Expected behavior

It should not clear the context

Screenshots or Videos

image

Platform

Additional context

I've over-simplified our case to the reproducible state. In our case we have a ServiceLocator with multiple factories, dependencies, events.

index.lazy.tsx can be replaced with an route with beforeLoad which will have redirects, but it was an workaround to fix problem previously.

mirus-ua commented 1 month ago

Yeah. I have the same issue

bgentry commented 1 month ago

I've just run into the same issue. In my case it is also where I'm throwing a redirect in my beforeLoad to redirect away from a bare route that has no search params to one which has a valid default param populated.

beforeLoad seems to get called again immediately after the redirect with the newly updated deps from loaderDeps, but with an undefined context value.

If I instead use the deprecated navigate arg in beforeLoad (rather than throw redirect({...})) everything works as expected, in case others are looking for a workaround.

bgentry commented 1 month ago

Actually, I just upgraded my app from v1.35.6 to v1.39.4 and I can no longer reproduce this issue. Seems like it may have been fixed @Arilas, maybe try your repo on the latest version and see if it's still broken?

bgentry commented 4 weeks ago

I take it back, I'm still encountering this issue. Not sure how I thought it was fixed last time.

For me, it happens in two case:

  1. When I load an initial route at / that renders a <Navigate /> redirect. The 2nd redirected route then has an undefined context. I updated my code to throw redirect instead of using <Navigate /> on my initial route, and that takes care of the issue.
  2. When my first file route /jobs/ throws a redirect in beforeLoad to the same route, but with updated an search value, the loader gets called an initial time with no context. This one gets worked around using the following and avoiding destructuring context as part of the loader function:
    loader: async ({ context, deps: { limit, state } }) => {
     if (!context) {
       // workaround for this issue:
       // https://github.com/TanStack/router/issues/1751
       return;
     }
     // ...

Although I didn't put together a simple reproduction, my workaround changes are visible in https://github.com/riverqueue/riverui/pull/80

schiller-manuel commented 3 weeks ago

this will be fixed by #1907