Values changed outside of lit-html rendering are not updated from lit-html

[justinfagnani]

Since lit-html stores previously rendered values for dirty-checking, any value changed from outside lit-html will not be overwritten in another render if the value passed to lit-html is the same.

See this live example: https://stackblitz.com/edit/lit-html-rt4n6e

This is actually working as intended, as it's intended that lit-html maintains control over bound values. But this issue has come up a few times in properties that are affected by user actions:

• HTMLInputElement.value
• Element.scrollTop

Currently the recommended solution would be to add event listeners which update the bound value so that they're in sync. That's non-obvious though, and we should handle this automatically if we can.

One option is to stop storing the previous value in AttributePart, PropertyPart, etc., and instead use the current bound attribute/property value instead as the source of truth.

There are two potential downsides:

• It might be slower, especially for native DOM properties. Crossing DOM bindings just to see if a value should be updated might slow things down.
• Values may be modified by the setter. For instance, e.className = ['a', 'b'] results in a className of a,b. This would defeat the dirty check and cause it to be set every render.

Another solution is to vend a directive that defers to the underlying property specifically for these cases:

import {live} from 'lit-html/directives/live.js';
// ...
html`<input .value=${live(value)}`>`;
// or
html`<div .scrollTop=${live(top)}`></div>`;

[AStaroverov]

I think directive way more clearify solution, but less intuitive for beginers.

Anyway, i prefer directive.

[ruphin]

How will the directive work in multi-part attributes?

[fopsdev]

If going for the slower "dom-checking" method then please make it configurable.
Its just because there are cases where the underlying state store takes responsibility for updating a element and doing some additional checking would just be overhead

[blikblum]

For reference, this is a test showing the performance impact of reading DOM property: https://mhevery.github.io/perf-tests/DOM-megamorphic.html

[mercmobily]

What would th implementation of live be? I am hitting this now and I would love to be able to fix it nearly. Thanks!

[fineline]

@mercmobily A similar directive that fixed the problem for me was provided on a linked issue here.

[mercmobily]

I am about to hit this problem. @justinfagnani do you have something "official" in the works? It will avoid me wasting time on my own (often half broken) solution...

[ernsheong]

Just encountered this. My element scroll was not preserved after a re-render.

[ernsheong]

Something worth considering for this issue is what happens when you are editing/focusing on an input when a re-render happens... input loses focus at the moment.

[mercmobily]

@ernsheong Are the two issues related...?

[ernsheong]

Upon closer examination, it seems so, but only because of the scroll. The scroll reset causes my nested input to lose focus after a render.

But my input is just my 2 cents, I may have overlooked something...

[lastmjs]

I have a flex container that has multiple identical custom elements with textareas in them. The flex children custom elements are created by mapping over an array of plain data objects. If I type into each of the textareas, and then remove one of the objects from the array and rerender, the values of the textareas are incorrect. If I remove an element, the remaining element's textarea has the value of the removed element's textarea. This happens whether or not I am binding to the value property of the textarea. Is this a related issue or something else entirely?

[ruphin]

This is an unrelated issue, there is most likely some kind of error in your code that passes the data around.
Here is a working example of what you are trying to do:

https://codepen.io/ruphin/pen/EBJGaa?editors=1010

[brother-donkey]

This happens with the checked attribute on certain input types as well.

[mercmobily]

@justinfagnani What solution are you guys leaning towards?

[innerop]

$0.02

nothing wrong with the events listener approach: it offers more control over the event stream processing... who says I want every keyup/press/etc or every scroll even to cause a render? if you provide a 'live' directive it should take a stream processing function e.g. debounce.

also what's confusing to me personally in your example is the use of '.value' on input which AFAIK suggests it's a property of the underlying element were it's actually an attribute. Maybe attributes are referenced with the .attributeName notation? I thought attributes don't have a preceding dot and properties on the element do (borrowing from the JS object properties syntax a.b.c etc)

@justinfagnani ^

[ruphin]

There are multiple reasons why you generally want to use properties over attributes when possible.

The fist one is that attributes are generally used to set the initial state of an element, and properties generally reflect the current state of an element. Not every element reacts to changes in attributes, but changing the state through properties will almost always have an effect.

The second reason is that attributes have overhead. You don't need to interact with the DOM if you set properties directly.

[ChrisMagnuson]

The work around I am using was linked to above here and spelled out here.

To summarize:

• Import directive
• Add the following directive

const forceWrite = directive((value) => (part) => {
  part.setValue(value);
});

• Change .checked=${isChecked} to .checked=${forceWrite(isChecked)}

In my case I am generating lists of radio buttons based on other input field's values.

If a radio button property was set to checked outside of lit-html then when a new list of radio buttons was rendered they would retain the incorrect state of checked = true.

If I am understanding it correctly, even though in the code .checked="${isChecked} isChecked was false lit-html remembered that when it generated that element it was false so it thought it didn't need to set it to false which incorrectly leaves it as true.

