Skip to content

Commit

Permalink
Fix ChildPart parentNode for top-level parts (#2098)
Browse files Browse the repository at this point in the history
  • Loading branch information
kevinpschaaf committed Aug 27, 2021
1 parent 0d703bf commit b3121ab
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 14 deletions.
5 changes: 5 additions & 0 deletions .changeset/bright-keys-kick.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'lit-html': patch
---

Fix ChildPart parentNode for top-level parts to return the parentNode they _will_ be inserted into, rather than the DocumentFragment they were cloned into. Fixes #2032.
17 changes: 15 additions & 2 deletions packages/lit-html/src/lit-html.ts
Original file line number Diff line number Diff line change
Expand Up @@ -853,7 +853,7 @@ class TemplateInstance implements Disconnectable {
_parts: Array<Part | undefined> = [];

/** @internal */
_$parent: Disconnectable;
_$parent: ChildPart;
/** @internal */
_$disconnectableChildren?: Set<Disconnectable> = undefined;

Expand All @@ -862,6 +862,11 @@ class TemplateInstance implements Disconnectable {
this._$parent = parent;
}

// Called by ChildPart parentNode getter
get parentNode() {
return this._$parent.parentNode;
}

// See comment in Disconnectable interface for why this is a getter
get _$isConnected() {
return this._$parent._$isConnected;
Expand Down Expand Up @@ -1061,7 +1066,15 @@ class ChildPart implements Disconnectable {
* consists of all child nodes of `.parentNode`.
*/
get parentNode(): Node {
return wrap(this._$startNode).parentNode!;
let parentNode: Node = wrap(this._$startNode).parentNode!;
const parent = this._$parent;
if (parent !== undefined && parentNode.nodeType === 11 /* Node.DOCUMENT_FRAGMENT */) {
// If the parentNode is a DocumentFragment, it may be because the DOM is
// still in the cloned fragment during initial render; if so, get the real
// parentNode the part will be committed into by asking the parent.
parentNode = (parent as ChildPart | TemplateInstance).parentNode;
}
return parentNode;
}

/**
Expand Down
73 changes: 61 additions & 12 deletions packages/lit-html/src/test/lit-html_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,13 @@ import {
} from '../lit-html.js';
import * as litHtmlLib from '../lit-html.js';

import {directive, Directive, PartType, PartInfo} from '../directive.js';
import {
directive,
Directive,
PartType,
PartInfo,
DirectiveParameters,
} from '../directive.js';
import {assert} from '@esm-bundle/chai';
import {
stripExpressionComments,
Expand All @@ -33,6 +39,7 @@ import {createRef, ref} from '../directives/ref.js';

// For compiled template tests
import {_$LH} from '../private-ssr-support.js';
import {until} from '../directives/until.js';
const {AttributePart} = _$LH;

type AttributePart = InstanceType<typeof AttributePart>;
Expand Down Expand Up @@ -67,6 +74,7 @@ suite('lit-html', () => {

setup(() => {
container = document.createElement('div');
container.id = 'container';
});

const assertRender = (
Expand Down Expand Up @@ -1667,11 +1675,14 @@ suite('lit-html', () => {

suite('ChildPart invariants for parentNode, startNode, endNode', () => {
class CheckNodePropertiesBehavior extends Directive {
render() {
render(_parentId?: string) {
return nothing;
}

override update(part: ChildPart) {
override update(
part: ChildPart,
[parentId]: DirectiveParameters<this>
) {
const {parentNode, startNode, endNode} = part;

if (endNode !== null) {
Expand All @@ -1690,37 +1701,75 @@ suite('lit-html', () => {
assert.equal<Node | null>(startNode.nextSibling, endNode);
}

if (parentId !== undefined) {
assert.equal((parentNode as HTMLElement).id, parentId);
}

return nothing;
}
}
const checkNodePropertiesBehavior = directive(
CheckNodePropertiesBehavior
);
const checkPart = directive(CheckNodePropertiesBehavior);

test('when the directive is the only child', () => {
const makeTemplate = (content: unknown) => html`<div>${content}</div>`;

// Render twice so that `update` is called.
render(makeTemplate(checkNodePropertiesBehavior()), container);
render(makeTemplate(checkNodePropertiesBehavior()), container);
render(makeTemplate(checkPart()), container);
render(makeTemplate(checkPart()), container);
});

test('when the directive is the last child', () => {
const makeTemplate = (content: unknown) =>
html`<div>Earlier sibling. ${content}</div>`;

// Render twice so that `update` is called.
render(makeTemplate(checkNodePropertiesBehavior()), container);
render(makeTemplate(checkNodePropertiesBehavior()), container);
render(makeTemplate(checkPart()), container);
render(makeTemplate(checkPart()), container);
});

test('when the directive is not the last child', () => {
const makeTemplate = (content: unknown) =>
html`<div>Earlier sibling. ${content} Later sibling.</div>`;

// Render twice so that `update` is called.
render(makeTemplate(checkNodePropertiesBehavior()), container);
render(makeTemplate(checkNodePropertiesBehavior()), container);
render(makeTemplate(checkPart()), container);
render(makeTemplate(checkPart()), container);
});

test(`part's parentNode is the logical DOM parent`, async () => {
const asyncCheckDiv = Promise.resolve(checkPart('divPromise'));
const makeTemplate = () =>
html`
${checkPart('container')}
<div id="div">
${checkPart('div')}
${html`x ${checkPart('div')} x`}
${html`x ${html`x ${checkPart('div')} x`} x`}
${html`x ${html`x ${[checkPart('div'), checkPart('div')]} x`} x`}
${html`x ${html`x ${[
[checkPart('div'), checkPart('div')],
]} x`} x`}
${html`x ${html`x ${[
[repeat([checkPart('div'), checkPart('div')], (v) => v)],
]} x`} x`}
${until(asyncCheckDiv)}
</div>
`;

// Render twice so that `update` is called.
render(makeTemplate(), container);
await asyncCheckDiv;
render(makeTemplate(), container);
});

test(`part's parentNode is correct when rendered into a document fragment`, async () => {
const fragment = document.createDocumentFragment();
(fragment as unknown as {id: string}).id = 'fragment';
const makeTemplate = () => html`${checkPart('fragment')}`;

// Render twice so that `update` is called.
render(makeTemplate(), fragment);
render(makeTemplate(), fragment);
});
});

Expand Down

0 comments on commit b3121ab

Please sign in to comment.