Skip to content

Accumulate data into export let data #5994

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

Closed
sserdyuk opened this issue Aug 17, 2022 · 7 comments · Fixed by #6050
Closed

Accumulate data into export let data #5994

sserdyuk opened this issue Aug 17, 2022 · 7 comments · Fixed by #6050
Labels
feature / enhancement New feature or request p2-nice-to-have SvelteKit cannot be used by a small number of people, quality of life improvements, etc.
Milestone

Comments

@sserdyuk
Copy link

Describe the bug

This is an "opinionated" take on the recent changes to routes and load function behavior. It's a big change, that I am not a fan of but can see the merit. Among other things it now requires more boilerplate code, which we all don't like to deal with.

Here's the problem: +page.svelte doesn't receive data unless there's +page.ts file with load function even though we might have data set by parent layouts. This forces one to create a bunch of +page.ts files with a the following boilerplate. Wouldn't be nicer to do that as a default for all pages, so we don't have to create +page.ts files unless we're actually doing something specific there?

import type { PageLoad } from './$types';

export const load: PageLoad = async ({ parent }) => {
  return await parent();
};

Reproduction

https://stackblitz.com/edit/sveltejs-kit-template-default-f8ky2l?file=src/routes/+page.js

The +page.js file seems to be completely unnecessary but you can't remove it or +page receives no data.

Logs

No response

System Info

System:
    OS: Linux 5.0 undefined
    CPU: (4) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 0 Bytes / 0 Bytes
    Shell: 1.0 - /bin/jsh
  Binaries:
    Node: 16.14.2 - /usr/local/bin/node
    Yarn: 1.22.10 - /bin/yarn
    npm: 7.17.0 - /bin/npm
  npmPackages:
    @sveltejs/adapter-auto: next => 1.0.0-next.64 
    @sveltejs/kit: next => 1.0.0-next.416 
    svelte: ^3.46.0 => 3.49.0 
    vite: ^3.0.0 => 3.0.8

Severity

serious, but I can work around it

Additional Information

No response

@babichjacob
Copy link
Member

babichjacob commented Aug 17, 2022

This is less work now than it used to be.

You can use $page.data to get data from ancestors and/or children without writing a load function and thereby +page.js file at all.

<!-- src/routes/+page.svelte (from the reproduction link). Could also be used in src/routes/+layout.svelte or any component, for example -->
<script>
  import { page } from "$app/stores";
</script>

<div>
{JSON.stringify($page.data)}
</div>

@sserdyuk
Copy link
Author

@babichjacob Thank you for the idea. Unfortunately, $page doesn't pass the types from the parents.

@benmccann
Copy link
Member

If it were done by default then the load functions could no longer run in parallel

@sserdyuk
Copy link
Author

@benmccann Yes, but you're by default sacrificing the brevity and requiring more boilerplate for an unknown amount of performance optimization. One could optimize for performance by creating the .ts file with a specific load function when it's really necessary.

@Rich-Harris
Copy link
Member

This isn't a regression. Previously you would have had to have a load function that received stuff and turned it into props. There is no way in which the current way of getting data into your page is more verbose than it used to be.

Yes, of course the amount of performance optimisation is unknown. That would be a very silly reason for making things unnecessarily slow by default.

@sserdyuk
Copy link
Author

Just for the record, I disagree. I think optimizing for brevity and developer productivity is more important than runtime performance, provided the developer has an option to optimize for it in specific cases when it's necessary.

@Rich-Harris
Copy link
Member

Our ultimate responsibility is to end users. We don't want to make it easy to build fast sites, we want to make it hard to build slow sites. It doesn't matter how conscientious individual developers are — if you have slowness baked into the system, optimizing it away will forever be on the backlog.

That said, we realised that we can merge data without affecting the ability to run load functions concurrently. Rendering doesn't begin happening until loading is complete, which means that each layout/page node can be rendered with the equivalent of { ...await parent(), data }. In that case PageData would include types of the parent data as well.

@Rich-Harris Rich-Harris reopened this Aug 17, 2022
@Rich-Harris Rich-Harris changed the title Unnecessary boilerplate Accumulate data into export let data Aug 17, 2022
@benmccann benmccann added this to the 1.0 milestone Aug 17, 2022
@dummdidumm dummdidumm added feature / enhancement New feature or request p2-nice-to-have SvelteKit cannot be used by a small number of people, quality of life improvements, etc. labels Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature / enhancement New feature or request p2-nice-to-have SvelteKit cannot be used by a small number of people, quality of life improvements, etc.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants