adobe / lit-mobx

Mixin and base class for using mobx with lit-element
Apache License 2.0
269 stars 16 forks source link

updated not called if observable not used in render #149

Closed madeleineostoja closed 2 years ago

madeleineostoja commented 2 years ago

Expected Behaviour

Observables should trigger updated() when extending MobxLitElement

Actual Behaviour

Observables only trigger an update if used within the render callback

Platform and Version

Sample Code that illustrates the problem

This is not reactive to observable/store updates

export class Element extends MobxLitElement {
   private store = store;

  updated() {
    console.log(this.store)
  }
}

This is reactive

export class Element extends MobxLitElement {
   private store = store;

  render() {
    html`<span>${this.store}</span>`
  }
}
benjamind commented 2 years ago

What purpose would triggering an update (render) be useful for if that property is not used in the render function?

It's relatively trivial to use autorun or other mobx observable methods to drive a computation that is stored on a property of the component that then drives the render update naturally like other property updated callbacks.

madeleineostoja commented 2 years ago

Other than being an opinionated and undocumented footgun (it took me SO long to debug why my lit-mobx integration wasn't working), I don't think it's beyond imagination to use pure logic/state based observables whose output aren't directly rendered but instead change what is rendered through local state/props. Like layout settings, global state, etc.

benjamind commented 2 years ago

In those cases you should just be using mobx directly. There's no need to automatically wrap every property into an observable context since 99% of them wouldn't be mobx observable.

madeleineostoja commented 2 years ago

Sure but it should still be documented if nothing else

benjamind commented 2 years ago

Added a note to the README.