kit icon indicating copy to clipboard operation
kit copied to clipboard

SvelteKit 2: load url.searchParams bug

Open martonx opened this issue 1 year ago • 9 comments

Describe the bug

When we changed searchParams, and force to reload sveltekit data, url.searchParams remain the same value as before the change.

Reproduction

Reproduction:

https://stackblitz.com/edit/sveltejs-kit-template-default-xeika8?file=src%2Froutes%2F%2Bpage.svelte

click on the change url button. This will do a window.history.pushState with new searchParams then invalidate data, so it'll trigger the page.js load function. In load function check url.searchParams (consol.log writes it).

The reproduction is a bit bad as this sandbox environment doesn't visibly change the url, but the reproduction is working locally or from any kind of hosting method.

This worked with SvelteKit 1.30.x. It is broken just after the SvelteKit 2.0 update. You can reproduce it with old and current SvelteKit.

Logs

No response

System Info

System:
    OS: Windows 11 10.0.22631
    CPU: (16) x64 AMD Ryzen 7 5800X 8-Core Processor
    Memory: 6.48 GB / 15.93 GB
  Binaries:
    Node: 18.16.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.19 - C:\Program Files (x86)\Yarn\bin\yarn.CMD
    npm: 9.5.1 - C:\Program Files\nodejs\npm.CMD
    pnpm: 8.7.5 - ~\AppData\Local\pnpm\pnpm.EXE
  Browsers:
    Edge: Chromium (120.0.2210.91)
    Internet Explorer: 11.0.22621.1

Severity

critical

Additional Information

No response

martonx avatar Jan 02 '24 23:01 martonx

I'm sorry severity is not annoyance, but this is a critical issue, as this one bug blocks us to upgrade to sveltekit 2.0

martonx avatar Jan 03 '24 08:01 martonx

The console shows the issue and solution:

Avoid using history.pushState(...) and history.replaceState(...) as these will conflict with SvelteKit's router. Use the pushState and replaceState imports from $app/navigation instead.

With that change, it works as expected.

CaptainCodeman avatar Jan 03 '24 22:01 CaptainCodeman

This was changed in #11307 - @Rich-Harris could you elaborate on why the change to use current.url instead of location.href in invalidate was necessary? Also, this uncovers a hole in the API: pushState is tightly coupled to shallow routing - you can't push state in the regular way that also updates $page.url. This together with the change to invalidate means you're now unable to get this working (@CaptainCodeman are you sure this worked fo you? I can't get it to work with changing this to use pushState from $app/navigation).

That said, @martonx why aren't you using goto in this case? Does this way allow you more fine-grained control over which things to reload / your problem is that goto would reload too much?

dummdidumm avatar Jan 04 '24 08:01 dummdidumm

I thought it did last night, but stackblitz isn't working for me this morning to confirm, I may have been mistaken.

CaptainCodeman avatar Jan 04 '24 15:01 CaptainCodeman

@dummdidumm I don't use goto because of I don't want to navigate in real. I just want to change the url, and fetch some fresh data. Actually this is a deep linked filtering in an interactive page. When you are changing the filters, I'm doing the filtering in the load method (this is why I use invalidate), and I'm keeping in sync the url query string params. This use case, this code works perfect with SvelteKit 1.30.x

I tried goto now. The issue with goto it overwrites the current page state for example scroll position. So if I switch these lines:

window.history.pushState(null, '', ${path}?${params.toString()}); invalidate('app:blogs');

to:

goto(${path}?${params.toString()});

The filtering is working, but the page is scrolling to the top, I don't know why. This is not visible in the repro code, but very frustrating in my real code, where the filters, and the filter results are not on the top of the page.

martonx avatar Jan 05 '24 20:01 martonx

@CaptainCodeman

I changed this line:

window.history.pushState(null, '', ${path}?${params.toString()});

to

pushState(${path}?${params.toString()}, $page.state);

nothing changed, nothing fixed. Am I right with this pushState usage? The documentation has no example of how should we use SvelteKit's app/navigation's own pushState method.

martonx avatar Jan 05 '24 20:01 martonx

It looks similar to the issue I opened 2 weeks ago (https://github.com/sveltejs/kit/issues/11465).

It seems that pushState and replaceState only work for shallow routing (url stays the same), but it's never made clear in the documentation (and the deprecation warning for history indicates otherwise).

AlexisGado avatar Jan 08 '24 16:01 AlexisGado

@dummdidumm I don't use goto because of I don't want to navigate in real. I just want to change the url, and fetch some fresh data. Actually this is a deep linked filtering in an interactive page. When you are changing the filters, I'm doing the filtering in the load method (this is why I use invalidate), and I'm keeping in sync the url query string params. This use case, this code works perfect with SvelteKit 1.30.x

I tried goto now. The issue with goto it overwrites the current page state for example scroll position. So if I switch these lines:

window.history.pushState(null, '', ${path}?${params.toString()}); invalidate('app:blogs');

to:

goto(${path}?${params.toString()});

The filtering is working, but the page is scrolling to the top, I don't know why. This is not visible in the repro code, but very frustrating in my real code, where the filters, and the filter results are not on the top of the page.

I think what you want is goto(${path}?${params.toString()}, { replaceState: true, noScroll: true });. replaceState replaces the url without navigating and noScroll prevents scrolling to the top of the page. https://kit.svelte.dev/docs/modules#$app-navigation-goto

Or better yet, use a GET action form that does this for you with progressive enhancement https://kit.svelte.dev/docs/form-actions#get-vs-post

The real issue is that after using pushState, I suspect the sveltekit tracked URL is not updated correctly so the invalidation called in the reproduction does not console log the updated URL search query params.

eltigerchino avatar Mar 18 '24 15:03 eltigerchino

I also ran into this problem, and its very annoying. For now I can work around it with this:

window.history.replaceState(history.state, '', url);

f-elix avatar Jun 28 '24 22:06 f-elix