Skip to content
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

Accessing the search object in the load function prevent goto navigation #8695

Closed
paoloricciuti opened this issue Jan 24, 2023 · 6 comments
Closed

Comments

@paoloricciuti
Copy link
Member

Describe the bug

I'm in a bit of a problem which is super niche but it might be useful for someone else too.

Basically in sveltekit-search-params (https://github.com/paoloricciuti/sveltekit-search-params) i create a store to access the query parameters and you can read and write to them seamlessly.

Before 1.0 every time you changed a query parameter it would've pushed a new history entry in the state so i've added the ability to debounce the history entry. However it's very important that the state of the url always match the state of the store. So basically the URL it's always immediately updated with replaceState set to true (so that it doesn't add an history entry) and after the debounce period the URL is updated again without changing anything with replaceState set to false (this basically just add the history entry).

Now the problem is that this second goto could be running together with a load function and if that load function is accessing the url param the second goto doesn't trigger a page navigation (so the url it's not updated).

Reproduction

  1. Navigate to https://stackblitz.com/edit/sveltejs-kit-template-default-1qknah?file=src/routes/+page.svelte
  2. Open the preview in a new window
  3. Observe that the index page does not have a load function
  4. Try to click on Change param
  5. Observe that everything works fine (despite the double timeout navigation sorcery...more on that explained in the not-working/+page.svelte
  6. Click on the link to go to /not-working
  7. Observe that the load function access url.search
  8. Try to click on Change param
  9. Observe that the console on the server logs the correct new url but the navigation does not actually occur

Logs

No response

System Info

Stackblitz

Severity

serious, but I can work around it

Additional Information

I understand this is a super niche case and i could change my library to not handle the debounce or trying to work around this bug. At the same time it seems to be a bug in sveltekit so it might be worth trying to fix it.

@eltigerchino
Copy link
Member

eltigerchino commented Jan 24, 2023

Further investigation shows that it occurs when referencing any property of event.url.

Also, using a shared load function such as +page.js has the same effect as +page.server.js.

@Rich-Harris
Copy link
Member

In your repro you're calling goto('', {...}), which means 'go to the current URL'. Which means url hasn't changed, which means a load function that only depends on url won't re-run. I don't think there's a bug here, unless I'm missing something?

@paoloricciuti
Copy link
Member Author

In your repro you're calling goto('', {...}), which means 'go to the current URL'. Which means url hasn't changed, which means a load function that only depends on url won't re-run. I don't think there's a bug here, unless I'm missing something?

The problem is not in the load function: basically in the not working page the first goto (the one that changes the parameters) doesn't work. Try open the page from stackblitz in a new tab without using the integrated preview.

@paoloricciuti
Copy link
Member Author

To better clarify this: the change params button does exactly the same thing in both pages but while in the working page the url gets redirected to the correct page (the one where the first goto point) in the not working page the url doesn't change at all.

@Rich-Harris
Copy link
Member

Ah ok. goto(url) resolves url against the current window.location. In the /working case, goto resolves immediately because the code for the page is already loaded (it's the same page) and loading is immediate (there's no load function), so by the time the setTimeout rolls around, we've already updated the page URL.

In the /not-working case, SvelteKit needs to make a request to the server, because there's a +page.server.js file with a load function. (In dev it will always go to the server if there's a +page.server.js, in prod it's smart enough to ignore +page.server.js files without a load function.) As a result, the page URL is unchanged by the time of the second goto, so the second one cancels the first with a navigation to ''.

The fix is simple — you just need to await the first goto:

-    setTimeout(()=>{
+    setTimeout(async ()=>{
        // in the first timeout i get the query and update the value
        const oldQuery=new URLSearchParams($page?.searchParams);
        oldQuery.set("test", Math.random());
        // than i navigate to the new query string with replaceState to true 
-        goto(`?${oldQuery.toString()}`, {
+        await goto(`?${oldQuery.toString()}`, {
            replaceState: true,
            keepFocus: true,
            noScroll: true,
        });
        /* this second timeout is to debounce another navigation which the sole purpose
         * is to add an history entry so it points to the current page. I can do this
         * either with goto("") or by getting the value from the page store both ways
         * i have the same problem
         */
        setTimeout(()=>{
            goto("", {
                replaceState: false,
                keepFocus: true,
                noScroll: true,
            });
        }, 0)
    }, 0);

This is all very hacky and ill-advised though. You might be interested in #2673

@Rich-Harris Rich-Harris closed this as not planned Won't fix, can't repro, duplicate, stale Jan 28, 2023
@paoloricciuti
Copy link
Member Author

Ah ok. goto(url) resolves url against the current window.location. In the /working case, goto resolves immediately because the code for the page is already loaded (it's the same page) and loading is immediate (there's no load function), so by the time the setTimeout rolls around, we've already updated the page URL.

In the /not-working case, SvelteKit needs to make a request to the server, because there's a +page.server.js file with a load function. (In dev it will always go to the server if there's a +page.server.js, in prod it's smart enough to ignore +page.server.js files without a load function.) As a result, the page URL is unchanged by the time of the second goto, so the second one cancels the first with a navigation to ''.

The fix is simple — you just need to await the first goto:

-    setTimeout(()=>{
+    setTimeout(async ()=>{
        // in the first timeout i get the query and update the value
        const oldQuery=new URLSearchParams($page?.searchParams);
        oldQuery.set("test", Math.random());
        // than i navigate to the new query string with replaceState to true 
-        goto(`?${oldQuery.toString()}`, {
+        await goto(`?${oldQuery.toString()}`, {
            replaceState: true,
            keepFocus: true,
            noScroll: true,
        });
        /* this second timeout is to debounce another navigation which the sole purpose
         * is to add an history entry so it points to the current page. I can do this
         * either with goto("") or by getting the value from the page store both ways
         * i have the same problem
         */
        setTimeout(()=>{
            goto("", {
                replaceState: false,
                keepFocus: true,
                noScroll: true,
            });
        }, 0)
    }, 0);

This is all very hacky and ill-advised though. You might be interested in #2673

Yeah that was I was thinking, silly me not thinking of awaiting goto to wait for the end of the navigation 😅

I ended up doing a far weirder thing with the navigating store but I'll refactor with await as soon as possible.

And I know it's a bit hacky but I guess it's the only way for my use case. I took a look at the issue but the fact that the load function re-run it's fine once I can make sure that the url actually update (I'll keep an eye on it because it could be a cool customization for the store). Thanks for the feedback and keep up the good work, sorry for wasting your time!

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

No branches or pull requests

3 participants