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('...') doesn't work after navigating to a page with parameters #6354

Closed
aradalvand opened this issue Aug 28, 2022 · 7 comments · Fixed by #6493
Closed

invalidate('...') doesn't work after navigating to a page with parameters #6354

aradalvand opened this issue Aug 28, 2022 · 7 comments · Fixed by #6493
Labels
breaking change documentation Improvements or additions to documentation feature / enhancement New feature or request
Milestone

Comments

@aradalvand
Copy link
Contributor

aradalvand commented Aug 28, 2022

Describe the bug

Repro: https://github.com/aradalvand/sveltekit-invalidate-with-params

It seems like having a route with params (e.g. routes/whatever/[id]) along with a load function that have depends(...) doesn't work as expected, when you navigate to that page with the client-side router, and then run invalidate('...'), it won't work. Very weird. I'm pretty confident this is a bug.

Reproduction

  • Clone https://github.com/aradalvand/sveltekit-invalidate-with-params
  • Run and open the app (the home page) in the browser
  • Open the console, run invalidate('user'). You'll see that the layout's load function gets re-run, as expected:
    image
  • Now, click on the link in the home page titled "Go to foo"
  • After navigating to the /foo/123 page, open the console and run the same command again. This time, it won't work, the layout's load function isn't re-run:
    image

Logs

No response

System Info

System:
    OS: Linux 5.15 Ubuntu 20.04.4 LTS (Focal Fossa)
    CPU: (4) x64 Intel(R) Core(TM) i7-7500U CPU @ 2.70GHz
    Memory: 486.11 MB / 3.77 GB
    Container: Yes
    Shell: 5.8 - /usr/bin/zsh
  Binaries:
    Node: 16.15.1 - /usr/bin/node
    npm: 8.11.0 - /usr/bin/npm
  npmPackages:
    @sveltejs/adapter-auto: next => 1.0.0-next.65
    @sveltejs/kit: next => 1.0.0-next.445
    svelte: ^3.44.0 => 3.49.0
    vite: ^3.0.4 => 3.0.9

Severity

blocking all usage of SvelteKit

Additional Information

The problem only exists for pages that have parameters, I have no idea why, but I think this information could be useful for diagnosing the problem.

@aradalvand aradalvand changed the title invalidate('...') doesn't work when navigating to a page with params invalidate('...') doesn't work after navigating to a page with parameters Aug 28, 2022
@dummdidumm
Copy link
Member

This works as expected. The invalidate/depends function expects an URL / constructs one, so in this case depends('user') becomes http://127.0.0.1:5173/user, but invalidate('user') inside foo becomes http://127.0.0.1:5173/foo/user - this doesn't match, so nothing reruns.
I've been bitten by this, too, since I wasn't aware of it, and while the docs hint at it, they don't make it very explicit. So we should at least enhance the docs, or allow arbitrary values for depends and invalidate - but not sure how to do this in a way that also preserves existing behavior. @Rich-Harris FYI marking this as potentially breaking change and to the 1.0 milestone in the sense of "we need to decide on what to do here".

@dummdidumm dummdidumm added documentation Improvements or additions to documentation breaking change feature / enhancement New feature or request labels Aug 29, 2022
@dummdidumm dummdidumm added this to the 1.0 milestone Aug 29, 2022
@aradalvand
Copy link
Contributor Author

aradalvand commented Aug 29, 2022

So we should at least enhance the docs, or allow arbitrary values for depends and invalidate

I'm surprised to hear that that's already not possible, I thought you could pass in any unique string to depends.

I think you should allow arbitrary values, as you said, it enables a variety of useful scenarios, such as the one in the repro I provided above, namely having depends('user') in +layout.ts and then running invalidate('user') whenever someone logs in or whatever to update the user information. And sometimes you have something like a GraphQL API where there's just a single endpoint (/graphql) and so you can't differentiate between the fetches via the URL so you have to resort to a custom string for depends.

@icalvin102
Copy link
Contributor

icalvin102 commented Aug 29, 2022

All the urls you pass to depends or fetch will be registered as full urls.

Meaning they are passed through https://developer.mozilla.org/en-US/docs/Web/API/URL/URL before being saved.

const { href } = new URL(url, location.href)

Arbitrary values are possible if you make new URL() think that you passed a full url.
The url has to contain a protocol string to be recognized as full url. In other words it has to match this regex /^[a-z]+:.*/

So as long as you prefix your identifier with one or more lowercase letters followed by a colon you should be fine.

Example:

depends(
  'custom:foo',
  'page:HOME_PAGE'
);

invalidate('custom:foo');
invalidate((u) => u.startsWith('page'));

@icalvin102
Copy link
Contributor

icalvin102 commented Aug 30, 2022

In my first proposed solution for custom dependencies in the load function (#1709) there was a lot of discussion with @Rich-Harris on how this could be implemented cleanly.

I wanted to have completely arbitrary identifiers for the invalidation. However the requirements of invalidating completely custom identifiers and urls don't mix well. For urls passed to fetch or depends one would like to resolve the full url from relative paths but custom identifiers should stay unaltered. The implementation and usage of this would be quite ugly because there we would need a way to differentiate between the "url" variant and the "custom" variant either by providing a flag or having separate functions for each variant.

Example:

function depends(identifier: string, isCustom?: boolean){}
function invalidate(identifier: string, isCustom?: boolean){}

This is of cause not very nice.

So I was quite happy when I came up with the solution described in my previous comment because it unifies the requirements of urls and custom identifiers. Of cause this does not give you complete freedom over what identifiers you can choose because of the (protocol like) prefix but it's close enough in my daily work. And being forced to use a prefix for custom identifiers makes it quite trivial to define namespaces/categories for them that are easily invalidated with a predicate.

Long story short: There is no need for another breaking change in this case IMO. The current implementation can deal with urls and custom identifiers quite fine. The only thing that is missing is good documentation on how to use it.

dummdidumm pushed a commit that referenced this issue Sep 1, 2022
Closes #6354.

Related to #6489, #6274 and #5305 

Co-authored-by: Rich Harris <[email protected]>
Co-authored-by: icalvin102 <[email protected]>
Co-authored-by: Immanuel Calvin Herchenbach <[email protected]>
@cassepipe
Copy link

cassepipe commented Aug 18, 2023

EDIT: Dont' do this, custom identifiers must start with lowercase letter followed by a : !

To clarify, if I have the following structure :
/chans/[chan_id]
- with a +layout.ts at /chans which calls depends(':custom')
- wiht a page.svelte at /chans/[chan_id] that calls invalidate(':custom')

Will the load function re run ?

@cornzz
Copy link

cornzz commented Sep 27, 2023

Wow, please make it more clear in the docs that the invalidation identifier is used to construct a URL object and we should include a colon in custom identifiers, I just spent way too much time trying to figure out why my load function wasn't rerunning.
It's too easy to ignore the current hint since invalidate will work with any string as long as you're in the same path...

@cassepipe
Copy link

cassepipe commented Sep 27, 2023

@cornzz
True, it's so easy to miss in a wall of text that there's there are two semi-colons in the sentence and that one is actually part of the the given regex !

Note that url can be a custom identifier that starts with [a-z]::

Also right I also found out that the litmus test to know if what you are passing to depends/invalidate is to try to pass the string to new URL()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change documentation Improvements or additions to documentation feature / enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants