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

Invalidate url and searchParams separately #8403

Closed
Algoinde opened this issue Jan 9, 2023 · 13 comments
Closed

Invalidate url and searchParams separately #8403

Algoinde opened this issue Jan 9, 2023 · 13 comments
Labels
feature request New feature or request p3-edge-case SvelteKit cannot be used in an uncommon way router
Milestone

Comments

@Algoinde
Copy link
Contributor

Algoinde commented Jan 9, 2023

Describe the problem

TL;DR: Inside layouts, searchParams invalidation is tied to URL invalidation. If searchParams change but the URL does not, invalidation still happens.

Picture this route setup:

/[type]/[slug]/[route]

Each level here uses nested layouts; the data is gotten inside layouts at every step, using the data from the parent. Currently I have the setup where I append a search param at the end of the URL:

/private/test/10/?preview

and want the logic at the /[type] layout to be different because of it, resulting in a different API request in load. But as soon as I introduce an event.url.searchParams dependency inside that layout, it starts invalidating on any URL change, not merely searchParams change.

Describe the proposed solution

It would be nice to split the searchParams invalidation off of URL invalidation, making them independent.

Alternatives considered

Currently I bullied it into ignoring URL validation with this hack.

const absolutelyNotSearchParams =
    Object.getOwnPropertyDescriptor(Object.getPrototypeOf(event.url), 'searchParams').get.call(event.url)

Importance

would make my life easier

Additional Information

Discord thread link on spelunking the code for the invalidation: https://discord.com/channels/457912077277855764/1061705110880780298

@dummdidumm
Copy link
Member

Could you provide a code snippet of what you want to achieve? I'm having a hard time wrapping my head around this.

@Algoinde
Copy link
Contributor Author

Algoinde commented Jan 9, 2023

Sure! Here's a slice of my project (click test, click the buttons), observe the layout 1, layout 2 requests rerunning in the console:

https://stackblitz.com/edit/sveltejs-kit-template-default-xhzbzu?file=src%2Froutes%2Fu%2F[uid]%2F%2Blayout.js

src/routes/u/[uid]/layout.js: I feel it is not supposed to invalidate the entire stack because I am not watching the url, i'm watching query params.

I want to slap that query param at the end and make the route near the root of the stack change the API request.

@dummdidumm dummdidumm added feature request New feature or request and removed awaiting submitter labels Jan 9, 2023
@dummdidumm
Copy link
Member

Ok I think I understand now.

You want foo?bar and baz?bar not to rerun +layout.js if its load function is only using the search params and vice versa

export function load({ url }) {
  url.searchParams...;
  url.search..;
  // ..
}

It's probably possible, but I'm wondering if it's worth the additional code, and if instead something like #6294 in combination with depends is better suited for such situations.

@dummdidumm dummdidumm added the p3-edge-case SvelteKit cannot be used in an uncommon way label Jan 9, 2023
@Algoinde
Copy link
Contributor Author

Algoinde commented Jan 9, 2023

Yeah, correct. Just throwing my edge case out there. From what I have seen, it will require additional query_changed and uses things. Not a priority because there is a workaround, but it will result in less "why is this happening" for users. I could see a couple of permutations where this would be unexpected (it was for me, had to deep into the SK logic for the first time for this).

@Rich-Harris Rich-Harris added this to the later milestone Feb 1, 2023
coryvirok added a commit to coryvirok/kit that referenced this issue Aug 8, 2023
Based on the conversations here:
- sveltejs#10504
- sveltejs#8403

Any changes to the URL will cause any `load` functions that access `url.searchParams` to be rerun even if the search portion of the URL has not changed (or indeed if there are no search params in the URL.)
@coryvirok
Copy link
Contributor

I've been trying to find a way around this bug for a few days without luck. My use case is similar to @Algoinde where I have a layout that loads a document based on a URL search param. That document gets used by numerous child routes but the document doesn't change unless the search param changes.

Here's the server layout:

// routes/doc[...section]/+layout.server.ts

export const load = (async (event) => {
  // Optional revision passed in via the url = ?revision=abcd123
  // We want to only rerun this load() when the revision search param changes.
  const revision = event.url.searchParams.get('revision') ?? 'latest'
  
  const doc = await loadDoc(revision)
  return {
    doc
  }
}) satisfies LayoutServerLoad

In my +page.svelte there are various internal links to other sections, e.g. /doc/section1/subsection1. Due to this bug, every navigation change causes the layout to reload, despite the url param not ever changing, (or even being provided.)

I understand why this is happening and I understand how to access the searchParams in a way that will not track the URL and cause the reload. However, I do not understand how to reload the layout only when the searchParams changes.

E.g. in my +page.svelte I have a link to a previous revision, /doc/section1?revision=previous

  • If I access the searchParams in an untracked way, clicking on this link will not cause the load to happen which means the wrong version of the doc will be shown.
  • If I access searchParams in the usual way, (url.searchParams.get(...)) every internal link to the same document will cause a reload of the doc, while links to previous revisions will work.

I've tried using depends + the untracked access method described in this issue but I don't understand how to check the previous value of the URL in order to call invalidate on the URL when I know there should be a reload of the layout.

Is there actually a workaround for this? How do we only reload a layout when a URL search param changes?

One suggestion would be to provide a second parameter to depends() that takes in the string/url to check and returns true if the url should be invalidated and false otherwise.

e.g.

