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

Page load functions should return props via derived stores #5850

Closed
joeally opened this issue Aug 8, 2022 · 4 comments
Closed

Page load functions should return props via derived stores #5850

joeally opened this issue Aug 8, 2022 · 4 comments
Milestone

Comments

@joeally
Copy link

joeally commented Aug 8, 2022

Describe the problem

Having a single load function which loads all of the data for a page means that you cannot reload part of the pages contents. If the user performs an action that invalidates part but not all of the page one has to reload the route which will fetch all of the data for the page. This means that the app will make more database and storage calls than necessary. If the props returned by the load function are not primitive values svelte will also re-render pretty much the entire page even if only a single only component needs updating. Because even if the returned props have equal values svelte is only able to check equality for primitive values.

This means that to get partial updates in pages we have to duplicate the load logic in event handlers in the page. The partial updates however will not be available to users with no Javascript so the developer is forced to handle that case separately. It would be much better if load only made the fetches that it needed to based on its inputs.

Describe the proposed solution

Instead of the load function returning a props object with a value specified for each component prop the load function should instead return a props object with a store specified for each component prop. This would also require that objects passed into the load function are stores.

Consider the following example load function in my proposed 'store style' for a hypothetical (and somewhat contrived) social network example where the user navigates between posts but and the user's profile information is displayed in the top corner of each page.

export const load = ({ request, stuff, url, session, status, error, params, fetch }) => {
  const post = derived([params], async ([$params], set) => {
    const postResponse = await fetch(`posts/${params.postId}.json`);
    const postData = await postResponse.json();
    set(postData);
  });
  const userProfile = derived([session], async ([$session], set) => {
    const userProfileResponse = await fetch(`userProfile.json?sessionId${$session.sessionId}`);
    const userProfileData = await userProfileResponse.json();
    set(postData);
  });
  return {
    props: { post, userProfile }
  }
}

Every time the user navigates to a different post the params store is updated causing the new post data to be fetched using the new postId. Any components that depend on the post prop will then be updated. However because the session has not changed the user profile does not need to be refetched. And since the userProfile store does not get updated any components that only depend on the userProfile prop don't need to be updated.

Alternatives considered

No response

Importance

would make my life easier

Additional Information

No response

Edit: Fixed some errors in my writing. I was a bit tired when I wrote this.

@benmccann benmccann added this to the 1.0 milestone Aug 8, 2022
@joeally joeally changed the title Page load functions should be return props via derived stores Page load functions should return props via derived stores Aug 9, 2022
@rmunn
Copy link
Contributor

rmunn commented Aug 9, 2022

Note that once #5778 (which implements #5748) is merged, the return values of load functions are going to change dramatically. Instead of returning a props object, load functions will be simply returning the data values that the page will need. There's also a plan to remove the stuff argument entirely so that load functions can be parallelized: the load for the layout will run at the same time as the load for the page, (You can opt back into getting values passed down from parent components by using await parent() instead of the stuff parameter, so if you need that it's still there).

Most importantly, there's also a feature where if the data used in the layout's load function changes, it won't re-run the load function for the page. Which means that you should be able to get what you want by careful splitting of your design between different layouts, which could then "depend" on each other via the named-layout feature. I.e., +layout-userprofile.svelte could return just the props needed for the user profile, and +layout-post@userprofile.svelte could load the data for the post the user navigates to. Because it "inherits" from the userprofile layout, its HTML will be placed inside the HTML for the user profile (so the user profile div will be the outer div, and the post will be the inner div, which is how I expect this would be laid out). But because load() functions will run indepedently from each other (and in parallel), you won't need to reload the user's profile when the navigate to a different post.

Note that this is all theoretical at the moment, and the design might go in a different direction by the time it's done. You can watch #5778 to see how the implementation of #5748 is progressing. But once that change goes in, I think you'll be able to achieve the result you want just by having different parts of your site load only the data that they themselves need.

@joeally
Copy link
Author

joeally commented Aug 9, 2022

For my hypothetical example indeed splitting things into nested layouts would work. However note that there will be many cases where the page itself has multiple different fetches some of which may need be updated and others not.

Consider the example of a dashboard where the user has multiple configurable widgets. The user may want to change one of the widgets without changing the others. All of those widgets operate a the same level in the data model so implementing them via nested layouts would be a strange and confusing way to organise the code.

Relying on nesting is limiting and will not fit some large number of use cases.

@dummdidumm
Copy link
Member

dummdidumm commented Aug 18, 2022

This API would force everyone to do the complex case for simple cases. I hear you though, it would be nice if there was some way to have this more fine-grained. It would probably only apply to the browser though, since with serverless environments you start fresh often, and with a node server you could overflow your cache fast.

When we say "this is a browser only feature", then it could be possible to solve the "avoid refetch" use case in userland:

import { browser } from '$app/env';
import { equals } from './utils';

let lastParams;
let cached;
async function refetchIfParamsChanged(fetch, newParams) {
  if (!browser) {
    return fetch();
  }
  if (!equals(lastParams, newParams) {
    cached = await fetch();
    lastParams = newParams;
  }
  return cached;
}

export async function load({ params, fetch }) {
  const data = await refetchIfParamsChanged(() => fetch('/some-api-call'), params);
  return data;
}

@Rich-Harris
Copy link
Member

Unfortunately the proposed API is a non-starter due to its complexity, and because it would cause 'tearing' (post and userProfile would update separately, even if they're invalidated at the same time).

The per-load invalidation is good enough for the vast majority of apps, and allows us to have a super-simple API. It's possible to solve this in userland in a variety of ways, as demonstrated above, so on the basis that 'simple things easy, hard things possible' is better than 'simple things hard, hard things hard' I'm going to close this as wontfix.

@Rich-Harris Rich-Harris closed this as not planned Won't fix, can't repro, duplicate, stale Aug 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants