Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

URL query change doesn't trigger preload method #701

Closed
PaulMaly opened this issue May 18, 2019 · 10 comments · Fixed by #704
Closed

URL query change doesn't trigger preload method #701

PaulMaly opened this issue May 18, 2019 · 10 comments · Fixed by #704

Comments

@PaulMaly
Copy link

Hi! If we have a link like this:

<a href="/?page=1">1</a>

and click it, this won't trigger preload to fetch the data again:

export async function preload({ params, query }) {
  const res = await this.fetch(`/items?page=${query.page}`),
        data = await res.json();
...
}

I believe it shoud work, but it didn't.

@Conduitry
Copy link
Member

I've come around to considering this an outright bug. In #484 (0.25.0), the behavior was changed so that these query-only navigations caused preload to be re-run, and this behavior changed in 0.26.0 for Svelte 3.

@Conduitry Conduitry added the bug label May 20, 2019
@arxpoetica
Copy link
Member

arxpoetica commented May 20, 2019

Hoo boy, this is a tricky one. I can see myself sometimes wanting a refresh on query change and maybe other times not wanting that behavior. I'm wondering if there ought to be an idiomatic way of expressing what the author wants...hmm...

Update: what does React/Vue/Angular do? Could we look to their example(s)?

@Conduitry
Copy link
Member

As I noted in chat:

The behavior silently changed between 0.25 and 0.26, which is what's making me think it's better to consider this a bug
And if we want to add something to let us control this at runtime, maybe that should be its own enhancement
Although granted, the behavior did change (non-silently) between 0.24 and 0.25, so there wasn't a lot of precedent for the always-call-preload behavior

I think possibly the way to address this is with some sort of additional logic here that also marks segment_dirty = true if we are the last segment and the query changed.

@PaulMaly
Copy link
Author

@Conduitry So, this works fine in 0.25.0? Can I use it with Svelte 3? I mean, maybe I just need to downgrade sapper until bug won't be fixed.

@Conduitry
Copy link
Member

No, 0.25.0 is only for Svelte 2. This behavior changed again in 0.26.0 as part of the rewrite for Svelte 3.

@PaulMaly
Copy link
Author

@Conduitry Oh, sad. ((

@Conduitry
Copy link
Member

The change in #484 doesn't actually do any checking for changed query string, it just makes it so that the deepest layer always is marked as changed. A somewhat analogous change here is:

diff --git a/runtime/src/app/app.ts b/runtime/src/app/app.ts
index 3f5e8e1..cfd9402 100644
--- a/runtime/src/app/app.ts
+++ b/runtime/src/app/app.ts
@@ -296,7 +296,7 @@ export async function hydrate_target(target: Target): Promise<{
 		branch = await Promise.all(route.parts.map(async (part, i) => {
 			const segment = segments[i];
 
-			if (current_branch[i] && current_branch[i].segment !== segment) segment_dirty = true;
+			if (current_branch[i] && (current_branch[i].segment !== segment || i === route.parts.length - 1)) segment_dirty = true;
 
 			props.segments[l] = segments[i + 1]; // TODO make this less confusing
 			if (!part) return { segment };

but I'm not comfortable just suggesting that yet. I'm back to not knowing what the right behavior is here.

@Conduitry
Copy link
Member

More musings from chat: What if preload continues to only be called when params change, but we provide some way for routes to declare some static list of query params that get stuck into route params? The Sveltest way to do this would probably be right in the filename somehow, but I can't think of anything right now that would be neither very confusing nor an invalid filename.

@PaulMaly
Copy link
Author

@Rich-Harris Thank you very much!

@IamSAB
Copy link

IamSAB commented Jan 30, 2021

I am having this issue right now. I use preload to load paginated data - and I still got the exact same problem as the posed question describes.
I am using Svelte 3 and Sapper. When that does not work, how am I supposed to do pagination? Or is it recommended to do pagination only client side? Or should I use the filesystem routing by creating a [page].svelte? But that seems kinda strange for me ... what should I then do, when I want to combine e.g. a filter or a search with the pagination?
Thanks in advance for pointing me into the right direction.

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

Successfully merging a pull request may close this issue.

4 participants