lit / lit-element

LEGACY REPO. This repository is for maintenance of the legacy LitElement library. The LitElement base class is now part of the Lit library, which is developed in the lit monorepo.
https://lit-element.polymer-project.org
BSD 3-Clause "New" or "Revised" License
4.49k stars 319 forks source link

Calling requestUpdate() from attributeChangedCallback() doesn't update. #1167

Closed Halt001 closed 3 years ago

Halt001 commented 3 years ago

When I call this.requestUpdate() from within attributeChangedCallback(...), because I changed a classMap, the render function is not called.

When I call this.requestUpdate() within a timeout it does seem to work. Is this the way to do it or is this a bug?

attributeChangedCallback(name: string, oldVal: AttributeType, newVal: AttributeType) {
    super.attributeChangedCallback(name, oldVal, newVal);

  ...

  this.myClassMap = {
    ...this.myClassMap,
    foo: newValueBasedOnChangedProperty,
  }

  // this doesn't seem to do anything
  this.requestUpdate(); 

  // this does trigger a re-render
  setTimeout(() => this.requestUpdate(), 0); 
}

What also seems to work is waiting for the updateComplete promise using this:

this.updateComplete.then(
  () => this.requestUpdate()
);

But it still feels like I'm putting the cart before the horse.

Versions

sorvell commented 3 years ago

I'm not able to reproduce this problem. See https://stackblitz.com/edit/lit-element-demo-azc8ae?file=src%2Fmy-element.js.

Can you please provide more info and/or include a reproduction? Thanks.

sorvell commented 3 years ago

OK, I was able to reproduce this if the attribute is reflected and is changing as a result of the property being set and implemented a fix here: https://stackblitz.com/edit/lit-element-demo-wdpubo?file=src%2Fmy-element.js.

In this case we are setting the attribute during and update and therefore attributeChangedCallback is called during the update so that requestUpdate does not queue an additional update.

In this case, I would suggest not putting your code into attributeChangedCallback but instead putting it in render or update or updated.

update(changedProperties) {
  if (changedProperties.has(propertyWithNewValue)) {
    this.myClassMap = {
      ...this.myClassMap,
      foo: newValueBasedOnChangedProperty,
    };
  }
}

If you also have an attribute that is not tied to a reactive property, you can manually call this.requestUpdate() in attributeChangedCallback to ensure that an update is processed.

If you need to respond based on rendering having occurred, you can put this code into updated instead.

Hope that helps.

sorvell commented 3 years ago

We're going to consider changing this for the next version of LitElement since it does seem to be a bit of a foot gun.

To address this, we may be able to reflect attributes before calling update. We cannot reflect attributes synchronously due to spec concerns, but we can adjust the ordering such that this is addressed.

Halt001 commented 3 years ago

Hi, you beat me to it. After I saw your first answer I made a little Angular app that demonstrated how we use the component and while making that I found out that it indeed only fails if we use property binding [mood]="mood". It does work properly if we use attribute binding [attr.mood]="mood"

I was about to upload my little Angular app when I saw your follow up. I will try your suggestions and let you how it goes.

Thank you for taking the time to investigate this.

Halt001 commented 3 years ago

I can also confirm that your suggested workaround works in our Angular app. Thank you for your help.