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

Support for observers and computed properties #30

Closed
ernsheong opened this issue Apr 2, 2018 · 9 comments
Closed

Support for observers and computed properties #30

ernsheong opened this issue Apr 2, 2018 · 9 comments

Comments

@ernsheong
Copy link
Contributor

ernsheong commented Apr 2, 2018

Will there be support for observers and computed properties as described in https://www.polymer-project.org/2.0/docs/devguide/observers? (They do not seem to be currently supported)

@ernsheong ernsheong changed the title Support for observers and computer properties Support for observers and computed properties Apr 2, 2018
@ernsheong
Copy link
Contributor Author

ernsheong commented Apr 3, 2018

kenchris/lit-element#15 offers a way to implement own observers in a didRender() callback.

I suppose computedProperties can easily be done by doing ${this._computeAnotherField(field)} in own instance method.

@sorvell
Copy link
Member

sorvell commented May 1, 2018

We currently do not plan to add observers of computed properties. The idea is that you should generally use _render and if necessary _didRender.

Our experience from PolymerElement has led us to believe that providing a library feature ("observers") to route changes to specific functions is not a good return on investment v. letting users just write the necessary code.

The reason for omitting "computed properties" is similar. In general, you can compute necessary values for rendering directly in _render. If you have computed public properties, these should not provoke rendering (not be in properties) and can be set in _didRender.

@sorvell sorvell closed this as completed May 1, 2018
@madeleineostoja
Copy link

madeleineostoja commented May 1, 2018

Definitely agree that listing observers in property config in PolymerElement, rather than handling it functionally, was kinda clunky.

But for the use-case of reacting to properties that shouldn't cause a render, would it make sense to document the _propertiesChanged callback from Polymer's property mixin in LitElement (perhaps as part of #37)?

@valpackett
Copy link

For properties that don't cause a render directly, setters seem to be a good replacement. I'm currently porting an app to LitElement, the app uses app-route, so for an element that used to have an observer on the route passed into it, I now have

  set route (route) {
    fetch(`some/thing/${route.path}`).then(resp => {
      this.someDeclaredProperty = somethingBasedOn(resp)
    })
  }

And everything re-renders correctly after the fetch is done.

@mkozak0108
Copy link

It's okay to replace observers with setters but getters lack memorization from Polymer computed properties.
If you access getter property multiple times from render it will be calculated each time. Even if you cache getter value inside render function it is still calculated on each render and not when dependent properties has changed.

For now I'm using my own decorator Gist

But I think it would be great to have something similar from lit-element module

@computed('firstName', 'lastName')
get fullName() {
  return `${this.firstName} ${this.lastName}`.trim();
}

@daKmoR
Copy link
Contributor

daKmoR commented Apr 3, 2019

probably best to use update for it

e.g.

update(changedProperties) {
  super.update(changedProperties);
  if (changedProperties.has('firstName') || changedProperties.has('lastName') {
    this.fullName = `${this.firstName} ${this.lastName}`.trim();
  }
}
  • no need for an extra decorators
  • will only be updated once if any of the involved properties changed
  • access it does not involve a function call

@daKmoR
Copy link
Contributor

daKmoR commented Apr 7, 2019

actually, I was soooo wrong 🙈

update is already patched e.g. it's not sync...

I could not find a public api that allows for sync logging or computed properties.

Opened a new issue #643

@cristinecula
Copy link

cristinecula commented Sep 2, 2019

You can achieve something similar to Polymer with this mixin:

const PropertyComputing = (base) => class extends base {
  updated(changedProperties) {
    super.updated(changedProperties);
    Object.entries(this.constructor.properties)
      .filter(([key, value]) => typeof value.compute === 'string')
      .forEach(([key, value]) => {
        if(value.deps.some(k => changedProperties.has(k))) {
          this[key] = this[value.compute](...value.deps.map(dep => this[dep]))
        }
      })
  }
}

class MyElement extends PropertyComputing(LitElement) {
  static get properties() {
    return {
      prop1: { type: Number },
      prop2: { type: Number },
      comp: { type: Number, deps: ['prop1', 'prop2'], compute: '_computeComp'},      
      comp2: { type: Number, deps: ['prop1', 'comp'], compute: '_computeComp2'},
    };
  }

https://stackblitz.com/edit/sbcgaj?file=my-element.js

@nomego
Copy link

nomego commented Sep 12, 2019

As an extension to @cristinecula 's post:
So we also have fair amount of logic based on computed partial states in Polymer and decided to look into porting this behavior to Lit, in the form of https://github.com/Neovici/computing-lit-element

It's not feature complete (no support for wildcards/paths) but it did make a sample component very convenient to migrate: Neovici/cosmoz-treenode@v3.0.1...v4.0.1#diff-4dfb1d30ec99074e5e06658034dd2775

The implementation is very compact (https://github.com/Neovici/computing-lit-element/blob/master/src/computingMixin.js#L43) and works like this:
When the element gets constructed, we analyze the properties to find the computed ones, and also find all dependency properties (we have to expand any computed properties we depend on), this analysis is only performed once.
In the update() lifecycle method of a lit-element (https://lit-element.polymer-project.org/guide/lifecycle#update), changedProperties are available to analyze, and it invokes the rendering with lit-html.
Because of this, when we override this method, we call the super.update(); at the end, to make sure our computed properties are a part of the same render cycle.
The trickiest part is in the loop. As we update the computed properties, they themselves get added to the changedProperties array while we iterate through it, so we actually catch the whole dependency tree of computed properties during the loop itself, and everything gets updated in the same render.

This all can be conveniently packaged in a mixin so the lit-elements that needs this functionality can just, well, mix it in.

We are looking into doing the same for notify to make a nice (interim) compatibility layer between lit-element and Polymer.

Hope this is useful for people ending up here :)

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

8 participants