Check for loading auth before rendering apollo provider #1945
Conversation
| } = useAuth() | ||
|
|
||
| if (loading) { | ||
| return null |
There was a problem hiding this comment.
I don't want to stop rendering here. I want rendering to continue down to the router so it can show the whileLoading() stuff.
There was a problem hiding this comment.
redwood/packages/router/src/router.tsx
Lines 65 to 67 in 42ec5eb
There was a problem hiding this comment.
@Tobbe I tried putting in the same whileLoading logic on the public routes in router.tsx as well and even used a private route in this issue and in both cases I end up stuck on the "/" route even though the address bar shows "/after-auth" as I demonstrated in #1892
So far this was the only thing that fixed the issue.
redwood/packages/web/src/components/RedwoodProvider.tsx
Lines 40 to 45 in 53349fd
Based on this comment from 0.21 someone has always wanted it removed but I've pretty much narrowed it down to the one thing causing both issues in #1892.
There was a problem hiding this comment.
@Tobbe I think the real issue here is probably since rendering is no longer blocked in the RedwoodProvider during auth loading, Location sets it's context before replaceState is called in restoreAuthState and then doesn't reset it's context since replaceState doesn't trigger the popstate event. I've seen some workarounds for this:
Screenshot of the order of events:
"Router Loading" logs the loading prop and pathname on useAuth within RouterImpl.
There was a problem hiding this comment.
Thanks for looking in to this @o0charlie0o What you're describing explains a few odd things I've noticed with auth state as well. So I'd love to get to the bottom of this.
15 minutes later...
So I've been digging through the code, and I think I might have a solution. I'm too tired right now to keep everything in my head, so I'll have to actually try it out to know if it'll work or not. I'll try to verify my idea tomorrow. In the meantime, can you help me by describing the absolute simplest way to reproduce this issue in a newly created RW app? Or if you can set up a repo I can clone that shows the problem. Thanks!
There was a problem hiding this comment.
I have cloned your repo now, and reproduced the issue.
Next step for me is to see if I can fix it :)
There was a problem hiding this comment.
I've spent some time working on this. I have also talked to @peterp about it. I have a way forward, but it's a lot of work, and restructuring of the RW code. Got stuck on a timing issue inside the auth code. Peter said he would think about it, see if he could come up with a good solution.
Basically I've created a new package, @redwoodjs/history that lifts that stuff higher up, so it can be used from both auth and router.
Unfortunately I don't think I will have time to finish this in the current sprint.
There was a problem hiding this comment.
@Tobbe thanks for the update!
There was a problem hiding this comment.
Hi gents, confirming we'll be pausing PR merges for this Sprint by the end-of-day tomorrow. Tomorrow will be focused on the final review and QA for PRs in this Sprint.
I'll move this as a priority for the next Sprint. Also, I'm working on improving our patch release process, which means hopefully we could get this out as a patch as soon as it's ready. TBD
Thanks for keeping this moving forward. Definitely important!
|
@Tobbe given the "go ahead" on /history, what is the next step with this PR. @o0charlie0o absolutely HUGE thank you for your work here and overall patience with us! |
|
Work continued in #2212 |
Fixes #1892