Skip to content

Commit

Permalink
[lit-html, lit-element] Add isConnected to RenderOptions. Fixes #2051 (
Browse files Browse the repository at this point in the history
…#2056)

* Add initialIsConnected to RenderOptions. Fixes #2051

* Fix typo. Add changeset.

* Rename initialIsConnected -> isConnected and beef up comments re: options being one-time

* Set isConnected once based on element's native isConnected

* Update API docs
  • Loading branch information
kevinpschaaf committed Aug 27, 2021
1 parent 7adfbb0 commit e5667d6
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 11 deletions.
6 changes: 6 additions & 0 deletions .changeset/thick-snails-repair.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'lit-element': patch
'lit-html': patch
---

Fixed issue where `AsyncDirective`s could see `this.isConnected === true` if a LitElement performed its initial render while it was disconnected.
3 changes: 3 additions & 0 deletions packages/lit-element/src/lit-element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,9 @@ export class LitElement extends ReactiveElement {
// updates are allowed after super.update, it's important to call `render`
// before that.
const value = this.render();
if (!this.hasUpdated) {
this.renderOptions.isConnected = this.isConnected;
}
super.update(changedProperties);
this.__childPart = render(value, this.renderRoot, this.renderOptions);
}
Expand Down
26 changes: 23 additions & 3 deletions packages/lit-element/src/test/lit-element_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -515,18 +515,38 @@ import {createRef, ref} from 'lit-html/directives/ref.js';
]);
});

// TODO(kschaaf): render does not currently support rendering to an
// initially disconnected ChildPart (https://github.com/lit/lit/issues/2051)
test.skip('directives render with isConnected: false if first render is while element is disconnected', async () => {
test('directives render with isConnected: false if first render is while element is disconnected', async () => {
container.appendChild(host);
container.remove();
await nextFrame();
assertRendering(host);
// Host directives render in an initially disconnected state.
// Note that child directives didn't render because by the time the
// host render happened, the child was not connected and is still
// pending
assert.deepEqual(log, [
'render-host-attr-false',
'render-host-prop-false',
'render-host-node-false',
]);
log.length = 0;
document.body.appendChild(container);
assertRendering(host);
// Directive reconnection happens synchronous to connectedCallback
assert.deepEqual(log, [
'reconnect-host-attr',
'reconnect-host-prop',
'reconnect-host-node',
]);
log.length = 0;
// The initial render of the child happens a microtask after the host
// reconnects, at which point its directives run in the connected state
await nextFrame();
assert.deepEqual(log, [
'render-child-attr-true',
'render-child-prop-true',
'render-child-node-true',
]);
});
});
});
Expand Down
39 changes: 31 additions & 8 deletions packages/lit-html/src/lit-html.ts
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,14 @@ export const nothing = Symbol.for('lit-nothing');
*/
const templateCache = new WeakMap<TemplateStringsArray, Template>();

/**
* Object specifying options for controlling lit-html rendering. Note that
* while `render` may be called multiple times on the same `container` (and
* `renderBefore` reference node) to efficiently update the rendered content,
* only the options passed in during the first render are respected during
* the lifetime of renders to that unique `container` + `renderBefore`
* combination.
*/
export interface RenderOptions {
/**
* An object to use as the `this` value for event listeners. It's often
Expand All @@ -337,6 +345,15 @@ export interface RenderOptions {
* any inherited context. Defaults to the global `document`.
*/
creationScope?: {importNode(node: Node, deep?: boolean): Node};
/**
* The initial connected state for the top-level part being rendered. If no
* `isConnected` option is set, `AsyncDirective`s will be connected by
* default. Set to `false` if the initial render occurs in a disconnected tree
* and `AsyncDirective`s should see `isConnected === false` for their initial
* render. The `part.setConnected()` method must be used subsequent to initial
* render to change the connected state of the part.
*/
isConnected?: boolean;
}

/**
Expand Down Expand Up @@ -952,13 +969,15 @@ class ChildPart implements Disconnectable {
private _textSanitizer: ValueSanitizer | undefined;
/** @internal */
_$parent: Disconnectable | undefined;
// TODO(kschaaf): There's currently no way to have the initial render
// of a part be `isConnected: false`. We may want to add this via renderOptions
// so that if a LitElement ends up performing its initial render while
// disconnected, the directives aren't in the wrong state
// https://github.com/lit/lit/issues/2051
/** @internal */
__isConnected = true;
/**
* Connection state for RootParts only (i.e. ChildPart without _$parent
* returned from top-level `render`). This field is unsed otherwise. The
* intention would clearer if we made `RootPart` a subclass of `ChildPart`
* with this field (and a different _$isConnected getter), but the subclass
* caused a perf regression, possibly due to making call sites polymorphic.
* @internal
*/
__isConnected: boolean;

// See comment in Disconnectable interface for why this is a getter
get _$isConnected() {
Expand Down Expand Up @@ -991,6 +1010,10 @@ class ChildPart implements Disconnectable {
this._$endNode = endNode;
this._$parent = parent;
this.options = options;
// Note __isConnected is only ever accessed on RootParts (i.e. when there is
// no _$parent); the value on a non-root-part is "don't care", but checking
// for parent would be more code
this.__isConnected = options?.isConnected ?? true;
if (ENABLE_EXTRA_SECURITY_HOOKS) {
// Explicitly initialize for consistent class shape.
this._textSanitizer = undefined;
Expand Down Expand Up @@ -1279,7 +1302,7 @@ export interface RootPart extends ChildPart {
* as such, it is the responsibility of the caller to `render` to ensure that
* `part.setConnected(false)` is called before the part object is potentially
* discarded, to ensure that `AsyncDirective`s have a chance to dispose of
* any resources being held. If a RootPart that was prevously
* any resources being held. If a `RootPart` that was prevously
* disconnected is subsequently re-connected (and its `AsyncDirective`s should
* re-connect), `setConnected(true)` should be called.
*
Expand Down

0 comments on commit e5667d6

Please sign in to comment.