Skip to content

Only call lit-html render if LitElement subclass implements render #917

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
`LitElement` renders when updates are triggered as a result of rendering ([#549](https://github.com/Polymer/lit-element/issues/549)).
* Properties annotated with the `eventOptions` decorator will now survive property renaming optimizations when used with tsickle and Closure JS Compiler.
* Moved style gathering from `finalize` to `initialize` to be more lazy, and create stylesheets on the first instance initializing [#866](https://github.com/Polymer/lit-element/pull/866).
* Fixed behavior change for components that do not implement `render()` introduced in ([#712](https://github.com/Polymer/lit-element/pull/712)) ([#917](https://github.com/Polymer/lit-element/pull/917))

## [2.2.1] - 2019-07-23
### Changed
Expand Down
21 changes: 15 additions & 6 deletions src/lit-element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ declare global {

export interface CSSResultArray extends Array<CSSResult|CSSResultArray> {}

/**
* Sentinal value used to avoid calling lit-html's render function when
* subclasses do not implement `render`
*/
const renderNotImplemented = {};

export class LitElement extends UpdatingElement {
/**
* Ensure this class is marked as `finalized` as an optimization ensuring
Expand Down Expand Up @@ -204,11 +210,14 @@ export class LitElement extends UpdatingElement {
// before that.
const templateResult = this.render();
super.update(changedProperties);
(this.constructor as typeof LitElement)
.render(
templateResult,
this.renderRoot,
{scopeName: this.localName, eventContext: this});
// If render is not implemented by the component, don't call lit-html render
if (templateResult !== renderNotImplemented) {
(this.constructor as typeof LitElement)
.render(
templateResult,
this.renderRoot,
{scopeName: this.localName, eventContext: this});
}
// When native Shadow DOM is used but adoptedStyles are not supported,
// insert styling after rendering to ensure adoptedStyles have highest
// priority.
Expand All @@ -229,6 +238,6 @@ export class LitElement extends UpdatingElement {
* update.
*/
protected render(): unknown {
return undefined;
return renderNotImplemented;
}
}
79 changes: 51 additions & 28 deletions src/test/lit-element_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -195,39 +195,42 @@ suite('LitElement', () => {
assert.equal(window['litElementVersions'].length, 1);
});

test('event fired during rendering element can trigger an update', async () => {
class E extends LitElement {
connectedCallback() {
super.connectedCallback();
this.dispatchEvent(new CustomEvent('foo', {bubbles: true, detail: 'foo'}));
}
}
customElements.define('x-child-61012', E);

class F extends LitElement {

static get properties() {
return {foo: {type: String}};
}
test(
'event fired during rendering element can trigger an update',
async () => {
class E extends LitElement {
connectedCallback() {
super.connectedCallback();
this.dispatchEvent(
new CustomEvent('foo', {bubbles: true, detail: 'foo'}));
}
}
customElements.define('x-child-61012', E);

foo = '';
class F extends LitElement {
static get properties() {
return {foo: {type: String}};
}

render() {
return html`<x-child-61012 @foo=${this._handleFoo}></x-child-61012><span>${this.foo}</span>`;
}
foo = '';

_handleFoo(e: CustomEvent) {
this.foo = e.detail;
}
render() {
return html`<x-child-61012 @foo=${
this._handleFoo}></x-child-61012><span>${this.foo}</span>`;
}

}
_handleFoo(e: CustomEvent) {
this.foo = e.detail;
}
}

customElements.define(generateElementName(), F);
const el = new F();
container.appendChild(el);
while (!(await el.updateComplete)) {}
assert.equal(el.shadowRoot!.textContent, 'foo');
});
customElements.define(generateElementName(), F);
const el = new F();
container.appendChild(el);
while (!(await el.updateComplete)) {
}
assert.equal(el.shadowRoot!.textContent, 'foo');
});

test(
'exceptions in `render` throw but do not prevent further updates',
Expand Down Expand Up @@ -266,4 +269,24 @@ suite('LitElement', () => {
assert.equal(a.foo, 20);
assert.equal(a.shadowRoot!.textContent, '20');
});

test(
'if `render` is unimplemented, do not overwrite renderRoot', async () => {
class A extends LitElement {
addedDom: HTMLElement|null = null;
createRenderRoot() {
return this;
}
}
customElements.define(generateElementName(), A);
const a = new A();
const testDom = document.createElement('div');
a.appendChild(testDom);
container.appendChild(a);
await a.updateComplete;
assert.equal(
testDom.parentNode,
a,
'testDom should be a child of the component');
});
});