Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Setting string property to undefined, with reflect: true, does not remove the attribute. #872

Closed
1 of 6 tasks
bschlenk opened this issue Dec 30, 2019 · 4 comments
Closed
1 of 6 tasks
Assignees

Comments

@bschlenk
Copy link
Contributor

bschlenk commented Dec 30, 2019

Description

Setting string property to undefined, with reflect: true, does not remove the attribute.

EDIT: I see this is documented here... so my question is, why is this so inconsistent? For strings & numbers, undefined tells it not to change, but for booleans, objects, & arrays, undefined removes the attribute? It would make a lot more sense if undefined & null both removed the attribute for all cases.

Live Demo

https://stackblitz.com/edit/lit-element-example-ahub5j?file=index.html

Steps to Reproduce

  1. Create my-element with mood set to good via attribute
  2. Append my-element to document.body
  3. Set mood to undefined via property
  4. See that the mood attribute is still set

Expected Results

Expected mood attribute to be removed

Actual Results

Mood attribute is still set to great

Browsers Affected

  • Chrome
  • Firefox
  • Edge
  • Safari 11
  • Safari 10
  • IE 11

Versions

  • lit-element: v2.2.1
  • webcomponents: N/A
@justinfagnani
Copy link
Contributor

I agree this is pretty inconsistent. I can't recall why this ended up this way, but removing the attribute when the property is set to null or undefined seems about right. I'm not sure how much we can change this at this point. If we could, do we have native reflected attributes we could look at.

cc @sorvell

@mario-d-s
Copy link

mario-d-s commented Jan 28, 2020

At the very least, can't it make a string attribute empty for example? Now, the previous value is kept, which is about the worst behavior I can imagine in this particular use case. I'm saying that because you say I'm not sure how much we can change at his point, but I can't imagine why anyone would actually rely on that as a feature.

@cintaccs
Copy link

cintaccs commented Feb 1, 2020

if you prefix the attribute with . <my-element .mood="great"></my-element> it returns null. Which is the same behaviour as for empty string. Setting undefined on a DOM attribute vs setting undefined on a js variable is not the same I guess.

https://lit-html.polymer-project.org/guide/writing-templates
"Bind to properties"

removing attributes makes sense for checked, disable etc. or for custom boolean types eventually - not for strings.

the lit-element docs
https://lit-element.polymer-project.org/guide/properties

Configure reflected attributes
If toAttribute returns undefined, the attribute is not changed.

so seems to work in accordance with the docs.

@sorvell
Copy link
Member

sorvell commented Jan 12, 2022

Fixed in Lit2... closing.

@sorvell sorvell closed this as completed Jan 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants