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

+layout.server.js not correctly passing data down during client-side hydration #5904

Closed
robertadamsonsmith opened this issue Aug 16, 2022 · 5 comments · Fixed by #5916
Closed
Labels
bug Something isn't working
Milestone

Comments

@robertadamsonsmith
Copy link

Describe the bug

After client side navigation, calling await parent() from within a +page.server.js exported load function returns {"data":null} instead of the value returned from the +layout.server.js exported load function

Reproduction

Logs

No response

System Info

stackblitz default template fork

Severity

serious, but I can work around it

Additional Information

No response

@pilcrowonpaper
Copy link

Seems like it works fine if +layout.server.js is changed to +layout.js

@Rich-Harris Rich-Harris added the bug Something isn't working label Aug 16, 2022
@Rich-Harris Rich-Harris added this to the 1.0 milestone Aug 16, 2022
@robertadamsonsmith
Copy link
Author

Seems like it works fine if +layout.server.js is changed to +layout.js

Good to know that the issue doesn't affect +layout.js. I need to use +layout.server.js to fetch server side data though, so that doesn't help.

If I didn't need to fetch server side data in my layout, that would be fine. Unfortunately +layout.js doesn't run on the server, and so can't access databases etc., which is why +layout.server.js exists.

@rmunn
Copy link
Contributor

rmunn commented Aug 16, 2022

On first navigation, the load function in +layout.server.js is run, because the server is hit. The second navigation is a client-side-only nagivation, so the server isn't hit, and therefore +layout.server.js isn't run and the values it returns aren't preserved. And instead, await parent() gets { data: null }, which indeed doesn't seem right.

In #5883 (reply in thread), Rich Harris wrote that load results are re-used; the question I don't yet know the answer to is whether load results from server load functions are intended to be preserved after client-side navigations. I suspect the answer is yes, in which case the issue title should probably be changed to "load() results from server discarded after client-side nagivation" to better reflect what the actual bug is.

@robertadamsonsmith
Copy link
Author

I agree - I don't know if changing the title of the issue would confuse somebody that has already seen it once though. Unless the issue extends beyond the +layout.server.js though, perhaps the title should be: "load() results from +layout.server.js discarded after client-side navigation"

I've reduced the reproduction to one which shows the problem in a more minimal way (i.e. without a +page.server.js and await parent()):
https://stackblitz.com/edit/sveltejs-kit-template-default-euznnp?file=src/routes/+layout.svelte

dummdidumm added a commit that referenced this issue Aug 16, 2022
Fixes #5904

- the data property was not unwrapped on the server, resulting in wrong results on client side navigations
- the root layout data was ommitted because of a wrong undefined check
@rmunn
Copy link
Contributor

rmunn commented Aug 16, 2022

#5921 (comment) suggests that it would be good to change the title. Though that will hopefully become a moot point once #5916 is merged and released.

@Conduitry Conduitry changed the title await parent() not working correctly +layout.server.js not correctly passing data down during client-side hydration Aug 16, 2022
Rich-Harris added a commit that referenced this issue Aug 16, 2022
* [fix] correctly provide server parent data

Fixes #5904

- the data property was not unwrapped on the server, resulting in wrong results on client side navigations
- the root layout data was ommitted because of a wrong undefined check

* more robust tests

* Update packages/kit/src/runtime/server/index.js

* Update packages/kit/src/runtime/server/index.js

* Update packages/kit/src/runtime/server/index.js

* deflake

* go ham

Co-authored-by: Rich Harris <richard.a.harris@gmail.com>
Co-authored-by: Rich Harris <hello@rich-harris.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants