tomasklaen / statin-preact

Preact bindings for statin.
3 stars 0 forks source link

SSR / SSG - Preact component signal "observer" causes 10 seconds pause / process blocking #2

Closed danielweck closed 2 years ago

danielweck commented 2 years ago

Hello :) Once again feel free to move this "issue" to a GitHub "discussion" if you feel this is out-of-scope with respect to Statin's design / feature set / intended use.

I am currently playing with Statin in a side project, where I use Preact WMR's SSG (build-time SSR) to statically pre-render webpages on the server side (MPA model), and to subsequently hydrate webpages in web browser clients (transition to SPA).

The 10-seconds setTimeout() "cleanup" routine for non-committed reactions (i.e. rendered in VDOM, but not DOM-mounted) causes a massive process block in the command line during the NodeJS build. This is obviously unnecessary as Preact's "render to string" doesn't expect component state to change beyond initial render (note that in Preact WMR, async components / lazy-loading is supported via the preact-iso router and also via preact/compat Suspense: "render to string" is simply invoked until the thrown Promises resolve ... but the component tree is statically-rendered based on the initial state only, no reactions/actions/effects are expected to run, no support for useEffect etc.).

My workaround is simply to reduce the 10-seconds setTimeout() to zero, by testing typeof window ==='undefined' 😄 I am sure there is a more sophisticated fix, such as disabling a good chunk of the component observer() logic, but frankly in my opinion this is not worth the effort. I can't see any negative side-effects (pun intended) in letting Statin's bootstrap routine run its course in a wrapped component, even if immediately aborted in a server-render. In fact, I would worry about introducing inconsistencies between the pre-rendered DOM and its mirror hydrated image (hydration bugs due to mismatched component states or generated (V)DOM are easy to create, hard to debug).

https://github.com/tomasklaen/statin-preact/blob/ea430a280f1577a7ae80aec5a030765ee3542e78/src/index.tsx#L19-L25

https://github.com/tomasklaen/statin-preact/blob/ea430a280f1577a7ae80aec5a030765ee3542e78/src/index.tsx#L45-L49

tomasklaen commented 2 years ago

Really sorry for the long response time 😞.

SSR is definitely not out of scope, I'd very much like this to work nicely with it, the problem is that I didn't need to use SSR yet, therefore I have exactly zero experience with everything related to it.

The ideal solution, as you've mentioned, would be to reliably detect we are in an SSR context, and simply render without setting up any once() reactions or useEffect()s. But as I've said, I'm not familiar with SSR context, so I don't know how to detect it, and relying on window == undefined seems dangerous. For example, afaik there's nothing stopping people from using preact-render-to-string in the browser.

From briefly reading through preact docs on SSR, I haven't noticed anything that would help here. I've also tried asking on preact slack, I'll update with the response. Alternatively I could study the source code, but I don't have that much time to sink into this atm 😞.

tomasklaen commented 2 years ago

According to people in preact slack, the only reliable method is an environemnt flag.

This means probably some setSSR(enabled: boolean) API that makes observer() ignore reactivity and just render while this flag is enabled.

On server, we could potentially default this flag to true if SSR is detected. Are there any SSR related ENV variable conventions?

danielweck commented 2 years ago

On server, we could potentially default this flag to true if SSR is detected. Are there any SSR related ENV variable conventions?

typeof window !== 'undefined' or typeof document !== 'undefined' are pretty standard ways to detect Server-Side-Rendering (which includes Static Site Generation) ... even though it may not actually be super accurate / it may cause false positives / negatives (e.g. Web Workers self context).

Best to wrap a boolean getter in isSSR(), and adapt as appropriate (e.g. Deno detection).

Just to name a few examples:

tomasklaen commented 2 years ago

Thanks for the exhaustive list!

Looking at it, trying to detect this internally to set a default seems too fragile to me. All of the checks are just window == undefined. If I used this alone to turn off observing by default, it would break in a context where there is no window, but stuff should be reactive, for example some react-native like projects. I know preact is DOM only, but who knows, I've been bitten by pretending I can predict all use cases before. So I guess it'll have to be explicit, e.g. only set by users manually.

I'm still trying to figure out the best name for this though. "SSR" seems too targeted since this could potentially be used for other reasons as well.

Something like passThroughMode maybe. I'm happy to hear any ideas :)

danielweck commented 2 years ago

Hello Tomas, just a quick courtesy email to let you know that I am lacking the time to tinker with this side project. Feel free to close my GitHub issues or convert them to "discussions" in order to keep your project tracker clean :)

tomasklaen commented 2 years ago

Oh no worries, you've already helped way more than I could possibly expect :)

And the issues are still relevant, I want them resolved myself, so no need for closing them.