lwc icon indicating copy to clipboard operation
lwc copied to clipboard

Timing of connectedCallback produces inconsistencies in SSR HTML vs client-side VDOM

Open divmain opened this issue 2 years ago • 4 comments

During SSR, connectedCallback is invoked for a component after a HostElement has been created from the VDOM generated during render. In other words, the HostElement is created and accessible to the component (as this) inside a component's connectedCallback. Later, this HostElement is serialized to HTML.

On the client, the connectedCallback is invoked after validation. This must be true because we can only connect an existing HTMLElement to the component & its VDOM once we're sure they're in an equivalent state.

This gives us two invariants with the existing design:

  1. On the server: connectedCallback has access to the HostElement at the time of invocation.
  2. On the client connectedCallback cannot have access to the HTMLElement until after validation occurs.

The problem arises when the component author mutates the HostElement/HTMLElement in the connectedCallback. For example, adding something as simple as this.classList.add('bar'); means that:

  1. in SSR HTML, the component's root element will have a className of bar
  2. on the client, the component's root element will not have a className of bar at the time of validation

At present, the result is that

  1. the original DOM is removed,
  2. it is replaced with new DOM,
  3. which is immediately connected and mutated with the new class name.

The end result is the same, but we have a hydration failure and unnecessarily replace potentially large subtrees of DOM nodes.

This is illustrated as a failing test in 63a36c30fdc19a9df6de1d76cff562dc8abae08a.

divmain avatar Aug 17 '22 08:08 divmain

This issue has been linked to a new work item: W-11609112

git2gus[bot] avatar Aug 17 '22 08:08 git2gus[bot]

Related: #2987

Long-term, we should probably try to align the server with native custom element lifecycle behavior.

nolanlawson avatar Aug 17 '22 16:08 nolanlawson

Chatted with @divmain, we have a few options:

  1. Run hydration validation after connectedCallback on the client. Two problems here: a. the user could do something inconsistent in connectedCallback and introduce hydration validation errors that shouldn't be errors b. if validation fails, connectedCallback could run twice for descendants (because the entire DOM subtree is being recreated)
  2. Instrument DOM APIs that mutate the DOM (e.g. classList.add) so that they warn if they are invoked on the server in connectedCallback. (The error message could be something like: "This will cause a hydration mismatch, please move this code to the constructor() instead if possible.) This works for some cases (e.g. mutating this.classList), but not other cases (firing an event up the tree e.g. for Context or routing, this is probably best placed in connectedCallback even if it may work in the constructor).
  3. Instrument DOM APIs like above, but emit signals in the created HTML (e.g. data- attributes) that would signal to the client that it needs to do special validation. (Essentially this means storing the diff between the DOM state before connectedCallback and the DOM state after connectedCallback so that the hydration validator is aware of it.) a. Same as above, but only encode what changed (className, attributes, etc.), not how it changed, so that hydration knows to skip validating it.
  4. Disallow access to DOM APIs that mutate altogether, on the server. It would force people to find a different way to write their component. This is probably not feasible, because there are so many cases where someone wants to mutate the DOM in the constructor or connectedCallback.

Overall this doesn't really have much to do with timing of connectedCallback per se – it's about the timing of validation.

nolanlawson avatar Aug 22 '22 21:08 nolanlawson

A couple of notes from our discussion today

  • If validation is disabled entirely in prod mode, we may be able to simply forget about this validation error. In this unique case, the DOM before validation and the DOM at the end of hydration will be the same. Since we know that this is the case, we can leave the hydration validation error as-is in dev mode, and let it be a non-issue in prod mode.
  • We need to do an all-up review of DOM APIs, and 1) which ones will be supported on the server, 2) which ones will be a no-op on the server, and 3) which ones will throw an error on the server. It may be that we don't want to allow access to this.classList, which eliminates the class of bug entirely.

divmain avatar Sep 21 '22 06:09 divmain