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.19k stars 486 forks source link

`beforeLoad` does not wait for async functions to resolve #1553

Closed MelB-24 closed 4 days ago

MelB-24 commented 2 weeks ago

Describe the bug

I have the same setup as the Authenticated Routes examples, but with firebase authentication. I have my auth in context, with a login and logout function. I have an _auth.tsx file in the routes folder. Then a _auth folder containing all protected routes.

When the auth context is set in localStorage, same as the example, routing happens as expected. However when wanting to set the user from the result of the signIn function from firebase the beforeLoad does not wait for the result and the context in the _auth file remains unchanged.

Your Example Website or App

-

Steps to Reproduce the Bug or Issue

With the same setup as the Authenticated Routes example, the only change required would be to have something like firebase authentication and then changing setStoredUser(username) to

const loggedInUser = await signIn(email, password);
setStoredUser(loggedInUser)

Expected behavior

The beforeLoad should allow for resolved async values being set into context

Screenshots or Videos

Screenshot 2024-05-03 at 12 06 25 PM

Platform

Additional context

No response

SeanCassiere commented 2 weeks ago

It is worth checking this out in the loader as well since #1551 has a similar issue where if the loader was awaited on correctly, then the correct component would have been rendered.

Yoni-apex commented 1 week ago

I have the exact same issue for the exact same case. Would love a good answer there <3

christofferbergj commented 1 week ago

I believe we have the same issue. Awaiting promises with Tanstack Query (ensureQueryData) does not resolve.

CleanShot 2024-05-07 at 16 05 07@2x

CleanShot 2024-05-07 at 16 06 15@2x

lecstor commented 1 week ago

Are you using React.StrictMode? as mentioned in another issue, disabling it seems to resolve the issue for me.

frederikvonsperling commented 1 week ago

Sandbox to reproduce: https://codesandbox.io/p/devbox/tanstack-router-before-load-with-query-8d9lcd

/posts/ works as expected

SeanCassiere commented 4 days ago

@frederikvonsperling I imported your example into Stackblitz and it looks to be working with 1.32.5.

https://stackblitz.com/edit/github-h31abz?file=src%2Fmain.tsx

Can this please be retested.

frederikvonsperling commented 4 days ago

@frederikvonsperling I imported your example into Stackblitz and it looks to be working with 1.32.5.

https://stackblitz.com/edit/github-h31abz?file=src%2Fmain.tsx

Can this please be retested.

@SeanCassiere Same issue persists in 1.32.5 https://codesandbox.io/p/devbox/tanstack-router-before-load-with-query-8d9lcd

going to /posts without trailing slash gives a white screen of death

SeanCassiere commented 4 days ago

Looked further into this. The beforeLoad not being awaited has NOT been fixed.

But a new bug has been found, as mentioned by @frederikvonsperling.

The new bug found by @frederikvonsperling involves a combination of the trailingSlash setting and React.StrictMode.

I'll open a new issue for this new bug shortly.

Edit: This has been fixed.

SeanCassiere commented 4 days ago

The issue here is entirely related to the trailingSlash issue in combination with React.StrictMode.

For example, consider the scenario where trailingSlash is set to never with React.StrictMode ON.

  1. When visiting /posts/ the URL then gets written to /posts.
  2. However, a loader which instantly resolves, finishes before the beforeLoad call.
  3. A white-screen is then shown.

Also, when trailingSlash is set to always with React.StrictMode ON.

  1. When visiting /posts the URL then gets written to /posts/.
  2. However, a loader which instantly resolves, finishes before the beforeLoad call.
  3. A white-screen is then shown.

I've tested these behaviour here: https://stackblitz.com/edit/github-h31abz-hjnco9?file=src%2Fmain.tsx


📢 This doesn't happen when the trailingSlash behaviour is set to preserve.

🧱 The default trailingSlash behaviour is never, so the beforeLoad not being awaited is what users thought was happening.


This is the created issue: #1591

SeanCassiere commented 4 days ago

Closing this as the correct bug shall be tracked in the issue mentioned.

nwi-di commented 1 day ago

@SeanCassiere Sorry, but for me this issue still exists and is not related to trailingSlash... As long as an async operation is made, the beforeLoad function of the route gets called before updating the route context. Without the async operation the context is first updated and then the beforeLoad gets called with fresh context...

See this repo for reproduction: CodeSandbox

Steps to reproduce:

Tested with latest (1.32.12) @tanstack/react-router

SeanCassiere commented 1 day ago

@nwi-di the beforeLoad is working correctly.

It's just a useAuth hook that's not production-ready and you not awaiting your auth.login call.

https://codesandbox.io/p/devbox/nostalgic-cloud-d5s8zx?file=%2Fsrc%2Fauth.tsx%3A7%2C31

Edit: TLDR; The problem you are facing in your repro is not the beforeLoad callback's execution being awaited, rather an async execution in React-land not being awaited.

nwi-di commented 1 day ago

Edit: TLDR; The problem you are facing in your repro is not the beforeLoad callback's execution being awaited, rather an async execution in React-land not being awaited.

So, you mean I need to await the auth.login call in login.tsx (line 43) as the login method is returning a Promise now? But either I am doing it wrong or it do not see / understand it correct. I updated the linked sandbox according to that but without difference 🤔

jmargeta commented 1 day ago

Note that running the codesandbox example above, the "Login" button has to be clicked twice to actually do anything but then I only get "Route.useLoaderData() is undefined":

in Firefox: image in Edge: image

SeanCassiere commented 1 day ago

The CSB link I posted earlier didn't seems to have all the changes I actually made. CSB has issues...

Either ways I've updated the "authenticated routes" example on main.

Once again, what you are experiencing is NOT about an asynchronous beforeLoad function not being awaited by Router. What you are experiencing is entirely in React-land and outside of the beforeLoad function's execution context.

Also, a weird aside, non of the issues you've mentioned show up when testing with Stackblitz, pretty much only in CSB.

nwi-di commented 1 day ago

Thanks for updating the example on main. I do not know why CSB has these issues at the moment...

Ok, to get things in the right order, I am trying to understand the issue here in more detail... Due to the asynchronous request in the login function, the react state updates are put to the JS jobs queue, right? Without the asynchronous request they remain on the call stack and are handled directly, but with asynchronous request they are handled later. So here is one difference, right?

Due to that, in the following promise chaining of the onFormSubmit the router invalidation and navigation are done and once they are finished, react updates the states (from the jobs queue).

With the "sleep" you put into the example, the router invalidation and navigation are put behind the react updates (due to the setTimeout), right? To make this work, the "sleep" must be placed after the call to auth.login and before navigate.

SeanCassiere commented 1 day ago

With the "sleep" you put into the example, the router invalidation and navigation are put behind the react updates (due to the setTimeout), right? To make this work, the "sleep" must be placed after the call to auth.login and before navigate.

You are more-or-less on the right track. React concurrent mode is also at play here, so the way it handles the batching of state updates can also come into play.

I don't really use these kinds of auth flows in my apps, rather relying on oauth2 flows instead. So this is just a simple auth flow that I came up with a few months back just to demonstrate the router concepts.

Don't look to copy the auth flow in the example. Rather just look at the concepts behind the usage of the Router's context, throws in the beforeLoad, and the usage of router.invalidate, and then adapt it to whatever your app needs are.