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

RFC: Hydration 2.0 #4442

Open
JoviDeCroock opened this issue Jul 12, 2024 · 2 comments
Open

RFC: Hydration 2.0 #4442

JoviDeCroock opened this issue Jul 12, 2024 · 2 comments

Comments

@JoviDeCroock
Copy link
Member

JoviDeCroock commented Jul 12, 2024

With Streaming Server Side Chunks as well as rendering to string asynchronously becoming more popular it's becoming apparent that our hydration algorithm isn't quite sufficient anymore. There's a set way in which it will behave well for these resumed hydration cases but a lot of innocent-looking footguns that can lead to opting out of hydration or in some cases UI bugs.

The goal of hydration is to re-use the HTML that is returned to us from the server and just attach the event-listeners so the UI is usable to the consumer again, this is the case for simple full-on hydration. We later on introduced the concept of resumed hydration so we could hydrate Suspense boundaries etc, the idea was that when a child throws a Promise because the JS chunk is being downloaded, ... we'd pause hydration, mark that VNode as in need for hydration, save the DOM-pointer we're on and continue. When the Promise resolved we could continue from the VNode with the DOM pointer and continue our hydration.

The benefit of this resumed hydration approach is that we can have parts of the DOM ready for interaction before all chunks (be that data or JS) are ready on the client. There's one upgrade over this which is streaming the HTML in as then we can even forego the latency introduced by those Promises on the server. This RFC explicitly does not look too deep into streaming, we touch on it in the bonus section, however we aim to solve the renderToStrincAsync() into resumed hydrate() workflow.

The main problem tackled in this RFC resides in the storing of the DOM-pointer to continue hydration from. When we are storing the DOM-pointer, we aren't aware what content that suspended node will return when it ends up resolving, i.e. will this render function return null, 1 DOM-node or multiple.

Problems

Suspending render returns null

When we have the following case

const X = () => {
  // Some logic to i.e. create your google-analytics script
  return null;
}

const SuspendingX = lazy(() => /* importing x */)

Then we'll suspend as soon as we try to render SuspendingX, however we'll reserve a DOM-pointer for this that isn't usable for any of its siblings anymore while this component will end up returning null so when we see <div><SuspendingX /><p>hello world</p></div> we'll reserve the <p> tag to hydrate <SuspendingX /> and end up duplicating <p>hello world</p> because we think it's a hydration mismatch.

Related issue #4075

Offset creation with multiple DOM-children

This builds on the previous case but instead of returning null it will return multiple DOM-nodes.

const Y = () => {
  return <Fragment><p>Foo</p><p>Bar</p></Fragment>;
}

const SuspendingY = lazy(() => /* importing y */)

When this VNode ends up suspending we'll reserve a DOM-pointer, which is the whole issue, because it's one DOM-pointer.... if we have any sibling that also is a <p> tag we'll end up hydrating with the DOM of <p>Bar</p> and the other way around.

Related issue #4438

Conditional DOM

This is an issue with hydration in general however the issue is a bit bigger when it comes to resumed hydration. A code snippet I end up seeing a lot looks like typeof window !== 'undefined' ? renderJsx() : null which is an automatic hydration mismatch because this DOM will never exist. Now there are solutions to this and one of them is creating some state that reflects your hydration state

let isHydrating = true;
hydrate(<App />, document.body)
isHydrating = false;

export const useIsHydrating = () => {
  const [isHydrating, setIsHydrating] = useState(isHydrating);
  useEffect(() => {
    setIsHydrating(false)
  }, [])
  return isHydrating
}

Effects only run on the client so when useEffect runs we are sure that hydration has completed. Now with resumed hydration this isn't the case, there are multiple Promises resolving and only when all of them are resolved we can talk about completed hydration, which can't be observed with an effect. We could of course make the above hook dumber and always render twice, but that is in a lot of case undesirable.

Related to this, observing mismatched nodes with i.e. the MutationObserver becomes harder as we don't know which mutations are part of a hydration boundary resolving or from a user interaction.

Proposed solution

The requirements that this solution should solve are that we have to basically be aware of the amount of DOM-nodes being returned from a Suspended node to create a certainty that we'll be able to resume hydrate correctly.

SSR markers

When we are server-side rendering with renderToStringAsync we inject markers for every suspending VNode, this marker will denote all DOM-nodes inside of this lazy boundary, i.e. for the following example

// x.jsx
const X = () => {
  <><div>Foo</div><div>Bar</div></>
}

// App.jsx
const LazyX = lazy(() => import('./x.jsx'))
const App = () => {
  return (
    <main>
        <Suspense><LazyX/></Suspense>
	    <div>Baz</div>
	</main>
  )
}

When we run our renderToStringAsync function we'd expect markers to denote the two produced nodes.

<main>
  <!-- $s -->
  <div>Foo</div>
  <div>Bar</div>
  <!-- /$s -->
  <div>Baz</div>
</main>

Now when we are hydrating and reserving DOM-nodes to continue hydration from, we can reserve the accurate amount of DOM-nodes. Similarly, if that would return null we'd just have two comment nodes with nothing in-between.

The most important part of this approach would be that we can accurately tell that boundary X has a certain amount of excessDomChildren, this will mean that as long as we are hitting Suspending children we'll slowly chip away from the excessDomChildren we have derived thanks to these comment-nodes.

One of the issues I see is that we don't really check leverage boundaries within Preact-render-to-string, instead we catch the error and await the Promise. This means that we'd have to denote every individual suspending component and its DOM and the logic to reserve these suspending DOM-nodes for excessDomChildren would live in Preact.

Bonus

Some more thoughts I had while writing this... Most of these could become their own RFC, which for streaming in particular warrants a follow-up.

Streaming

None of the above solutions provide a holistic solution for streaming renders, because with streaming renders we'd have the fallback of a Suspense boundary active, which hopefully is either nothing or a single DOM-node. We should however inform ourselves with the presence of the preact-island comment node to either not reserve a DOM-node or looking at the fallback of a given encountered Suspense boundary.

One of the most pressing investigations for streaming would be the reason behind needing a DOM-node to wrap your Suspense boundary and investigating how resumed-hydration behaves when the fallback is being switched out for the actual content. I reckon that finding a solution to these might involve us needing a fixture-based testing suite where we can actually leverage streaming, ...

Debug checks

There's also more to be done on the DX front, even if we wouldn't go for any of the above solutions we should build some developer warnings for these malformed hydrations.

Warn for multiple/no node returns

When we are in resumed hydration mode we should observe the vnodes that have _hydrating set to true and when they resolve and return either no or multiple DOM-nodes we should warn our user.

New MutationObserver snippet

In our wiki we currently provide a snippet to spot hydration mismatches with the mutation-observer, this observer tears down after we complete our synchronous hydration attempt. This does not account for any Suspenseful boundaries that have yet to resolve and by extension means that we'll fail to report some mismatches.

Hydration callback

A type of callback for when hydration finishes would be really good to have to support having state inside of components like

export const useIsHydrated = () => {
  const [isHydrated, setIsHydrated] = useState(false)
  useEffect(() => { setIsHydrated(true) },[])
  return isHydrated;
}

The above is often used to avoid rendering something on the server and also not hydrating on first pass, however that means that any client-side rendered component will be subject to two renders while we only want this to impact our first hydration pass.

Existing work

In following this approach we could supersede PR's like #4438 and #4437. We'll however need to document the remaining common pitfalls, like using typeof document for non-null returns during hydration and mismatches resulting from spec in compliant DOM. Debug checks for the latter already exist and the former probably just needs documenting.

@JoviDeCroock
Copy link
Member Author

Something else that I have been wondering is whether we should make our lookup for excessDomChildren stricter i.e. if the VNode has a prop id with a value then we should maybe consider this similar to how we treat the key property when we are diffing children.

@JoviDeCroock
Copy link
Member Author

Another thing worth doing would be to fully bail out of hydration when we have a mismatch, this means unmounting all excessDomChildren and switching to a full normal render instead, even for siblings, parents,... the danger currently is that a sibling does not opt out and ends up duplicating UI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant