[lit-html - SSR] missing parent check for defer-hydration? #2946
Comments
|
My current hypothesis is that this could happen if you were hydrating a shadow root with an immediate child that is a void (aka empty) element, such as render() {
return html`<input max=${this.maxLen}>`;
}Void elements are special because when the HTML parser encounters a child of one of them, it hoists the child up as a sibling, since those elements aren't supposed to have children. In SSR, when an element has an attribute binding, we represent that with a child comment like Working on a repro to confirm. |
|
indeed that was the element render function render() {
return html`
<p>Hello World</p>
<input
type="text"
value=""
class="shadow-input"
@focusin=${this.handleFocusIn}
/>
`;
}and it was confusing as removing but as you now explained it - it totally makes sense 👍 |
### Background In SSR, when an element has an attribute binding, we insert a `<!--lit-node 0-->` comment to tell `hydrate()` about the attribute binding. However, if that element was a *void* element, such as `<input>` or anything from [this](https://html.spec.whatwg.org/multipage/syntax.html#void-elements) list, then when the HTML parser encounters it, all of the children it has will be hoisted up as siblings. For example: ```ts render() { return html`<input max=${this.maxLen}>`; } ``` Is SSR'd as: ```html <input max="42"><!--lit-node 0--></input> ``` But parsed by the browser as: ```html <input max="42" /> <!--lit-node 0--> ``` This means when `hydrate()` encounters a case like this, we need to check `previousSibling` instead of `parentElement`, to find the element to receive the attribute binding. ### Bug We already accounted for void elements where we actually create the attribute parts: https://github.com/lit/lit/blob/ac356997351874706f8be235559c765861dce67d/packages/lit-html/src/experimental-hydrate.ts#L340 But we did not account for it where we remove the `defer-hydration` attribute: https://github.com/lit/lit/blob/ac356997351874706f8be235559c765861dce67d/packages/lit-html/src/experimental-hydrate.ts#L161 This actually only crashed `hydrate()` if the void element was an *immediate child of a shadow root*, because in every other case, `element.parentElement` would return *something*, even if it wasn't the correct node. ### Fix We now also account for void elements when we remove the `defer-hydration` attribute in the same way that we already did for creating attribute parts. Note as part of this I switched from `previousSibling` to `previousElementSibling`, so that we can assume `removeAttribute` is defined. Seems like this should be safe? Hopefully fixes #2946 cc @daKmoR ### Alternative idea I also thought about an alternative way to handle void elements generally, which is to detect them up-front during SSR rendering, since they are a small fixed set of tag names, so that we get an explicit signal about whether we need to check the parent or the sibling during hydration, instead of doing the previousSibling -> parentElement fallback. See 66b702b for how that would look. This could potentially have slightly better performance during hydration, since it replaces a DOM call with a string check. Not sure it's worth it though, and I think it would be a breaking change because it changes the rendering scheme.
|
Fixed and released |
|
thaankks 🙇♂️ updated and works as expected 💪 |
Description
I am not sure why but I currently have a situation where
marker.parentElementinexperimental-hydrate.tsis null.https://github.com/lit/lit/blob/main/packages/lit-html/src/experimental-hydrate.ts#L161-L164
when I check the code I see this
adding check so that
parentcan not be null seems to make it work.e.g.
Is that just a missing check? or is there a 0 chance that
parentElementis ever null? If so I need to check why it's null in my case...The text was updated successfully, but these errors were encountered: