Skip to content
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

Render any value returned by #render. #712

Merged
merged 12 commits into from
Jan 16, 2020
Merged
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

## Unreleased
<!-- ### Added -->
### Changed
* The value returned by `render` is always rendered, even if it isn't a `TemplateResult`. ([#712](https://github.com/Polymer/lit-element/issues/712)
<!-- ### Removed -->

### Added
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
},
"typings": "lit-element.d.ts",
"dependencies": {
"lit-html": "^1.0.0"
"lit-html": "^1.1.1"
},
"publishConfig": {
"access": "public"
Expand Down
40 changes: 20 additions & 20 deletions src/lit-element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@
* subject to an additional IP rights grant found at
* http://polymer.github.io/PATENTS.txt
*/
import {TemplateResult} from 'lit-html';
import {render} from 'lit-html/lib/shady-render.js';
import {render, ShadyRenderOptions} from 'lit-html/lib/shady-render.js';

import {PropertyValues, UpdatingElement} from './lib/updating-element.js';

Expand Down Expand Up @@ -45,15 +44,17 @@ export class LitElement extends UpdatingElement {
* optimizations. See updating-element.ts for more information.
*/
protected static['finalized'] = true;

/**
* Render method used to render the lit-html TemplateResult to the element's
* DOM.
* @param {TemplateResult} Template to render.
* @param {Element|DocumentFragment} Node into which to render.
* @param {String} Element name.
* Render method used to render the value to the element's DOM.
* @param result The value to render.
* @param container Node into which to render.
* @param options Element name.
* @nocollapse
*/
static render = render;
static render:
(result: unknown, container: Element|DocumentFragment,
options: ShadyRenderOptions) => void = render;

/**
* Array of styles to apply to the element. The styles should be defined
Expand Down Expand Up @@ -194,14 +195,11 @@ export class LitElement extends UpdatingElement {
*/
protected update(changedProperties: PropertyValues) {
super.update(changedProperties);
const templateResult = this.render() as unknown;
if (templateResult instanceof TemplateResult) {
bicknellr marked this conversation as resolved.
Show resolved Hide resolved
(this.constructor as typeof LitElement)
.render(
templateResult,
this.renderRoot,
{scopeName: this.localName, eventContext: this});
}
(this.constructor as typeof LitElement)
.render(
this.render(),
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 @@ -216,10 +214,12 @@ export class LitElement extends UpdatingElement {
}

/**
* Invoked on each update to perform rendering tasks. This method must return
* a lit-html TemplateResult. Setting properties inside this method will *not*
* trigger the element to update.
* Invoked on each update to perform rendering tasks. This method may return
* any value renderable by lit-html's NodePart - typically a TemplateResult.
* Setting properties inside this method will *not* trigger the element to
* update.
*/
protected render(): TemplateResult|void {
protected render(): unknown {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elide the return if you type this as unknown|void?

Copy link
Member Author

@bicknellr bicknellr Sep 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't seem like it:

src/lit-element.ts:241:23 - error TS2355: A function whose declared type is neither 'void' nor 'any' must return a value.

241   protected render(): unknown|void {
                          ~~~~~~~~~~~~

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a shame to add non-observable, non-type annotation code to satisfy the type-checker. Can we cast our way out of this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how I would cast the function in place since class syntax isn't expression based (and I assume we don't want to bind it / remove it from the prototype by making it a property), so I tried pulling it out of the class as this:

LitElement.prototype.render = function() {} as () => unknown;

But then I get another couple of errors that seem like TS is no longer recognizing that render is also on the prototype:

src/lit-element.ts:219:18 - error TS2576: Property 'render' is a static member of type 'LitElement'

219             this.render(),
                     ~~~~~~

src/lit-element.ts:243:22 - error TS2576: Property 'render' is a static member of type 'LitElement'

243 LitElement.prototype.render = function() {} as () => unknown;
                         ~~~~~~

I'm not familiar with pre-classes TS, so maybe that's not the right way to do it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(from offline) I'm going to leave the return undefined; as is and assume that any given minifier should be smart enough to remove it.

return undefined;
}
}
56 changes: 56 additions & 0 deletions src/test/lit-element_styling_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -798,6 +798,62 @@ suite('Static get styles', () => {
const bodyStyles = `${cssModule}`;
assert.equal(bodyStyles, '.my-module { color: yellow; }');
});

test(
'Styles are not removed if the first rendered value is undefined.',
async () => {
const localName = generateElementName();

class SomeCustomElement extends LitElement {
static styles = css`:host {border: 4px solid black;}`;

renderUndefined: boolean;

constructor() {
super();
this.renderUndefined = true;
}

static get properties() {
return {
renderUndefined: {
type: Boolean,
value: true,
},
};
}

render() {
if (this.renderUndefined) {
return undefined;
}

return htmlWithStyles`Some text.`;
}
}
customElements.define(localName, SomeCustomElement);

const element = document.createElement(localName) as SomeCustomElement;
document.body.appendChild(element);

await (element as LitElement).updateComplete;
assert.equal(
getComputedStyle(element)
.getPropertyValue('border-top-width')
.trim(),
'4px');

element.renderUndefined = false;

await (element as LitElement).updateComplete;
assert.equal(
getComputedStyle(element)
.getPropertyValue('border-top-width')
.trim(),
'4px');

document.body.removeChild(element);
});
});

suite('ShadyDOM', () => {
Expand Down