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

createResource onHydrated option not called during hydration #967

Closed
katywings opened this issue May 7, 2022 · 3 comments
Closed

createResource onHydrated option not called during hydration #967

katywings opened this issue May 7, 2022 · 3 comments
Labels
bug Something isn't working
Milestone

Comments

@katywings
Copy link
Contributor

Describe the bug

The onHydrated option in createResource is not called during hydration.

Your Example Website or App

https://github.com/katywings/solid-on-hydrated-repro/blob/master/src/routes/index.tsx

Steps to Reproduce the Bug or Issue

  1. Setup solid with ssr (e.g. using solid-start)
  2. Change a route and add a createResource using the onHydrated option
  3. Open up the route in the browser
  4. onHydrated will not be called during client hydration

Expected behavior

The onHydrated option in createResource should be called during hydration (if I understand the usecase for this option properly)

Screenshots or Videos

onHydrated.reprod.mp4

Platform

  • OS: [Linux]
  • Browser: [Firefox]
  • Version: [1.3.17 and 1.4.0-beta.2]

Additional context

Potential solution:
During my debugging I noticed, that the variable p is empty during hydration. But the variable v is set and has the proper value. Replacing p with v at the following line, would solve the problem and make it so that onHydrated is actually called:

if (initP && p === initP && options!.onHydrated)

But tbh. i don't understand the full createResource logic... changing p to v might break other things 😅.

Video of my proposed solution:

onHydrated.fix.mp4

Sidenote: this is related to the discord discussion about TurboSolid: https://discordapp.com/channels/722131463138705510/722167424186843267/971362009184620584

@katywings
Copy link
Contributor Author

P.S: @ryansolid Apart from above issue, I noticed a behaviour about onHydrated that maybe should be documented (when onHydrated is officially announced :D)

Signal setters don't work inside of onHydrated, you have to wrap them with raf or setTimeout:
https://github.com/katywings/solid-on-hydrated-repro/blob/master/src/routes/index.tsx#L14

@ryansolid
Copy link
Member

ryansolid commented May 8, 2022

Yeah it's a timing thing. It can be the value or the promise. So we should check either. Simple fix. It will be in 1.4. What is more interesting is the signal setting. It is due to still being hydration at that time. Which means it does trigger the graph, just rendering is skipped. I will fix that too I guess. I was thinking we needed to set this earlier but it's fine I think push it to the end of hydration.

EDIT:
Hmm.. yes it is more awkward if it can change data that hasn't be read yet in downstream portions not yet hydrated like during streaming. It would be better not to try to set signals here.

ryansolid added a commit that referenced this issue May 8, 2022
@katywings
Copy link
Contributor Author

It can be the value or the promise. So we should check either. Simple fix

Awesome ❤️, thank you 🙏!

EDIT: Hmm.. yes it is more awkward if it can change data that hasn't be read yet in downstream portions not yet hydrated like during streaming. It would be better not to try to set signals here.

Yeah I think it makes sense like this, kinda similar to how batching works "glitch-free" 😉.

@ryansolid ryansolid added the bug Something isn't working label May 9, 2022
@ryansolid ryansolid added this to the 1.4.0 milestone May 9, 2022
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

No branches or pull requests

2 participants