const { url, depends, request } = event
const currentRevision = new URL(request.url).searchParams.get('revision') ?? 'latest'

// Depend *only* on the `revision` search param
depends(request.url, (url: string) => {
  // Return true if the provided URL should be invalidated
  const newRevision = new URL(url).searchParams.get('revision') ?? 'latest'
  return newRevision !== currentRevision
})

@mikolajkazmierczak
Copy link

mikolajkazmierczak commented Sep 19, 2023

I have another use case where this would be very useful.

In my project I have a /products layout/page that displays a list of products, and /products/[id] page that displays a given product. The /products/[id] page still shows products from /products undearneath.

// /products +layout.server.js
export async function load({ locals: { db }, url }) {
  const search = new URL(url).searchParams.get('search');
  const { items } = await db.read('products', { search });
  return { items };
}

The problem is I want to be able to query the products list with a URL param.

To do this I need to:

  1. show the list using a param (e.g. /products?search=tea)
  2. when a user clicks on a given product from the list show a product page (e.g. /products/bubbletea)
  3. when a user wants to go back to the products list I add the param (/products?search=tea) so that the user doen't have to manually use the search again and, obviously (*see note below*), to prevent the list from refreshing, hovewer...

That causes the list to be refreshed not just everytime the user navigates back to the products list, but EVERYTIME the users clicks on any product. That's because of the fact that the /products layout relies on the URL to filter the list and the parameter disappears from the url when the user is navigating to a chosen product. Hovewer when the user goes back to a /products?search=tea the param reappears exactly how it was before on that pathname so it shouldn't trigger the reload in my case. I can hovewer see a use case where it should, so...
My point is we should be able to control when the reload is happening.

Personally I think that we should be able to control all (or most) conditions of a reload. My proposition:

export async function load({ reload }) {
  reload({ parentLoad: false, searchParams: false }) // etc
  return;
}

*note*: I'd just like to add here that I think that kind of behaviour seems intuitive to me. That even though the pathname depends on a URL, when absolutely nothing changes in it since last time the given route was visited there is no need for a refresh. I do understand why what is happening is happening, but - in my head - intuitively it shouldn't be happening.

@coryvirok
Copy link
Contributor

Bump.

Any updates on this? Without this fix, certain types of access patterns are not possible with Kit.

E.g. It's not possible to load layout data using a search param such that subsequent navigations to child routes does not invalidate the layout data.

@Bewinxed
Copy link

Bewinxed commented Oct 28, 2023

Bump.

Any updates on this? Without this fix, certain types of access patterns are not possible with Kit.

E.g. It's not possible to load layout data using a search param such that subsequent navigations to child routes does not invalidate the layout data.

I ran into this and you can do any of the following:

  1. Add an onNavigate/onUpdate in the +layout to invalidateAll
  2. surround the slot with a #key $page.url.pathName Block.
  3. invalidate all in a $: block in the page.

@coryvirok
Copy link
Contributor

coryvirok commented Oct 28, 2023

Bump.
Any updates on this? Without this fix, certain types of access patterns are not possible with Kit.
E.g. It's not possible to load layout data using a search param such that subsequent navigations to child routes does not invalidate the layout data.

I ran into this and you can do any of the following:

  1. Add an onNavigate/onUpdate in the +layout to invalidateAll
  2. surround the slot with a #key $page.url.pathName Block.
  3. invalidate all in a $: block in the page.

The issue is that I don't want to invalidate all since that would invalidate both the layout and the page. I want to invalidate just the page but leave the layout alone. Imagine an app that uses 2 url search parameters. The first is used to load the layout data and the second is used by the page to load data. Theres no way (that I know of due to this bug) to change just the pages url search param and have only the page load rerun.

@Bewinxed
Copy link

Bewinxed commented Oct 28, 2023 via email

@coryvirok
Copy link
Contributor

I tried to do what you're saying by making a nested page with a route param according to the url param you want to switch. Then surrounding the <slot in the layout with a key block like in my 2nd method

On Sat, 28 Oct 2023 at 8:09 PM Cory Virok @.> wrote: Bump. Any updates on this? Without this fix, certain types of access patterns are not possible with Kit. E.g. It's not possible to load layout data using a search param such that subsequent navigations to child routes does not invalidate the layout data. I ran into this and you can do any of the following: 1. Add an onNavigate/onUpdate in the +layout to invalidateAll 2. surround the slot with a #key $page.url.pathName Block. 3. invalidate all in a $: block in the page. The issue is that I don't want to invalidate all since that would invalidate both the layout and the page. I want to invalidate just the page but leave the layout alone. Imagine an app that uses 2 url parameters. The first is used to load the layout data and the second is used by the page to load data. Theres no way (that I know of due to this bug) to change just the pages url search param and have only the page load rerun. — Reply to this email directly, view it on GitHub <#8403 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACFY5BLW4PVBVANTTGQJHT3YBU35NAVCNFSM6AAAAAATVCU4B2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOBTHA3TGMBZGQ . You are receiving this because you commented.Message ID: @.>

Ah yes, I would imagine that works fine if you can move the search params into route parameters.

@coryvirok
Copy link
Contributor

Bump.

Any updates on this?

@Algoinde
Copy link
Contributor Author

Algoinde commented Jul 4, 2024

Can this be considered fixed by #11258?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request p3-edge-case SvelteKit cannot be used in an uncommon way router
Projects
None yet
Development

No branches or pull requests

6 participants