Skip to content

Invalidate all + redirects not working as expected. #6844

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

Closed
smblee opened this issue Sep 15, 2022 · 4 comments · Fixed by #6924
Closed

Invalidate all + redirects not working as expected. #6844

smblee opened this issue Sep 15, 2022 · 4 comments · Fixed by #6924

Comments

@smblee
Copy link

smblee commented Sep 15, 2022

Describe the bug

discord thread: https://discord.com/channels/457912077277855764/1020064864066093196

Hey all - I think i ran into a bug or unexpected behavior that i can't wrap my head around.

The app is structured so that the root / acts as a central "router" to determine current user's state and redirect them to appropriate landing pages.

The child pages also have their own redirection logic, and if user somehow got to the page directly, user is redirected back to the router /.

There is a notion of "profiles" and user can switch between the profiles using a switcher component (think AWS, switching between profiles with different permissions).

This component should
set the profile id in cookie from client side => invalidate/rerender the current page (client sided, via invalidateAll()) => if the current page has any redirection logic, redirect with new profile; if page can render with new profile, render with new data, etc.

The problem I am running into is, I call invalidateAll(), invalidation/rerunning nested layout code runs as expected (with the new profile id value), but at the very end of redirects, the profile id is seen as the previous value (which causes the "redirect loop" error, even when there really isn't a loop).

Reproduction

https://github.com/smblee/sveltekit-invalidate-all-bug

I reproduced it as close to my current repo as possible.

Say there are the following pages / (router, doesn't actually render), notset (ONLY when no profile set),signup (ONLY when profile is not-onboarded), onboarded-home (ONLY when profile is onboarded).

try going from Set not onboarded profile => Set onboarded profile or the other way around. (it will always fail on the first try, in one direction).

you will fail with the following log trace: (
it can seem a bit overwhelming, but it's pretty simple. I basically logged the location and current profile id value at that time in code. and any redirection that happens)

+layout |  not-onboarded-id
(student)/+layout |  not-onboarded-id
(student)/signup/+page |  not-onboarded-id
Selector.svelte:11 ---------------------------
Selector.svelte:12 SETTING PROFILE: START
Selector.svelte:13 prev: not-onboarded-id
Selector.svelte:14 expected: onboarded-id
Selector.svelte:17 SETTING PROFILE: SET COOKIE
Selector.svelte:18 cookie: profileId=onboarded-id
Selector.svelte:19 SETTING PROFILE: INVALIDATE ALL START
+layout |  onboarded-id
(student)/+layout |  onboarded-id
(student)/signup/+page |  onboarded-id
(student)/signup/+page | redirecting to /
Selector.svelte:21 SETTING PROFILE: INVALIDATE ALL ENDED
Selector.svelte:22 ---------------------------
+layout |  onboarded-id
+page |  onboarded-id
+page | redirecting to /onboarded-home
(student)/(onboarded)/+layout |  not-onboarded-id                   <------ should be `onboarded-id`
+(student)/(onboarded)/+layout | redirecting to /
+page |  not-onboarded-id                               <------ should be `onboarded-id`
+page | redirecting to /signup
+layout |  onboarded-id

Error: Redirect loop...

Logs

No response

System Info

System:
    OS: macOS 12.5
    CPU: (10) arm64 Apple M1 Pro
    Memory: 109.05 MB / 32.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.15.1 - ~/.nvm/versions/node/v16.15.1/bin/node
    npm: 8.11.0 - ~/.nvm/versions/node/v16.15.1/bin/npm
  Browsers:
    Chrome: 105.0.5195.125
    Safari: 15.6
  npmPackages:
    @sveltejs/adapter-auto: next => 1.0.0-next.75
    @sveltejs/kit: next => 1.0.0-next.483
    svelte: ^3.44.0 => 3.50.1
    vite: ^3.1.0 => 3.1.1

Severity

blocking an upgrade

Additional Information

No response

@smblee
Copy link
Author

smblee commented Sep 15, 2022

(going off of the log above) I think whats happening here is...

After the invalidation is completed, we are redirected to / (we have correct id here). Then / tries to redirect you to onboarded-home.

I think the expected behavior here is, in order to load onboarded-home, the app should get the data from the parents of this page (+layout => (student)/+layout => (student)/(onboarded)/+layout), but the log indicates you are going from +page directly to (student)/(onboarded)/+layout (skipping the first two layouts, getting stale data from before).

If the route isn't re-loaded from the root layout, we won't know there was a change when we get the data from await parent()

Current:

...
/+layout |  onboarded-id
/+page |  onboarded-id
/+page | redirecting to /onboarded-home
/(student)/(onboarded)/+layout |  not-onboarded-id                   <------ should be `onboarded-id`
/+(student)/(onboarded)/+layout | redirecting to /
/+page |  not-onboarded-id                               <------ should be `onboarded-id`
/+page | redirecting to /signup
/+layout |  onboarded-id

REDIRECT LOOP error :(

Expected?

...
/+layout |  onboarded-id
/+page |  onboarded-id
/+page | redirecting to /onboarded-home
+/+layout | onboarded-id
+/(student)/+layout | onboarded-id
-/(student)/(onboarded)/+layout |  not-onboarded-id                   <------ should be `onboarded-id`
+/(student)/(onboarded)/+layout |  onboarded-id
-/+(student)/(onboarded)/+layout | redirecting to /
-/+page |  not-onboarded-id                               <------ should be `onboarded-id`
-/+page | redirecting to /signup
-/+layout |  onboarded-id

-REDIRECT LOOP error :(
+You are at onboarded home!

@smblee
Copy link
Author

smblee commented Sep 16, 2022

ok i encountered this issue elsewhere again.

I think the problem is invalidateAll not causing all the layout load functions in the tree to run again, if you were redirected from another layout.
updateSomeData => invalidateAll() => throw redirect(...) due to the updated data => layout data state is out of sync in the redirected page

@j4w8n
Copy link

j4w8n commented Sep 17, 2022

I ran across something similar. It took me a while to figure out what was going on. You may be seeing the same thing.

My issue is that when the invalidateAll runs, and loads start re-firing, throwing a redirect in one of them is like throwing an error; which I suspect interrupts the remaining load functions from re-firing or completing. Therefore no updated info in those areas.

Just the nature of throwing a redirect using sveltekit's method.

@smblee
Copy link
Author

smblee commented Sep 19, 2022

@j4w8n I also think it has to do with "redirects" being a throw messing up some invalidate tree traversal. Would help a lot if we can get some officia thoughts/usage around invalidate/invalidateAll/depends from the maintainers.

dummdidumm added a commit that referenced this issue Sep 20, 2022
- Fixes #6844, invalidation is now only reset after navigation settled
- Fixes #5305, newer invalidations are no longer swalloed by ongoing older ones
- Fixes #6902, the pending prefetch is reset upon invalidation
Rich-Harris added a commit that referenced this issue Sep 21, 2022
* [fix] tighten up navigation and invalidation logic

- Fixes #6844, invalidation is now only reset after navigation settled
- Fixes #5305, newer invalidations are no longer swalloed by ongoing older ones
- Fixes #6902, the pending prefetch is reset upon invalidation

* fix redirect token logic

* batch synchronous invalidations

* tests

* tweak load cache timing and reduce code a little

* Apply suggestions from code review

Co-authored-by: Rich Harris <[email protected]>

* lint

Co-authored-by: Rich Harris <[email protected]>
Co-authored-by: Rich Harris <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants