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

Possible new lifecycle hook #9

Open
filipbech opened this issue Oct 28, 2017 · 11 comments
Open

Possible new lifecycle hook #9

filipbech opened this issue Oct 28, 2017 · 11 comments

Comments

@filipbech
Copy link
Contributor

Should we add a lifecycle hook for when properties have data - before the first render, so we can do some stuff.

If I want to set a property based on another property, I have to do some not-so-nice settimeout-0 stuff to make it work...

I put it in my demo to show the usecase:
https://github.com/filipbech/lit-element-example/blob/master/demo.js#L69-L71

What do you think? wouldn't this be a nice handle, or am I missing something obvious?

@kenchris
Copy link
Owner

The following works :-)

    async connectedCallback() {
        await super.connectedCallback();
        this.favorite = this.product.isfavorite;
    }

But I am wondering whether we should just render immediately when connected. What is your opinion @justinfagnani ?

@filipbech
Copy link
Contributor Author

yeah, I saw you did the same thing on invalidate... or maybe the connectedCallback could actually return a promise that resolves? this seems kind of "hacky" and async/await compiles down to crap (huge pile actually) for IE11 support...

@kenchris
Copy link
Owner

kenchris commented Oct 28, 2017

That is exactly what it does today. It returns a promise which resolves :-)

You could also do

connectedCallback() {
  super.connectedCallback().then(_ => {
    this.favorite = this.product.isfavorite;
  });

But as I said, we could probably just let connectedCallback call render directly and not as a microtask.

@filipbech
Copy link
Contributor Author

super.connectedCallback currently returns undefined... do you have something locally or on another branch?
https://github.com/kenchris/lit-element/blob/master/src/lit-element.ts#L87 - no return

@kenchris
Copy link
Owner

Ah it uses to return the result of this.invalidate(). We should fix that.

@justinfagnani
Copy link

justinfagnani commented Oct 28, 2017

I personally wouldn't return anything from connectedCallback() that's changing the signature, and subclasses might not forward the return value...

edit: "wouldn't" not "would" :)

@filipbech
Copy link
Contributor Author

that is a valid point... what would you suggest? lifecycle hook or something completely different?

@kenchris
Copy link
Owner

@justinfagnani so any downsides to forcing a render in connectedCallback? (ie. no microtask)

@justinfagnani
Copy link

If I want to set a property based on another property

There's a few ways to do this:

  1. Define a getter for the dependent property, if it's cheap to compute. Then it's always up to date and takes up no additional storage.
  2. Manually implement the setter for the source property, and set the dependant property in it. The base class needs to be able to handle this case.
  3. Implement an observer feature for the base class, where observers are called before render()
  4. Have invalidate call renderCallback() which calls render() in the base class. The subclass can override renderCallback() to do work before or after the base class calls render(), including setting the dependent property before render().

I think 1) is best if possible. In this case I think it highlights a data-dependency questions that seems currently unanswered in your example: Should setting element.favorite also set element.product.favorite? If it's truly just a derived property, then no. If you're trying to sync the two, then maybe yes (though I find such APIs tricky to reason about).

export class ProductItemElement extends LitElement {
  static get properties() {
    return {
      product: {
        type: Object
      },
    };
  }

  get favorite() { return this.product && this.product.isFavorite; }
}

@justinfagnani
Copy link

justinfagnani commented Oct 28, 2017

@kenchris

any downsides to forcing a render in connectedCallback? (ie. no microtask)

Mainly that all the data from above might not be there yet.

Usually, just having property initializers is enough to kick off rendering. For the rare element that doesn't initialize any properties, calling invalidate() in the constructor works. Calling invalidate() in connectedCallback would be useful if the template depends on tree-state, like this.isConnected, this.parentNode, etc.

@kenchris
Copy link
Owner

I was thinking for cases where you might want to do

(Typing on phone)

this.canvas = this.$('myCanvas'); which requires the Dom to exist

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

No branches or pull requests

3 participants