diff --git a/.changeset/modern-parrots-talk.md b/.changeset/modern-parrots-talk.md new file mode 100644 index 0000000000..5153a8c39e --- /dev/null +++ b/.changeset/modern-parrots-talk.md @@ -0,0 +1,5 @@ +--- +'lit-html': patch +--- + +Fix SSR hydration bug relating to and other void elements having attribute bindings. diff --git a/packages/labs/ssr/src/test/integration/tests/basic.ts b/packages/labs/ssr/src/test/integration/tests/basic.ts index 5f7d4c6d5e..ae758bdc22 100644 --- a/packages/labs/ssr/src/test/integration/tests/basic.ts +++ b/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 comments, become + // siblings instead of children. + registerElements() { + class VoidElementHost extends LitElement { + @property() + maxLen = 64; + + override render() { + return html``; + } + } + customElements.define('void-element-host', VoidElementHost); + }, + render() { + return html``; + }, + expectations: [ + { + args: [], + html: '', + 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 ******************************************************/ diff --git a/packages/labs/ssr/src/test/lib/render-lit_test.ts b/packages/labs/ssr/src/test/lib/render-lit_test.ts index 1b3a6690c4..ee640700ba 100644 --- a/packages/labs/ssr/src/test/lib/render-lit_test.ts +++ b/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, `
` ); }); +test('input element with attribute expression with string value', async () => { + const {render, inputTemplateWithAttributeExpression} = await setup(); + const result = await render(inputTemplateWithAttributeExpression('foo')); + assert.is( + result, + `` + ); +}); + +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 + // 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, + `

hi

` + ); +}); + test('multiple attribute expressions with string value', async () => { const {render, templateWithMultipleAttributeExpressions} = await setup(); const result = await render( diff --git a/packages/labs/ssr/src/test/test-files/render-test-module.ts b/packages/labs/ssr/src/test/test-files/render-test-module.ts index 61bf8e82f2..7d0fd3f107 100644 --- a/packages/labs/ssr/src/test/test-files/render-test-module.ts +++ b/packages/labs/ssr/src/test/test-files/render-test-module.ts @@ -35,6 +35,12 @@ export const templateWithMultiBindingAttributeExpression = ( x: string, y: string ) => html`
`; +// prettier-ignore +export const inputTemplateWithAttributeExpression = (x: string) => +html``; +// prettier-ignore +export const inputTemplateWithAttributeExpressionAndChildElement = (x: string) => + html`

hi

`; /* Reflected Property Expressions */ diff --git a/packages/lit-html/src/experimental-hydrate.ts b/packages/lit-html/src/experimental-hydrate.ts index 7b7ffaec67..11f6cf44ae 100644 --- a/packages/lit-html/src/experimental-hydrate.ts +++ b/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++]