ionic-team / stencil

A toolchain for building scalable, enterprise-ready component systems on top of TypeScript and Web Component standards. Stencil components can be distributed natively to React, Angular, Vue, and traditional web developers from a single, framework-agnostic codebase.
https://stenciljs.com
Other
12.5k stars 783 forks source link

bug: Boolean property not removing corresponding attribute when set to false programmatically #5510

Closed a7medm7med closed 5 months ago

a7medm7med commented 6 months ago

Prerequisites

Stencil Version

4.12.6

Current Behavior

The disabled attribute remains on the element even after setting the property to false.

Expected Behavior

The disabled attribute should be removed from the element when the corresponding property is set to false.

System Info

System: node 18.19.0
    Platform: windows (10.0.22631)
   CPU Model: 11th Gen Intel(R) Core(TM) i5-11400H @ 2.70GHz (12 cpus)
    Compiler: D:\Projects\web-components\node_modules\.pnpm\@stencil+core@4.12.6\node_modules\@stencil\core\compiler\stencil.js
       Build: 1704730339
     Stencil: 4.12.6
  TypeScript: 5.2.2
      Rollup: 2.42.3
      Parse5: 7.1.2
      Terser: 5.26.0

Steps to Reproduce

1- Set up a StencilJS component with a boolean property, for example: 2- Instantiate the component and set the property to false programmatically, for example: 3- Observe that the disabled attribute is not removed from the element.

Code Reproduction URL

https://github.com/a7medm7med/stencil-bug-3

Additional Information

No response

rwaskiewicz commented 6 months ago

Thanks! Something doesn't appear to be right here, I'll get it ingested into our backlog

tanner-reits commented 5 months ago

@a7medm7med I took a look at this issue again and your provided reproduction case. I was able to get the expected behavior by applying the following patch: Changes-On-fb174e.patch

I did need to update your component's render function to render a tag since when I added reflect to the prop, the text no longer appeared... I'll look into that separately from this issue.

Let me know if that works for you!

a7medm7med commented 5 months ago

@tanner-reits Thanks for your update and for sharing the patch. However, in my case, I found that I don't need to use the reflect option. Additionally, it seems that the issue extends beyond just the appearance of the text; the attribute itself should not remain.

Let me know if you have any thoughts or if there are any updates on your end!

tanner-reits commented 5 months ago

@a7medm7med I'm a little confused what you mean by "in my case, I found that I don't need to use the reflect option". reflect is what keeps the property value in sync with an attribute value. If reflect: true is not set on the @Prop(), then the attribute cannot be removed by the runtime. You can read a bit more about this option in the Stencil docs.

a7medm7med commented 5 months ago

@tanner-reits To be honest, I can't recall the specific problem I encountered, but it seems that the 'reflect' option suffices for now. Let's close this issue temporarily. If I encounter any further issues, I'll create a new one.