[ruphin]

That work around is correct, and is a somewhat heavy-handed but totally valid approach to getting the desired outcome.

The real problem is that the model (what lit-html thinks is the value of the input) and the reality (the actual state of the input) is not the same. This happens when "a radio button is set to checked outside lit-html" and this change is not correctly synced back into the model. If lit-html thinks the checked property was false and still is false, it won't bother setting it to false. This is good, because it cuts down on useless operations, but if your checked property changed to true outside the knowledge of lit-html, then it is obviously bad, because the property should be set to false again.

The "real" solution to fixing this problem is by making sure your input state is always synced with what lit-html thinks, but this can be very cumbersome and sometimes hard do correctly. In those cases, a simple solution like the above can be appropriate.

I will try my hand at implementing a live directive over the weekend, which is an alternative to this force directive, but in the grand scheme of things I expect the performance difference between these to not be very significant.

[justinfagnani]

A live() directive would be great, IMO.

I would also love if we had more benchmarks that used property bindings, both to custom elements and native elements, so that we could check out changes like switching Property parts to not track their previous values and defer to the underlying property. If that was fast enough it would simplify the model and reduce some memory overhead.

[mercmobily]

we could check out changes like switching Property parts to not track

their previous values and defer to the underlying property This would be really ideal, and it will save a lot of people from a lot of head scratching -- and it will prevent extra documentation for edge cases. Justin, wouldn't it be better to have this somehow configurable (as a parameter) in lit-html, so that developers can create elements using lit-element with the right parameter to lit-html depending on their preference?

…

On Sun, 25 Aug 2019 at 07:49, Justin Fagnani ***@***.***> wrote: A live() directive would be great, IMO. I would also love if we had more benchmarks that used property bindings, both to custom elements and native elements, so that we could check out changes like switching Property parts to not track their previous values and defer to the underlying property. If that was fast enough it would simplify the model and reduce some memory overhead. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#877>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAQHWXR2HWU7U2TC5GVQQLTQGHCJRANCNFSM4HASYF4A> .

[ruphin]

I'm fairly certain the performance cost of using underlying properties for source of truth is very minimal. I am concerned it will cause problems with setters, both because it will invalidate the cache causing lit-html to write on every render, and because custom setters are generally more expensive to write to than basic properties.

[jridgewell]

I think @justinfagnani is saying we can not check the cached value (on the part), but instead check it directly on the node:

if (node[prop] !== value) {
  node[prop] = value;
}

[ruphin]

I understand that. The problem I was referring to is when properties have setters and getters that do not return the exact value that was written, as mentioned by @justinfagnani in the initial post:

Values may be modified by the setter. For instance, e.className = ['a', 'b'] results in a className of a,b. This would defeat the dirty check and cause it to be set every render.

Here is a basic implementation of the suggested live directive:

const live = directive((value) => (part) => {
  const {single, element, name} =  part.committer;
  if (single && (value !== element[name])) {
    part.setValue(value);
  }
});

Live demo: https://stackblitz.com/edit/lit-html-yhtdf2?file=src/index.js

We could add additional checks to verify that it is used only in property parts, since this may fail to work in other contexts, but I'm not sure if that will be necessary. It may also be interesting to have multi-part support, but I think that is such a rare use case that it's not worth the additional complexity.

I can make a proper PR for this feature once we agree on the scope.

[jridgewell]

Can we check for coercion?

const live = directive((value) => (part) => {
  const {single, element, name} =  part.committer;
  if (single && (value !== element[name])) {
    part.setValue(value);
    part.commit();
    if (element[name] !== value) {
      console.warn('live binding value not respected');
    }
  }
});

[qwelias]

Seems like it's by design tho.

here

[LarsDenBakker]

We created @open-wc/lit-helpers for these kinds of directives, so that we have a place to experiment and see whether certain functionality is useful before it is added to lit-html officially. https://open-wc.org/developing/lit-helpers.html#live-binding

[danbrycefairsailcom]

Currently the recommended solution would be to add event listeners which update the bound value so that they're in sync.

Do you have an example of this working with scrollTop?

Specifically, I am trying to set scrollTop programmatically, to auto-scroll an element to the bottom.

This is the problematic code:

    const elem: Element = this.shadowRoot.querySelector('.my-elem');
    if (elem) {
        // TODO: Doesn't work
        elem.scrollTop = elem.scrollHeight;
    }

[mercmobily]

Thanks for this Justin. May I ask... can you ask the documentation guys to actually document it? At the moment there is no sign of it in https://lit-html.polymer-project.org/guide/template-reference and I think it's a very useful directive in some common cases

…

On Thu, 16 Jan 2020 at 07:47, Justin Fagnani ***@***.***> wrote: Closed #877 <#877> via #1057 <#1057>. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#877>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAQHWXTZMQ5XZ5ISWE4GZPTQ56OCFANCNFSM4HASYF4A> .