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

Allow maxage in layouts load #1515

Closed
gterras opened this issue May 21, 2021 · 4 comments
Closed

Allow maxage in layouts load #1515

gterras opened this issue May 21, 2021 · 4 comments
Labels
feature request New feature or request
Milestone

Comments

@gterras
Copy link

gterras commented May 21, 2021

Is your feature request related to a problem? Please describe.
Layouts cannot cache the return value of their load function, as seen in the docs. If the function fetches an endpoint it will be called each time a link is clicked or prefetched.

Describe the solution you'd like
load() function in a __layout.svelte can return a maxage property to cache its return value, just like indexes can do.

Workaround : return a context in the layout load function then use it along maxage in each children indexes.

With this simple structure, 2 main pages containing 2 tabs each (meaning each sibling tabs share the same data), the parent layout seems like a good place to fetch this data:

├── cat
│   └── __layout.svelte ---> fetching data.json
│   ├── data.json.js
│   ├── general
│   │   └── index.svelte
│   ├── details
│   │   └── index.svelte
├── dog
│   └── __layout.svelte ---> fetching data.json
│   ├── data.json.js
│   ├── general
│   │   └── index.svelte
│   ├── details
│   │   └── index.svelte

I would love to be able to cache both data.json from the layouts. Navigating from cat/xxx to dog/xxx would not trigger a fresh fetch beyond the first visit.

Describe alternatives you've considered

  • understanding the reasons behind [maxage] only applies to page components, not layout components.
  • trying cumbersome combinations of indexes and layouts until it works
  • building a side cache system

How important is this feature to you?
I can live without it especially because the workaround is easy, if not feasible I feel like the docs should explain why in that case.

@benmccann
Copy link
Member

I guess when the page component and layout specify different maxages that you'd want the child to override

@benmccann benmccann added the feature request New feature or request label May 22, 2021
@Rich-Harris
Copy link
Member

maxage (or now cache.maxage) is used to set cache control headers for the page as a whole. In that context, it clearly doesn't make any sense to have it in layout components as well, which is why it 'only applies to page components'.

But yeah, we also use it to cache the result of calling load, but only for the page. The more I think about it though, the more it seems like that's the bit that should change — rather than letting maxage dictate how load results are cached in the client for layouts as well as pages, we should stop caching load results altogether.

Hear me out: we already have a mechanism for caching network requests — cache-control headers. It's very rare that you'd be using cache.maxage to avoid doing 'work' inside load rather than avoiding the fetch (and in those cases a more bespoke i.e. userland caching approach would probably go further), so all we really achieve by storing load results in memory is increasing the heap size and introducing confusion over who is in charge of cache behaviour (the page or the endpoint).

I think getting rid of that behaviour would make things simpler externally, and more consistent and understandable externally. But we would need to implement #4625 first.

@Rich-Harris Rich-Harris added this to the 1.0 milestone May 17, 2022
@madeleineostoja
Copy link

Just wanted to add another voice of support for cleaning this up. I also ran into the cache maxage issue on load in layouts and it took me a while to figure out why my requests were being called fresh every time I changed route or hovered over a link (I missed the single line in the docs about it not working on layouts). Since the cache maxage was returned from load I figured it was actually specific to what load returns, rather than the page, not the other way around.

I think removing that feature entirely and then documenting why it has that behaviour (as per Rich's comment above) and alternatives (eg: cache-control and/or link to userland solution like context) would be a good move. Especially since it'll help bridge the gap between how pages and how layouts work, the finer details of which have caught me out a few times (see also: shadow endpoints for layouts).

Emphasis on the documentation though, I feel like since Sveltekit is moving so fast towards v1 a lot of these features get stripped or changed with only barebones documentation on API basics rather than documentation on how to achieve common use cases like this the "sveltekit way"

@benmccann
Copy link
Member

#5778 added a setHeaders method that I believe should allow you to do this: https://kit.svelte.dev/docs/load#input-methods-setheaders

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
Projects
None yet
Development

No branches or pull requests

4 participants