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

Update to use Polymer's properties-mixin #2

Merged
merged 18 commits into from
Jan 17, 2018
Merged

Conversation

sorvell
Copy link
Member

@sorvell sorvell commented Jan 10, 2018

NOTE, do not merge yet. Depends on Polymer 3.x being released on npm.

Steven Orvell and others added 3 commits January 10, 2018 12:03
* _indvalidateProperties always triggers render.
* remove bespoke `html`
export class PolymerLitElement extends PropertiesMixin(HTMLElement) {

ready() {
this.attachShadow({mode: 'open'});
Copy link
Contributor

Choose a reason for hiding this comment

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

Given this line, I think we can add a shadowRoot: ShadowRoot as property on this class. This should remove the need for (el.shadowRoot as ShadowRoot) in every usage of this field.

Copy link
Contributor

Choose a reason for hiding this comment

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

HTMLElement already has a shadowRoot property. It's just possibly undefined, so it needs a trailing ! modifier.

export class PolymerLitElement extends PropertiesMixin(HTMLElement) {

ready() {
this.attachShadow({mode: 'open'});
Copy link
Contributor

Choose a reason for hiding this comment

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

HTMLElement already has a shadowRoot property. It's just possibly undefined, so it needs a trailing ! modifier.

// TODO(sorvell): propertiesChanged should have `_getData`
const result = this.render(this.__data);
if (result) {
render(result, this.shadowRoot as DocumentFragment);
Copy link
Contributor

Choose a reason for hiding this comment

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

render(result, this.shadowRoot!)

README.md Outdated
}

// Render method should return a `TemplateResult` using the provided lit-html `html` tag function
render(props, html) {
Copy link
Contributor

Choose a reason for hiding this comment

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

remove html

README.md Outdated
}

// Render method should return a `TemplateResult` using the provided lit-html `html` tag function
render(props, html) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than props as an argument, I'd use destructuring:

render({foo}) {
   return html`... ${foo}`;
}

super.ready();
}

render(props) {
Copy link
Contributor

Choose a reason for hiding this comment

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

use destructuring


get nextRendered() {
if (!this._nextRendered) {
this._nextRendered = new Promise((resolve) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Later I think it'll be good to reject in case there's an exception during rendering. ie:

_flushProperties() {
  super._flushProperties();
  try {
    const result = this.render(this.__data);
    if (result) {
      render(result, this.shadowRoot!);
    }
  } catch (e) {
    if (this._nextRenderedReject) {
      this._nextRenderedReject(e);
    }
  }

Steven Orvell added 3 commits January 16, 2018 16:21
* destructuring in render calls
* avoid returning `nextRendered` in `invalidate` for now.
@sorvell sorvell merged commit 75b73e3 into master Jan 17, 2018
@sorvell sorvell deleted the properties-mixin branch January 17, 2018 00:36
jbingham added a commit to jbingham/lit-element that referenced this pull request Jan 29, 2019
"Made to share" and "Interoperable" seemed redundant. Fast/light is most valuable to users. Standards seems like lit#2.
justinfagnani pushed a commit that referenced this pull request Jan 31, 2019
"Made to share" and "Interoperable" seemed redundant. Fast/light is most valuable to users. Standards seems like #2.
@hzmsrv hzmsrv mentioned this pull request Oct 26, 2019
bicknellr pushed a commit that referenced this pull request Jul 26, 2021
Add js-framework-benchmarks for lit-html
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants