solidjs / solid

A declarative, efficient, and flexible JavaScript library for building user interfaces.
https://solidjs.com
MIT License
32.21k stars 917 forks source link

Data is being refetched on the client when the user interacts during streaming SSR #2217

Closed phonzammi closed 1 month ago

phonzammi commented 3 months ago

Describe the bug

Here in this example, I'm performing streaming SSR with renderToStream(), fetching data using createResource, and using <Suspense> to display a fallback placeholder. I want to test Progressive Rendering with solidjs.

While the page is loading and rendering, the server is fetching the data, the client displays the Loading ..., we click on the counter button and it's interactive already. But after the server finishes fetching the data, the Loading ... (Suspense boundary) disappears, and then the client starts fetching that data again.

There may currently be limitations with createResource when dealing with this kind of scenario.

Note that this only happens (refetch on the client side) if we click the counter button during rendering while the server is still fetching the data. It doesn't happen afterward.

Your Example Website or App

https://stackblitz.com/~/github.com/phonzammi/solid-streaming-ssr-test

Steps to Reproduce the Bug or Issue

  1. Load/Reload / page.
  2. Click on the counter button a couple of times (You'll see the counter is incremented).
  3. after the server finishes fetching the data, the Loading ... (Suspense boundary) disappears.
  4. Look on the browser console, the client starts fetching that data again.

Expected behavior

As a user, I expected that the page is already interactive, the client does not refetch again and simply displays the content after it has been fetched on the server.

Screenshots or Videos

https://github.com/solidjs/solid/assets/39896528/ba501fbf-294c-4e8d-b818-51ab8420bc60

Platform

Additional context

I've also tried it with solid-start (solid-start-streaming-ssr-test) and the result is the same.

Lastly, I tried with solid-query (solid-query-streaming-ssr-test) and everything works as expected

ryansolid commented 2 months ago

Yeah this is one of those things that I suspect people aren't going to like the answer to but it is a reality. If state changes before something hydrates we need to bail out otherwise it could cause a hydration error. It means that we don't take the streamed HTML but render it in the client ourselves. It means that we don't take the serialized resource data but refetch as the source of that data could depend on the state you changed.

Now you might be thinking, can't you do something smarter here? Possibly, but think about it what it would entail. Picture if the count impacted which movies were fetched. You don't know until you run the code that there is a data dependency there and at that point you can't really recover from it. You are midway hydrating a section and realize the data isn't there. You'd have to start over. More over how would you know it had changed. The signals themselves would have to know that they had been updated since start up and be able to propagate that information to whatever was reading it in a way that would only impact it during hydration. Maybe some sort of internal update clock that starts from zero. But even then how to have a read propagate that. This level of sophistication around data flow doesn't exist anywhere today.

The closest is another approach might be doing something like resumability, serializing the reactive graph and avoiding hydration in the normal sense. This could run the code for the first time from those reactive changes. I think long term this sort of approach might be compelling if we can overcome the additional serialization costs.

The solution today here is hoist the fetching or the cache. I imagine this is what Solid Query is doing. If it is not, then that is potentially unsafe. It's also what we do in Solid Router with our load and cache functions. In a sense this is what RSCs do because they know that their data can't depend on client state (there is no way to get client state/props into a server component). We can't do that here except by pulling the fetching to the synchronous top level because presumably it will finish hydrating the data fetching before any events are processed. Sure other things can change and need to be client rendered. But the data fetching will be considered resolved. Otherwise there is a risk of hydration error which is unacceptable to be caused by end user action.

phonzammi commented 2 months ago

Thank you very much for your detailed explanation; I really appreciate it, even though I couldn't understand or agree with everything just yet. I hope that's okay with you.

From one perspective, we know that the fetching doesn't depend on the counter. Unless we use it like this :

const [movies] = createResource(count(), fetchMovies)

Quoted from Solidjs docs :

There are two ways to use createResource: you can pass the fetcher function as the sole argument, or you can additionally pass a source signal as the first argument. The source signal will retrigger the fetcher whenever it changes, and its value will be passed to the fetcher.

phonzammi commented 2 months ago

Hi @ardeora, we would love to hear your thoughts on this. Please let us know.

The solution today here is hoist the fetching or the cache. I imagine this is what Solid Query is doing. If it is not, then that is potentially unsafe.

ryansolid commented 2 months ago

Thank you very much for your detailed explanation; I really appreciate it, even though I couldn't understand or agree with everything just yet. I hope that's okay with you.

From one perspective, we know that the fetching doesn't depend on the counter. Unless we use it like this :

const [movies] = createResource(count(), fetchMovies)

Yeah but you don't know that until you run the code.. At the time the counter is incremented the code for the resource may have not even loaded yet and it hasn't run. Which means that by the time we are running on the client collecting the deps the initial state has already been updated. The resource reads the count and sees the updated value and has no idea it has been updated. To be fair I said this could cause hydration errors, but more than likely we'd just swallow that update if this was the only thing we care about. It'd load the serialized data but it would be for the value of count that was on the server even though the client had it updated.

The truth is though other things could hydrate improperly if count was updated (like conditionals) so we have to bail out of hydrating. That is unavoidable. The question is whether we can still use the serialized data when not hydrating. Harder to determine. I'm not aware of any framework that can determine if specific state can be updated when running any evaluation that includes multiple state vars as state doesn't keep a history. More so if you aren't under "hydration" is this a sort of check you'd have to always perform on any initialization. The thing is it is possible to make the resource use the serialized data initially always but without being able to tell if the source is different your UI will be messed up.

What I imagine something like TanstackQuery does, similar to Solid Router's cache is it ties the data to a specific cache key. Which means for its purposes it knows if the source generates the same key then it can use the cached data. But since our createResource primitive is not key based it can't make that sort of assumption. But it also explains why people aren't likely seeing this in SolidStart.

brillout commented 1 month ago

The closest is another approach might be doing something like resumability [...] avoiding hydration

If there is a way to isolate the hydration bailout (so that clicking on a counter doesn't cancel the hydration of all potentially unrelated suspense boundaries), then that seems really neat :eyes:

Alternatively, I guess a solution is to first hydrate the suspense boundary before any state update. (While considering that suspense boundaries don't update state: they only set the initial state.)

I wonder whether Solid could postpone its whole signal mechanism after hydration, but maybe it contradicts Solid's eager rendering and/or Solid's philosophy.

Picture if the count impacted which movies were fetched. [...] This level of sophistication around data flow doesn't exist anywhere today.

I wonder what React does in this situation. Because React doesn't have built-in signals, maybe it simply doesn't have to handle this. And, I guess that tools that integrate with the HTML stream, such as https://github.com/vikejs/vike-react/tree/main/packages/vike-react-query#readme, follow the approach of "first hydrate with initial state before any state changes".

I guess the question then arises: how would a TansTack Query + Redux/Zustand integration look like.

RSC [...] they know that their data can't depend on client state (there is no way to get client state/props into a server component)

I see.

It's quite exciting that Solid is taking a fundamentally different approach than RSC. Looking forward to see how things will unfold.

ryansolid commented 1 month ago

Yeah.. there are a lot of challenges with serialization approaches like resumability because in preventing execution they need to remove closures to hoist the code. And this puts pretty strict rules on how component code is authored. And even with this there are other complications on making sure things can bail out just right. Because while resumability doesn't "hydrate" it still has the "initial state" === "server state" constraint. So to me that is only a starting point.

As you touched an alternative is delay all hydration but that is brutal. Even with event replay you basically have no visual interactivity until the slowest part of the page has loaded. This extends the uncanny valley time way worse than if you paint held. I think it basically negates most of the benefit of streaming. To be fair you could possibly delay the hydration call yourself if this is what you wanted to do. If you got on the end of the stream and flushed something to trigger the hydrate call I guess it would work. But it also isn't great.

As you also said React doesn't really handle much here. Like their selective hydration has the ability to bail out to prioritize, and when I asked Andrew Clark directly how they handled this case he said they bailed out of hydration very similar to how we do and just client render from that point. Since React doesn't manage the serialization of your data sources(outside of RSCs) it probably doesn't care and we have stuff like Tanstack doing the heavy lifting here.

Those libraries are key based so they don't really have any concern with data dep changes because it will miss cache anyway and fetch. If someone were to update it while streaming React might bail out like I said, but if tanstack still has the key it won't fetch again. So generally this is the solution. Our resources are too primitive to handle the case on their own because their automatic id generation don't have a key that is independent of hydration.

The best way to handle this is hoist the data fetching (but not the read) above the Suspense/Code splitting boundary (it's why we have preload in our router, Remix has loaders etc...). And/Or implement a key-based cache system on top of our Resources as does Tanstack Query and Solid Router's data API's. These work fine under these conditions.

Finally RSCs don't hydrate the server components so the impact of this stuff is smaller but it is still the same within the client components. That being said it encourages you to fetch on the server and not use like Tanstack Query etc... That is until you have heavily mutation based work flow and its lack of granular invalidation is a non-starter. So in a sense the path we are taking is the one that would have naturally flowed if RSCs hadn't existed, something we share with the Tanstack ecosystem which also is of this mindset.

phonzammi commented 1 month ago

I made a few changes to hoist the fetching. Please take a look and let me know if it looks good : https://github.com/phonzammi/solid-streaming-ssr-test/commit/a468ab68db022b3508cf60d658005fe2db4620c4?

https://github.com/user-attachments/assets/bc7ea930-9bb8-4ae9-b4d2-7fcfa9dc1ec6

phonzammi commented 1 month ago

Or this one: https://github.com/phonzammi/solid-streaming-ssr-test/commit/0eafc3420a7e6e66099a560ec210fe574f568af6

Which one do you think is better?

phonzammi commented 1 month ago

@ryansolid, thank you for all of your answers. I realize now that the mistake was mine.

I mistakenly included the fetching inside the <Suspense> component, even though I had done it correctly in the Solid Query test.

We can close this issue if you consider it resolved on your end.

ryansolid commented 1 month ago

Awesome great to hear that hoisting fixed it for you.