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

Improve fetching data on a component-level #1689

Open
brillout opened this issue Jun 12, 2024 · 8 comments
Open

Improve fetching data on a component-level #1689

brillout opened this issue Jun 12, 2024 · 8 comments
Labels
enhancement ✨ New feature or request

Comments

@brillout
Copy link
Member

brillout commented Jun 12, 2024

By using Vike extensions, such as vike-react-query and soon vike-react-telefunc, you can fetch data on a component-level (thus on a layout-level as well).

Since TanStack Query is caching independently the URL, it just works. That said, there is potential for improving the caching behavior.

Note

Data fetching Vike Extensions already have a cache (they always need to), so it's just a matter of configuring the cache.

Technical details.

Approach A

vike-{react,vue,solid} provides a new component hook useDataCacheKey() that returns a key composed of:

  • The URL, but without the pathnames of all the nested layouts that the component doesn't belong to.
  • The timestamp when Vike changed the URL.

Vike extensions can then use the key to cache data. In other words, the behavior is exactly as the website visitor expects: data is re-fetched only when navigating the relevant parts of the UI.

Approach B

Alternatively, vike-{react,vue,solid} wraps the nested layouts in a stateful wrapper component (e.g. using React's useState()). Upon navigation, the state of the appropriate wrapper components is updated to trigger a re-render of nested layouts (the page and outer layout aren't re-rendered). But, so far, I think Approach A is superior. (For example, I don't see how Approach B can support the use case when the user clicks on the active link merely to refresh the page.)

@brillout brillout added enhancement ✨ New feature or request high-prio 💫 labels Jun 12, 2024
@brillout brillout changed the title [Nested Layouts] Cache data of outer layout [Nested Layouts] Fetch data on layout-level Jun 27, 2024
@s3ththompson
Copy link

@brillout Can you share what the DX would look like for fetching data on layout-level with useDataCacheKey()?

If data() hooks are still page-level (and not nested), I'm having trouble understanding how I would be able to fetch shared layout data (e.g. menu links from a CMS that apply to all pages) in one place.

@brillout
Copy link
Member Author

Can you share what the DX would look like for fetching data on layout-level with useDataCacheKey()?

It's only meant for generating the cache key for data-fetching tools that use a cache such as TanStack Query.

If data() hooks are still page-level (and not nested), I'm having trouble understanding how I would be able to fetch shared layout data (e.g. menu links from a CMS that apply to all pages) in one place.

Would onBoot() work for you? In the meantime you can use onBeforeRender() with a cache.

@s3ththompson
Copy link

s3ththompson commented Aug 28, 2024

Here is the example I'd like to achieve: https://svelte-layout-data.netlify.app/

The submenu under /settings is populated by menu items loaded from a CMS. (And the entire site is prerendered as a static site.)


onBoot() isn't a good fit, because only some of the pages need the settings submenu. Meanwhile, onBeforeRender() requires that I copy-and-paste the data fetching code across all of the settings pages. Technically, this works, but it requires that I repeat myself for each page (and the data fetching is no longer colocated near the layout markup). Finally, fetching data at the component-level with vike-react-query isn’t an option if I want to prerender the site and turn my CMS query into static JSON at build time.

To me, the opportunity of nested layouts is not just to avoid duplicating markup, but to avoid duplicating data fetching code across multiple page hooks.

I can work around things for my use case, but I feel like Vike's data loading story for nested layouts would be improved with layout hooks.


The way Svelte handles this is very elegant. Layouts can define data fetching hooks, just like pages.

Here is the routing / data-fetching hierarchy for the above example.

src/routes/
├── (app)/
│   ├── about/+page.svelte
│   └── +page.svelte
├── settings/
│   ├── [slug]/+page.svelte
│   ├── +layout.server.ts # nested layout data hook (fetch submenu items from CMS)
│   ├── +layout.svelte # nested layout (settings submenu)
│   └── +page.svelte
└── +layout.svelte # global layout (top-level menu)

Full repo here, in case you'd like to look in more depth.

@s3ththompson
Copy link

s3ththompson commented Aug 28, 2024

By the way, I think the question of where layout data fetching happens is orthogonal to whether nested layouts need to rerender on route change.

I think I'm advocating for an implementation that allows layouts to have their own data() hooks (and by definition, perhaps onBeforeRender() hooks). When a route change occurs, Vike would call each page hook and each parent nested layout hook in parallel. When all the hooks return, it would merge them together to form a single pageContext and render the page.

(I think avoiding re-rendering nested layouts is a nice optimization, but it's separate and could come later. In the meantime, devs could continue relying on query caching and/or component memoization to make route changes cheap.)

@brillout
Copy link
Member Author

Meanwhile, onBeforeRender() requires that I copy-and-paste the data fetching code across all of the settings pages.

You can use config inheritance to avoid that.

(and the data fetching is no longer colocated near the layout markup)

Precisley: if you colocate +onBeforeRender next to +Layout then you don't have to repeat yourself.

Finally, fetching data at the component-level with vike-react-query isn’t an option if I want to prerender the site and turn my CMS query into static JSON at build time.

What makes you think that? AFAICT you should be able to use vike-react-query with pre-rendering. I see, you don't want TanStack Query to try to get fresh data upon client-side navigation.


I think you have a point about nested layouts and data fetching, let me think about it.

@brillout
Copy link
Member Author

brillout commented Aug 29, 2024

After mulling it over a bit, I agree with you.

I went ahead and created a feature request for it: #1833 - Make +data and +onBeforeRender cumulative.

So you'll be able to have this:

# Data shared across all pages
pages/+data.js
pages/+Layout.js

# Data shared across all setting pages
pages/settings/+data.js
pages/settings/+Layout.js

# Data only used by one page
pages/settings/privacy/+data.js
pages/settings/privacy/+Page.js

Would that work for you?

In the meantime, is using +onBeforeRender (while collocating it next to your +Layout) an acceptable workaround for you?

In the meantime, devs could continue relying on query caching and/or component memoization to make route changes cheap.

Hm, I think Vike can (and I think should) do some caching for +data (but not for +onBeforeRender):

  • Upon rendering the first page, there isn't any caching.
  • Upon client-side navigation, fetched data is re-used for +data hooks that aren't collocated next to a +Page.js file. One exception would be if the URL of the next page is exactly the same than the current URL: caching isn't used at all and all +data are re-fetched (enabling users to click on the active link in order to simulate a hard refresh).

In the example above:

  • pages/+data.js is always re-used upon client-side navigation. (It's refetched only when the user does a hard refresh, or when the user clicks on the active link and if the active link points to the exact same URL than the current URL.)
  • pages/settings/+data.js is re-used upon client-side navigating pages/settings/** pages.
  • pages/settings/privacy/+data.js is never re-used: it's always fetched anew.

Edit: +data cache update: #1833 (comment).

@brillout brillout changed the title [Nested Layouts] Fetch data on layout-level Fetch data on component-level Aug 29, 2024
@brillout brillout changed the title Fetch data on component-level Improve fetching data on a component-level Aug 29, 2024
brillout added a commit that referenced this issue Aug 29, 2024
@s3ththompson
Copy link

@brillout excellent news!

You can use config inheritance to avoid that.

I'm glad you brought up using config inheritance to co-locate +onBeforeRender and +Layout ... I think this is a powerful feature. The missing piece is really just making +onBeforeRender / +data cumulative, as you've outlined in the new issue.

In the meantime, is using +onBeforeRender (while collocating it next to your +Layout) an acceptable workaround for you?

Yes, I can work around this in the meantime, but I will be very excited to migrate to cumulative +data when it's ready. :)

(And your +data caching plan makes sense.)

Thanks for being open to consider this!

@brillout
Copy link
Member Author

Thank you for the constructive conversation! We're a little bit busy with a couple of priorties at the moment, but I do consider #1833 high priority so you can expect this coming reasonably soon.

Btw. in case your company is up for it, we're loooking for sponsors (#1350), it makes a massive difference for us.

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

No branches or pull requests

2 participants