lit-element icon indicating copy to clipboard operation
lit-element copied to clipboard

Secondary property change not reflected when first property set via attribute

Open dbatiste opened this issue 5 years ago • 3 comments

Description

When a second property is changed internally due to a change in another property via setting its attribute, the second property is not reflected as expected (given reflect: true). For example, given the following element, where setting myprop in turn changes another property _someprop, the change to _someprop does not get reflected if myprop was changed via setAttribute/removeAttribute:

import { html, LitElement } from 'lit-element/lit-element.js';
class MyElem extends LitElement {
    static get properties() {
        return {
            myprop: { type: Boolean, attribute: true },
            _someprop: { type: Number, reflect: true }
        };
    }
    constructor() {
        super();
        this._myprop = false;
        this._someprop = 0;
    }
    get myprop() {
        return this._myprop;
    }
    set myprop(val) {
        const oldVal = this._myprop;
        if (oldVal !== val) {
            this._myprop = val;
            this._someprop += 1;
            this.requestUpdate('myprop', oldVal);
        }
    }
    render() {
        console.log('render', this.myprop, this._someprop);
        return html`<div></div>`;
    }
}
customElements.define('my-elem', MyElem);

From @arthurevans (Slack thread):

OK, I can reproduce this, and it seems to me like a bug. The following appears to work around the issue:

this.requestUpdate('myprop', oldVal).then(() => {
    this._someprop += 1;
});

This renders twice whenever you change myprop, so it's not great.

A little more info: this is sort of a corner case. When LitElement sets a property because an attribute has changed, it sets a flag. It checks that flag before reflecting a property to an attribute, to avoid an infinite loop. In this case, because one attribute change is causing a different property to be set, the second property isn't being reflected, because LitElement doesn't actually check that the property that triggered the requestUpdate is the same as the one doing the reflecting. That's the bug.

As Arthur mentioned, waiting for the first property update to render before changing the second property works around the issue but results in more than one render.

Live Demo

https://stackblitz.com/edit/property-reflect-bug?file=my-element.js

Steps to Reproduce

  1. Create the element as describe above, and set myprop via attribute
  2. Observe that _someprop change is not reflected to it's corresponding attribute as expected

or...

  1. Go to the StackBlitz live example linked above.
  2. Click on the button to update myprop via attribute
  3. Observe that _someprop change is not reflected to it's corresponding attribute as expected

Expected Results

The second property change should be reflected to its corresponding attribute (given reflect: true) without requiring the second iteration of render.

Actual Results

The second property change is not reflected to its corresponding attribute.

Browsers Affected

  • [x] Chrome
  • [x] Firefox
  • [x] Edge
  • [x] Safari 11
  • [x] Safari 10
  • [x] IE 11

Versions

  • lit-element: v2.3.1
  • webcomponents: v2.4.3

dbatiste avatar Apr 17 '20 04:04 dbatiste

Sorry for the delay on this. We're looking at addressing the issue here, but we're concerned about potentially degrading performance. We also view this as somewhat of a corner case. In the meantime, you can workaround the issue by computing the secondary property in update as is shown here:

https://stackblitz.com/edit/property-reflect-bug-mrjep4?file=my-element.js

sorvell avatar May 04 '20 17:05 sorvell

sorvell, thank you for the PR and the workaround. I'm facing the same problem. I didn't think it was so "corner case".

I have an element with 2 reflected attributes/properties: points and maxPoints. Both have custom setters so that:

  1. if you assign points higher than maxPoints, the operation is "rolled back", i.e. points is not changed;
  2. if you assign maxPoints lower than the currently assigned points, points are decreased to maxPoints.

Making those assignments through properties works fine. But making them via attributes doesn't: in the second scenario, 2 attribute changes should be triggered, but only maxPoints is actually updated. In the end, the property points holds the correct value, but the attribute points is not updated.

Leokuma avatar May 22 '20 22:05 Leokuma

We got this problem too. While there's no fix, we solve it using: myObject = Object.Assign({}, myObject)

after any changes on 'myObject' occurs. This should trigger the shouldUpdate method and do the trick for us.

gabrielgons avatar Jun 14 '20 23:06 gabrielgons

Closing as this is fixed in the current Lit version and there are no plans to change the behavior in the previous version of LitElement.

See https://lit.dev/playground/#gist=9a6934613b6204e035a3baff1fa54a3e.

sorvell avatar Sep 08 '22 21:09 sorvell