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

Load function leaks props to other components #2133

Closed
vwkd opened this issue Aug 8, 2021 · 2 comments · Fixed by #2356
Closed

Load function leaks props to other components #2133

vwkd opened this issue Aug 8, 2021 · 2 comments · Fixed by #2356
Labels
bug Something isn't working p2-nice-to-have SvelteKit cannot be used by a small number of people, quality of life improvements, etc.

Comments

@vwkd
Copy link

vwkd commented Aug 8, 2021

Describe the bug

When returning an object with the props property from the load function in a page component, according to the docs they are available as props to the same page component

If the load function returns a props object [sic!], the props will be passed to the component when it is rendered.

However currently, the props are also passed to all other page components including the error component (but not the layout component!).

Furthermore, this issue also occurs for the load function in the error component. The load function in the layout component however behaves as expected.

I noticed this when reading through the console logs, which warned that page components that didn't expect any props were passed props. Upon further investigation, I found they were passed the props that were returned in the load functions of other page components.

This isn't really a breaking issue, because for every component the props returned from its own load function take precedence. So in practice the issue can occur only if you forget to remove props (export let data) that you don't use, or like me follow to investigate console warnings.

Reproduction

See https://github.com/vwkd/sveltekit-load-bug

This example has three different pages /, /about, and /about/contact as well as a layout and an error page. The layout component has a different colour than the page components for clarity. Click around the pages using the navigation menu.

The load function returning the props is in the index route. You can see that the props from the index's load function are passed to the about, the about/contact and the error page. They are however, correctly, not passed to the layout component.

Note, if you open the website on any other page than the index page, then the data will be undefined until you open the index page at least once, because the index page hasn't yet been prefetched.

main.mov

It's important to realise that there is nothing special about the index component. The load function could have been in any other of the components (about, about/contact or error). You can confirm this yourself by moving the <script context="module">...</script> code to another component. The error branch in the reproduction repo is such an example, where the load function is in the error component instead.

error.mov

It gets interesting when you have several components with prop exporting load functions, because the other components then switch their props depending on the latest component that was loaded. The double branch in the reproduction repo is such an example, where in addition to the load function in the index component there is a load function in the about/contact component. You can see the about component switched depending on if the index or the about/component was the last visited.

double.mov

Lastly, on the layout branch in the reproduction repo is an example of the load function in the layout component, which behaves correctly, and doesn't pass the props to other page components and error component.

layout.mov

Logs

No response

System Info

System:
    OS: macOS 11.5
  Binaries:
    Node: 16.6.0 - ~/.nvm/versions/node/v16.6.0/bin/node
    npm: 7.19.1 - ~/.nvm/versions/node/v16.6.0/bin/npm
  Browsers:
    Chrome: 92.0.4515.131
    Firefox: 90.0.2
    Safari: 14.1.2
  npmPackages:
    @sveltejs/adapter-static: ^1.0.0-next.11 => 1.0.0-next.16 
    @sveltejs/kit: next => 1.0.0-next.142 
    svelte: ^3.34.0 => 3.42.1

Severity

annoyance

Additional Information

Not sure if this is actually a Svelte bug, or a SvelteKit bug.

@vwkd vwkd changed the title Props returned from load function are passed to all page components and error component Load function leaks props to other components Aug 8, 2021
@benmccann benmccann added bug Something isn't working p2-nice-to-have SvelteKit cannot be used by a small number of people, quality of life improvements, etc. labels Aug 9, 2021
@rmunn
Copy link
Contributor

rmunn commented Sep 1, 2021

I've investigated this, and I think I've found the line of code responsible for this behavior. In renderer.js, there's a loop that sets the props_0, props_1, etc. properties on the root component from the results of the load() functions of each component in the nested tree. props_0 gets set from the props of the top-level component (in this case, __layout.svelte), props_1 is set from the next component in the tree (whatever __layout.svelte has in its <slot></slot>), which in this case is index.svelte or about.svelte or about/contact.svelte and even __error.svelte when you load the "Incorrect" link.

Thing is, the props_1 property of the root component is only set when there's a load() function, because of the if (loaded) part of that line of code. If there's no load() function, then the props_1 property of the root component is left unchanged, which is why you're seeing this "leaking" happening. Because when you load index.svelte, props_1 gets set to { data: "Hello from Index!" }. Then when you load about.svelte, since there's no load() function that line of code is skipped and the props_1 property is not changed.

You can verify this by adding a load() function that does nothing to your about.svelte component:

<script context="module">
    console.log("Prefetching about component.");

    export async function load() {
        return { };
    }
</script>

If you make that change (and nothing else), then you'll see that visiting the About page resets the data prop to undefined, but visiting About/Contact (which doesn't have a load() function) leaves the data prop containing whatever it used to contain.

The evidence that adding a load() function changes this behavior makes me pretty sure I've identified the line of code responsible (which was introduced in #901). But I'm not sure what to do about it. Should that if block acquire an else condition like this?

const loaded = filtered[i].loaded;
if (loaded) result.props[`props_${i}`] = await loaded.props;
else result.props[`props_${i}`] = {};

Or perhaps that could be simplified down to:

const loaded = filtered[i].loaded;
result.props[`props_${i}`] = loaded ? await loaded.props : {};

But I don't know what else might be affected by making that change, and I don't have time right now to make that change in Svelte-Kit and test it thoroughly. So I'll have to leave creating a PR to someone else.

@rmunn
Copy link
Contributor

rmunn commented Sep 3, 2021

Found the time to write up a unit test and create a proper PR: #2356.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working 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.

3 participants