As a developer I would like to do sync actions on property changes

[daKmoR]

Description

There is no public API that allows for sync actions like logging or "computed" properties.

Example use case:
Logging We like cats too :) whenever the property mood gets set to cat.

Live Demo

https://stackblitz.com/edit/lit-element-example-nkwdco?file=index.js

Expected Results

=> set to cat
We like cats too :)
=> set to foo
=> set to cat
We like cats too :)

Actual Results (when using public API update)

=> set to cat
=> set to foo
=> set to cat
We like cats too :)

Code for reference:

  update(changedProperties) {
    super.update(changedProperties);
    if (changedProperties.has('mood')) {
      if (this.mood === 'cat') {
        console.log('We like cats too :)');
      }
    }
  }

Note

Workaround with _requestUpdate works but it's a private API.

Code for reference:

  _requestUpdate(name, oldValue) {
    super._requestUpdate(name, oldValue);
    if (name === 'mood') {
      if (this.mood === 'cat') {
        console.log('We like cats too :)');
      }
    }
  }

Versions

• lit-element: v2.1.0

[daKmoR]

another usage example

this.firstName = 'foo';
// fullName should be updated whenever firstName or lastName changes

if (this.fullName.contains('foo')) {
  // do something 
  // never reached as fullName will only be updated in the next microtask if using `update`
}

[CaptainCodeman]

You could use the hasChanged method:

  static get properties() {
    return { 
      mood: {type: String, hasChanged: (newVal, oldVal) => {
        if (newVal === 'cat') {
          console.log('We like cats too :)');
        }
        return true
      }}
    }
  }

[daKmoR]

hmmm using hasChanged probably could work but that certainly looks like abuse 🙈
also when extending web components it's tough as you can't easily add another function in your child...

[blikblum]

using hasChanged probably could work but that certainly looks like abuse

and you don't have access to component instance (this). So anything that needs to access the instance won't work

[Westbrook]

@daKmoR On the side of logging, to better inform the outcomes you hope to see in this request, can you explain how you see this as being different than a custom getter/setter?

get mood() { return this._mood; }
set mood(mood) {
  if (mood === this.mood) return;
  if (mood === 'cat') console.log('We like cats too :)');
  const old = this._mood;
  this._mood = mood;
  this.requestUpdate('mood', old);
}

In the context of your computed properties, it would seem that a getter would also provide much of what could be needed here:

get fullName() {
  return `${this.firstName} ${this.lastName}`;
}
...
this.firstName = 'foo';
// fullName should be updated whenever firstName or lastName changes

if (this.fullName.contains('foo')) {
  // do something
}

In the above case the // do something would be hit because the properties update of firstName is available synchronously. It's certainly not "reactive" in a way that would allow logging, but helping to clarify the the outcomes you hope to see in this request.

Maybe it's just boilerplate, or convention vs configuration, but whether those or something else I look forward to your thoughts here.

[mattlohkamp]

does 'updated' fire too late for your purposes, @daKmoR ?

updated(props){
  if(props.has('mood'){  //  mood has changed
    if(this.mood === 'cats'){  //  mood has changed to 'cats'
      console.log('we like cats too :)')    
  }
}

[daKmoR]

actually, this whole thing started while I was preparing a blog post about testing - and I wanted to have a simple example of how you can do an action on a property change. In my ignorance, I thought just log something that should be easy... oh boy was I wrong 🙈

That was the actual test I started out with.

it('logs "We like cats too :)" if the value is set to "cat"', async () => {
  const el = /** @type {A11yInput} */ (await fixture(html`
    <a11y-input></a11y-input>
  `));
  const logSpy = sinon.spy(el, 'log');

  el.value = 'cat';
  expect(logSpy.callCount).to.equal(1);
});

So primarily it's not about logging - it was just an example. Depending on which type of logging you wanna do updated could certainly be good enough.

Why not a custom getter/setter?

I would still like to use all the features of a property.

• reads inital attribute
• reflect it to attribute
• can react to attribute changes
• show up in a properties list (you can use to auto generate playgrounds)
• ...

this of course works but I would need to redo all the attributes logic

Why not just a getter?

• It should support arbitrary actions that do not need accessing the property
• The computation of this "combined" value could be heavy (e.g. it should be performant to access it many many times)

Conclusion

As far as I can tell these are the hard requirements:

• It needs to be synchronous
• It needs to support arbitrary actions
• It needs to allow adding of more actions when extending

You can read the really long full story here - the link goes tot he relevant part where I discover the "issue": https://dev.to/open-wc/testing-workflow-for-web-components-g73#spying-on-code

[sorvell]

It seems like making _requestUpdate something protected like setProperty would unlock this? That's probably a reasonable change.

[blikblum]

It seems like making _requestUpdate something protected like setProperty would unlock this?

AFAIK, yes

[daKmoR]

It seems like making _requestUpdate something protected like setProperty would unlock this? That's probably a reasonable change.

what do you mean? rename _requestUpdate to setProperty?
or just call setProperty inside _requestUpdate like so

_requestUpdate(name, oldValue) {
  // _requestUpdate code then at the end
  this.setProperty(name, oldValue);
}

if yes then I (or someone) could prepare a PR for it 💪

[daKmoR]

@sorvell if you could give a hint on how it should look like I'm sure I could prepare a PR 🤗

[sorvell]

Sorry, I was thinking just renaming `_requestUpdate` to `setProperty`. I can't say for sure if this makes sense as I haven't really thought about it much more, but we can iterate on the PR.

…

On Sat, Apr 27, 2019 at 6:39 AM Thomas Allmer ***@***.***> wrote: @sorvell <https://github.com/sorvell> if you could give a hint on how it should look like I'm sure I could prepare a PR 🤗 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#643 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAATIK6IAJYLIOQK4JQGQX3PSRJPRANCNFSM4HECYBNA> .

[blikblum]

I was thinking just renaming _requestUpdate to setProperty

This covers my use case (i was overriding _requestUpdate)

[jolleekin]

The best place to do this is in _requestUpdate.

  /**
   * Called synchronously when a property has changed.
   */
  protected propertyChanged(name: PropertyKey, oldValue?: unknown): void {}

  private _requestUpdate(name?: PropertyKey, oldValue?: unknown) {
    let shouldRequestUpdate = true;
    // If we have a property key, perform property update steps.
    if (name !== undefined) {
      const ctor = this.constructor as typeof UpdatingElement;
      const options =
          ctor._classProperties!.get(name) || defaultPropertyDeclaration;
      if (ctor._valueHasChanged(
              this[name as keyof this], oldValue, options.hasChanged)) {
        if (!this._changedProperties.has(name)) {
          this._changedProperties.set(name, oldValue);
        }


        // Hook to allow synchronous property change observers.
        this.propertyChanged(name, oldValue);


        // Add to reflecting properties set.
        // Note, it's important that every change has a chance to add the
        // property to `_reflectingProperties`. This ensures setting
        // attribute + property reflects correctly.
        if (options.reflect === true &&
            !(this._updateState & STATE_IS_REFLECTING_TO_PROPERTY)) {
          if (this._reflectingProperties === undefined) {
            this._reflectingProperties = new Map();
          }
          this._reflectingProperties.set(name, options);
        }
      } else {
        // Abort the request if the property should not be considered changed.
        shouldRequestUpdate = false;
      }
    }
    if (!this._hasRequestedUpdate && shouldRequestUpdate) {
      this._enqueueUpdate();
    }
  }

[jolleekin]

Synchronous property change observers are useful in many cases, at least for me. Right now, I have to use custom setters to achieve that, which has a fair amount of boilerplate.

[nomego]

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 :)

[justinfagnani]

@nomego The replacement for computed properties is getters. In cosmoz-treenode, it looks like every computed property can be replaced by a getter that accesses properties of this instead of taking parameters.

[nomego]

@justinfagnani Well it can, I initially ported it like that, but there were two issues with it.

First, given:

			_path: {
				type: Array,
				computed: '_computePath(ownerTree, keyProperty, keyValue)'
			},
			_pathToRender: {
				type: Array,
				computed: '_computePathToRender(_path, hideFromRoot, showMaxNodes)'
			},
			_pathText: {
				type: String,
				computed: '_computePathText(_pathToRender, valueProperty, pathSeparator)'
			}

_path is expensive to calculate, _pathToRender depends on it, and _pathText further depends on _pathToRender. valueProperty has a tendency to get updated, and with computed properties we never have to re-evaluate _path or _pathToRender. It is also very clear what makes up the state.
We could do it with getters and setters but it would require us to dirty-check every "save" on our own.

The second thing is that expensive getters gets mixed up with local properties, it's hard for a dev in a team to realize that this._valueProperty is fast but this._path is slow, I ended up doing a lot of const path = this._path just to avoid evaluating the getters unnecessarily, and it just becomes a nightmare in readability and maintainability within the team.

For us, the porting of this functionality makes the transition a lot easier, the state a lot more clear, and we avoid the pitfalls of re-evaluating these things unnecessarily.

[sorvell]

This has been inactive for awhile, but we recently merged #959 which is at least related to this.

In general we've chosen an asynchronous rendering model for LitElement because we think it sets up users for great performance out of the box. Rendering updates are naturally batched and occur before anything is painted to the screen. This does put a burden on the test environment, and this is one reason we've exposed the updateComplete promise. This promise can easily be awaited to check the rendering state of an element.

We've also recently landed the ability to customize LitElement's default property handling by specifying a custom property descriptor using static getPropertyDescriptor. This should enable users to make base classes that specify custom property options and perform specific actions. For example, it should be simple to make an observer type feature similar to what existed in Polymer. It's also possible to make properties that update the element synchronously by calling performUpdate. This matches our core development philosophy: setup developers for success as much as possible out of the box while also empowering them to innovate outside the box.