Skip to content

@Prop({ reflect: true }) boolean never removes its attribute unless the DOM value is already the canonical "" (side effect of #2485's enumerated-attribute guard) #6828

Description

@icarobteles

Stencil version: 4.43.5 (verified same code path on main)

Current behavior

When a @Prop({ reflect: true }) boolean prop is set to false, Stencil only calls removeAttribute if the element's current attribute value is exactly "". If the attribute was written with any other literal string (skeleton-loading="true", disabled="true", etc.) — most commonly because it was present in the static/SSR HTML before the component upgraded — the attribute is never removed, even though the property correctly reports false and re-renders keep happening.

Expected behavior

A real Boolean-typed @Prop({ reflect: true }) should always reflect via presence/absence, regardless of what the attribute's literal string value was before Stencil ever touched it.

Minimal repro

@Component({ tag: 'my-el' })
export class MyEl {
  @Prop({ reflect: true, mutable: true }) flag: boolean = false;
  render() { return <div>x</div>; }
}
<my-el id="a" flag="true"></my-el>   <!-- buggy: attr stuck forever -->
<my-el id="b" flag></my-el>          <!-- fine: canonical "" -->
<script>
  customElements.whenDefined('my-el').then(() => {
    document.getElementById('a').flag = false;
    document.getElementById('b').flag = false;
    // a.hasAttribute('flag') -> still true ("true")
    // b.hasAttribute('flag') -> false (correct)
  });
</script>

Verified this exact repro against a live build (@stencil/core@4.43.5, real browser via Playwright), for both a raw minimal component and a real-world one (a Stencil design system component reflecting skeleton-loading).

Root cause

src/runtime/vdom/set-accessor.ts, in setAccessor:

if (newValue == null || newValue === false) {
  if (newValue !== false || elm.getAttribute(memberName) === '') {
    elm.removeAttribute(memberName);
  }
}

This guard was introduced in #2485 (fixing #2484, draggable={false} in static JSX) to preserve an explicit "false" string for enumerated attributes (draggable, aria-*, etc.), where per HTML semantics an explicit "false" is meaningfully different from absence.

The problem is that setAccessor is the same code path used to reflect @Prop({ reflect: true }) values (via $attrsToReflect$), where Stencil already knows statically (propType & Boolean, from the component's own compiled metadata) that the prop is a true Boolean, not an enumerated attribute. The guard shouldn't apply to that path at all, but it currently applies universally.

Why this matters in practice

Any consumer whose templating writes the attribute as a literal string — Angular [attr.x]="cond" bindings, SSR templates that serialize booleans as "true"/"false", etc. — instead of binding the property directly, permanently loses the ability to ever clear that attribute once set. This breaks any [attr]-presence CSS selector tied to it (e.g. :host([disabled]) { pointer-events: none } never releasing).

Comparison with Lit

For contrast, Lit's reflect: true (@lit/reactive-element's _$ET) has no equivalent bug: it computes the attribute value purely from the converted property value (converter.toAttribute) and unconditionally calls removeAttribute/setAttribute — it never inspects the attribute's current live DOM value before deciding. Verified this with an isolated LitElement repro of the same scenario; both the flag="true" and bare flag initial-markup cases correctly clear the attribute.

Suggested fix

Scope the guard to the actual small set of enumerated attributes (draggable, contenteditable, spellcheck, aria-*), or skip it entirely when the write originates from $attrsToReflect$ (a real typed @Prop), since propType is already known at that call site.

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions