refactor(hydration): refactors hydration logic to only render once on mismatches

[jodarove]

Details

This PR refactors the hydration logic based on vue.js.

The main advantage of this approach (vs master) is when there are mismatches while doing the hydration. With this approach:

1. The component tree only needs to render once (vs two times).
2. It reuses more DOM from SSR, ex: there's one mismatch in the root node of a subtree, only that node (and subtree) is replaced with the client vdom (vs replacing the whole tree, dismissing already hydrated nodes).
3. (as of now) It recovers from mismatches in production, always ensuring that the rendered DOM is from the client vdom.

Another benefit of this implementation is that it allows (or will) partial hydration of children in a cleaner manner. (required in #2716)

The downside of this approach is related to perf in production (when there are no mismatches). See performance analysis section. I want the reviewer's feedback on this matter.

Performance analysis

Current PR as-is:

The cause of the performance regression is that with this PR, the hydration logic tries to find mismatches even in prod, to ensure that the rendered DOM matches client vdom. But the existing logic (master) skips these checks in prod, assuming that everything matches.

if those mismatches check are skipped from prod (like in master), this PR improves the hydration performance:

Do you think we should skip the verification elements (and ce) attributes for mismatches with csr in prod or keep the PR as is? Pls, share your thoughts.

Does this pull request introduce a breaking change?

• ✅ No, it does not introduce a breaking change.

Does this pull request introduce an observable change?

• ✅ No, it does not introduce an observable change.

[nolanlawson]

I think the question of whether the perf hit is acceptable or not hinges on these questions:

• How common are hydration mismatches?
• What causes a hydration mismatch?
• What scenarios commonly have hydration mismatches (e.g. B2C vs B2B)? Are these scenarios perf-sensitive?
• Do developers have a lot of control over hydration mismatches? I.e. if they see a warning about it in dev mode, can they reasonably fix it?

[nolanlawson]

Is there an error message we could put in here that would help the developer solve the mismatch?

[jodarove]

this is a general error shown after hydration is completed, on top of the errors shown to help devs to solve the mismatches.

[jodarove]

ill answer your questions inline:

I think the question of whether the perf hit is acceptable or not hinges on these questions:

• How common are hydration mismatches?

In theory, there should be none, but in practice, there may be some due to stale data on the server, ex: cache the server response, and during the cache ttl some of the data was updated; or differences in the environment, ex (just as an example): locker running on the client and not the server.

• What causes a hydration mismatch?

When SSR generates content different than the one generated on the client. This will usually happen when SSR is using different data from the client, or some library only available on the client (or server) modifies the client vdom output (ssr output). Ex: some content is sanitized only on the server and not on the client (or vice-versa).
if SSR and Client have a compatible environment and use the same data, then the mismatch is a bug in the hydration logic.

• What scenarios commonly have hydration mismatches (e.g. B2C vs B2B)? Are these scenarios perf-sensitive?

I can't talk by B2C or B2B, I would say all scenarios are perf sensitive. IMHO, when there is a mismatch, is because a "user error", ex: when updating a record it does not invalidate the cache of the rendered ssr page for that record.

for me, the question is: when in prod, if there's a mismatch, is it ok that the rendered DOM is not accurate? To this, I would say no, but this is something that is specific to the apps (B2C, etc.) using hydration. I would like the reviewer's opinion on it.

• Do developers have a lot of control over hydration mismatches? I.e. if they see a warning about it in dev mode, can they reasonably fix it?

I would say yes.

[nolanlawson]

for me, the question is: when in prod, if there's a mismatch, is it ok that the rendered DOM is not accurate?

I see, so the choice is between incorrect data in prod vs improved performance in prod. Based on what you say about mismatches, they also sound like they would be quite common. In that case, it seems that the perf tradeoff is worth it.

[pmdartus]

for me, the question is: when in prod, if there's a mismatch, is it ok that the rendered DOM is not accurate?

I see, so the choice is between incorrect data in prod vs improved performance in prod. Based on what you say about mismatches, they also sound like they would be quite common. In that case, it seems that the perf tradeoff is worth it.

I would second @nolanlawson's feedback.

Here are a couple of thoughts about the performance regression:

• To effectively evaluate the performance regression introduced by this PR, I think we should compare the client-side rendering time vs. the hydration time. I don't think we have this data point today.
• I would like to challenge our approach for validating attributes and more specifically validateClassAttr and validateStyleAttr. The current assumption is that it is always more performant to compare the attribute value with the VNode values. Is it really the case? Could it be more performant to always patch the style and class attributes?

[pmdartus]

Suggested change

	function createVMWithProps(element: Element, Ctor: typeof LightningElement, props: object) {
	function createVMWithProps(element: Element, Ctor: typeof LightningElement, props: object): VM {

[pmdartus]

The connectedCallback is invoked here and in hydrateCustomElement. Is there a way to centralize this to reduce the risk for lifecycle invocation timing mismatch between the root and the children components?

[jodarove]

ill say yes, by creating a virtual vnode for the root element. however, I would not like to include it in this PR.

[nolanlawson]

Could it be more performant to always patch the style and class attributes?

I did this analysis for the style attribute a few months ago. TL;DR our current approach is faster for <3 props, whereas patching the attribute directly is faster for >=3 props. My hunch is that most style attrs have <3 props, so I kept the existing behavior.

That said, there may be some other benefits to patching the style / class attributes directly, e.g. making our code simpler.

[jodarove]

I would second @nolanlawson's feedback.

👍

Here are a couple of thoughts about the performance regression:

• To effectively evaluate the performance regression introduced by this PR, I think we should compare the client-side rendering time vs. the hydration time. I don't think we have this data point today.

I did this analysis when adding the perf tests: #2663. lmk if you think something should be added.

• I would like to challenge our approach for validating attributes and more specifically validateClassAttr and validateStyleAttr. The current assumption is that it is always more performant to compare the attribute value with the VNode values. Is it really the case? Could it be more performant to always patch the style and class attributes?

I ran the two with a modified test so it does include classes, attrs and styles, comparing: this PR as is (tip-of-tree), and only checking for tagName and always patching attrs and props (-this-change). comparing the attributes is faster.

What's your opinion about a middle ground between what this PR is proposing and what's on master?
Given node and vnode, which are a mismatch:

• check if the elements are compatible (have the same tagName), if they are not, we would replace the node.
• if they are compatible, in prod only, skip the attrs validation, assuming it is a match and continuing the hydration.

With this approach, the only difference (in prod mode) between the server and client will only be the attributes. Notice that any mismatch down the tree will get resolved, and for custom elements, the props are always set. And it improves perf compared to master:

[pmdartus]

With this approach, the only difference (in prod mode) between the server and client will only be the attributes.

As discussed previously, I would favor consistency over performance. We can remove those guardrails in the future are they are too noisy/slow.

Could it be more performant to always patch the style and class attributes?

I did this analysis for the style attribute a few months ago. TL;DR our current approach is faster for <3 props, whereas patching the attribute directly is faster for >=3 props. My hunch is that most style attrs have <3 props, so I kept the existing behavior.

The rehydration assertions are doing the inverse operations than in rendering. We analyzed what is the fastest way to set attributes, here we are interested in what is the fastest way to check if the element attribute value is with the VNode attribute.