Use hasChanged to check if changing attribute is dirty

[cdata]

We have a property that affects complex underlying state in our component. The underlying state that is affected by this property changes very rapidly (on rAF timing), and it is not worth the cost to serialize and reflect that underlying state back to the property/attribute on every frame.

So, we have configured the property with hasChanged: () => true, so that we can handle assignments to it even when the value essentially looks the same from the outside.

We received a bug report that this works as expected for property assignment, but not for setting attributes (google/model-viewer#592).

The root of the problem is that the attributeChangedCallback implementation of UpdatingElement uses a very basic, blanket dirty check that is not aware of hasChanged:

https://github.com/Polymer/lit-element/blob/508af0ee635549d1f2a87feccff4eb97b82c51ba/src/lib/updating-element.ts#L517-L521

It violates my expectations to have two different modes for dirty checking, despite explicitly configuring a check. Please consider using hasChanged in this case, or implementing an equivalent that works for attributes!

[justinfagnani]

I don't think we can reuse hasChanged, since that might expect different types than two strings. We could possibly add an attributeHasChanged property option. What you have it do in this case?

hasChanged should already be being called from _attributeToProperty after it sets the property, which should control re-rendering. Is that not happening here? Is the attribute being set on rAF timing in this case?

[justinfagnani]

@cdata, @sorvell thinks we should be able to just remove the attribute change dirty check so that all attribute changes flow through hasChanged. Is this still important to you?

[cdata]

Yes, this request still stands. We have worked around it, but ideally we would not have to do that. The number of properties that fall under the conditions described in the original issue has expanded over time.

[justinfagnani]

@sorvell should we move this to https://github.com/lit/lit ? I don't recall if the behavior already changed in 3.0

[justinfagnani]

This is fixed in Lit 2.