[ssr] Fix hydration bug with attribute bindings on void elements (#2952)

### 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.

.changeset/modern-parrots-talk.md

@@ -0,0 +1,5 @@
+---
+'lit-html': patch
+---
+
+Fix SSR hydration bug relating to <input> and other void elements having attribute bindings.

packages/labs/ssr/src/test/integration/tests/basic.ts

@@ -1580,6 +1580,54 @@ export const tests: {[name: string]: SSRTest} = {
     stableSelectors: ['input'],
   },
 
+  'AttributePart on void element in shadow root': {
+    // Regression test for https://github.com/lit/lit/issues/2946.
+    //
+    // Confirms that we do not crash when hydrating a shadow root containing an
+    // immediate child that is a void element with an attribute binding. This is
+    // an edge case because when the HTML parser encounters a void element, any
+    // children it has, including our <!--lit-node 0--> comments, become
+    // siblings instead of children.
+    registerElements() {
+      class VoidElementHost extends LitElement {
+        @property()
+        maxLen = 64;
+
+        override render() {
+          return html`<input max=${this.maxLen} />`;
+        }
+      }
+      customElements.define('void-element-host', VoidElementHost);
+    },
+    render() {
+      return html`<void-element-host></void-element-host>`;
+    },
+    expectations: [
+      {
+        args: [],
+        html: '<void-element-host></void-element-host>',
+        async check(assert: Chai.Assert, dom: HTMLElement) {
+          const host = dom.querySelector('void-element-host') as LitElement & {
+            maxLen: number;
+          };
+          assert.instanceOf(host, LitElement);
+          assert.equal(host.maxLen, 64);
+
+          await host.updateComplete;
+          // eslint-disable-next-line @typescript-eslint/no-non-null-asserted-optional-chain
+          const input = host.shadowRoot?.querySelector('input')!;
+          assert.instanceOf(input, HTMLElement);
+          assert.equal(input.getAttribute('max'), '64');
+
+          host.maxLen++;
+          await host.updateComplete;
+          assert.equal(input.getAttribute('max'), '65');
+        },
+      },
+    ],
+    stableSelectors: ['input'],
+  },
+
   /******************************************************
    * PropertyPart tests
    ******************************************************/

packages/labs/ssr/src/test/lib/render-lit_test.ts

@@ -111,13 +111,38 @@ test('text expression with null value', async () => {
 test('attribute expression with string value', async () => {
   const {render, templateWithAttributeExpression} = await setup();
   const result = await render(templateWithAttributeExpression('foo'));
-  // TODO: test for the marker comment for attribute binding
   assert.is(
     result,
     `<!--lit-part FAR9hgjJqTI=--><div class="foo"><!--lit-node 0--></div><!--/lit-part-->`
   );
 });
 
+test('input element with attribute expression with string value', async () => {
+  const {render, inputTemplateWithAttributeExpression} = await setup();
+  const result = await render(inputTemplateWithAttributeExpression('foo'));
+  assert.is(
+    result,
+    `<!--lit-part AYwG7rAvcnw=--><input x="foo"><!--lit-node 0--><!--/lit-part-->`
+  );
+});
+
+test('input element with attribute expression with string value and child element', async () => {
+  // Void elements like input will have their children hoisted to become
+  // siblings by the HTML parser. In this case, we rely on the fact that any
+  // <!--lit-node 0--> comments we create are prepended instead of appended,
+  // so that they will be hoisted as the next sibling, so that we can use
+  // .previousElementSibling to find the effective parent.
+  const {render, inputTemplateWithAttributeExpressionAndChildElement} =
+    await setup();
+  const result = await render(
+    inputTemplateWithAttributeExpressionAndChildElement('foo')
+  );
+  assert.is(
+    result,
+    `<!--lit-part BIugdiAuV4I=--><input x="foo"><!--lit-node 0--><p>hi</p></input><!--/lit-part-->`
+  );
+});
+
 test('multiple attribute expressions with string value', async () => {
   const {render, templateWithMultipleAttributeExpressions} = await setup();
   const result = await render(

packages/labs/ssr/src/test/test-files/render-test-module.ts

@@ -35,6 +35,12 @@ export const templateWithMultiBindingAttributeExpression = (
   x: string,
   y: string
 ) => html`<div test="a ${x} b ${y} c"></div>`;
+// prettier-ignore
+export const inputTemplateWithAttributeExpression = (x: string) =>
+html`<input x=${x}>`;
+// prettier-ignore
+export const inputTemplateWithAttributeExpressionAndChildElement = (x: string) =>
+  html`<input x=${x}><p>hi</p></input>`;
 
 /* Reflected Property Expressions */
 

packages/lit-html/src/experimental-hydrate.ts

@@ -157,11 +157,6 @@ export const hydrate = (
       // Create and hydrate attribute parts into the current ChildPart on the
       // stack
       createAttributeParts(marker, stack, options);
-      // Remove `defer-hydration` attribute, if any
-      const parent = marker.parentElement!;
-      if (parent.hasAttribute('defer-hydration')) {
-        parent.removeAttribute('defer-hydration');
-      }
     } else if (markerText.startsWith('/lit-part')) {
       // Close the current ChildPart, and pop the previous one off the stack
       if (stack.length === 1 && currentChildPart !== rootPart) {
@@ -337,7 +332,12 @@ const createAttributeParts = (
   // the previousSibling; for non-void elements, the comment is guaranteed
   // to be the first child of the element (i.e. it won't have a previousSibling
   // meaning it should use the parentElement)
-  const node = comment.previousSibling ?? comment.parentElement;
+  const node = comment.previousElementSibling ?? comment.parentElement;
+  if (node === null) {
+    throw new Error('could not find node for attribute parts');
+  }
+  // Remove `defer-hydration` attribute, if any
+  node.removeAttribute('defer-hydration');
 
   const state = stack[stack.length - 1];
   if (state.type === 'template-instance') {
@@ -391,11 +391,7 @@ const createAttributeParts = (
         instance._parts.push(instancePart);
       } else {
         // templatePart.type === PartType.ELEMENT
-        const instancePart = new ElementPart(
-          node as HTMLElement,
-          state.instance,
-          options
-        );
+        const instancePart = new ElementPart(node, state.instance, options);
         resolveDirective(
           instancePart,
           state.result.values[state.instancePartIndex++]