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

SvelteKit 2: load url.searchParams bug #11503

Open
martonx opened this issue Jan 2, 2024 · 10 comments
Open

SvelteKit 2: load url.searchParams bug #11503

martonx opened this issue Jan 2, 2024 · 10 comments
Labels
bug Something isn't working router

Comments

@martonx
Copy link

martonx commented Jan 2, 2024

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
Copy link
Author

martonx commented Jan 3, 2024

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

@CaptainCodeman
Copy link
Contributor

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.

@dummdidumm
Copy link
Member

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 dummdidumm added the bug Something isn't working label Jan 4, 2024
@CaptainCodeman
Copy link
Contributor

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

@martonx
Copy link
Author

martonx commented Jan 5, 2024

@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
Copy link
Author

martonx commented Jan 5, 2024

@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.

@AlexisGado
Copy link

AlexisGado commented Jan 8, 2024

It looks similar to the issue I opened 2 weeks ago (#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).

@eltigerchino
Copy link
Member

@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.

@f-elix
Copy link
Contributor

f-elix commented Jun 28, 2024

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);

@eltigerchino
Copy link
Member

eltigerchino commented Oct 8, 2024

I'm not sure how to fix this without updating the internal current.url so that it invalidates using the URL that was passed to pushState. However, updating current.url also updates the page store URL, which is what we don't want when using pushState.

Some notes on shallow routing with pushState / replaceState:

  • It is not an actual navigation because the underlying route does not change.
  • The browser URL changes and a history entry is created.
  • $page.url and $page.route will not be updated.
  • Navigation hooks will not run e.g., beforeNavigate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working router
Projects
None yet
Development

No branches or pull requests

6 participants