fix: wrong attr behavior caused by empty strings

size-change-log.md

@@ -7,3 +7,4 @@ A running record of how each correctness/feature change affects the `WebComponen
 | Move default-attribute reflection out of the constructor         | ~1.19 kB (≈0)      | 1.2 kB        | **Custom Elements Spec compliance.** Constructors may not mutate attributes, so `document.createElement` on a class with `static props` threw `NotSupportedError` in real browsers. Defaults now reflect on first connect, and markup/SSR-provided attributes win. Pure code movement — no net size cost.                                                        |
 | Safe prop cloning (functions/instances) + define-time validation | 1.31 kB (+0.12 kB) | 1.2 → 1.35 kB | **No more `DataCloneError`.** `structuredClone` of a function/class-instance default crashed construction. Defaults are now cloned per key (plain data deep-copied so instances don't share object/array state; non-cloneable values kept by reference), and a one-time warning names any default whose type can't reflect to an attribute. Limit raised to fit. |
 | Derive prop types from defaults only; log type violations instead of throwing (+ `static strictProps` opt-in) | 1.33 kB (+0.02 kB) | 1.35 kB | **Loud, not fatal.** The proxy setter locked a prop's type on first write and threw on mismatch; inside `attributeChangedCallback` that `TypeError` vanished into `window.onerror` and silently skipped rendering. Types are now taken from `static props` defaults only (undeclared props stay untyped), and a declared-type violation is logged via `console.error` and skipped so `render()`/`onChanges()` still run. Teams wanting hard enforcement can set `static strictProps = true` to restore throwing. |
+| Correct empty-string / removed attribute handling; drop native-setter writes | 1.33 kB (≈0) | 1.35 kB | **State no longer corrupts on common attribute changes.** `attr=""` coerced to boolean `true` (echoed as `attr="true"`); `removeAttribute` wrote `null` into the proxy and threw before `render()`/`onChanges()`; `deserialize('', 'number')` threw. `attributeChangedCallback` now keeps strings as-is (`''` stays `''`), resets removed attributes to the declared default, wraps `deserialize` so a malformed value falls back to the raw string instead of skipping render, and no longer writes vestigial `this[prop]` fields that clobbered native setters like `title`/`id`/`lang`. Logic swap — no net size cost. |

src/WebComponent.js

@@ -115,6 +115,7 @@ export class WebComponent extends HTMLElement {
    * Triggered when an attribute value changes
    * @typedef {{
    *  property: string,
+   *  name: string,
    *  previousValue: any,
    *  currentValue: any
    * }} Changes
@@ -148,22 +149,31 @@ export class WebComponent extends HTMLElement {
   }
 
   attributeChangedCallback(property, previousValue, currentValue) {
-    const camelCaps = getCamelCase(property)
+    if (previousValue === currentValue) return
 
-    if (previousValue !== currentValue) {
-      this[property] = currentValue === '' || currentValue
-      this[camelCaps] = this[property]
-
-      this.#handleUpdateProp(camelCaps, this[property])
+    const key = getCamelCase(property)
+    const type = this.#typeMap[key]
+    let next
+    if (currentValue === null) {
+      // removal resets to the declared default (or undefined if none)
+      next = this.constructor.props?.[key]
+    } else if (type && type !== 'string') {
+      // typed props deserialize; a malformed value falls back to the raw
+      // string so render()/onChanges() are never skipped
+      try {
+        next = deserialize(currentValue, type)
+      } catch {
+        next = currentValue
+      }
+    } else {
+      // strings (and untyped props) stay as-is; '' stays ''
+      next = currentValue
+    }
 
+    // write through the proxy; item 25 makes this log-not-throw by default
+    this.props[key] = next
     this.render()
-      this.onChanges({ property, previousValue, currentValue })
-    }
-  }
-
-  #handleUpdateProp(key, stringifiedValue) {
-    const restored = deserialize(stringifiedValue, this.#typeMap[key])
-    if (restored !== this.props[key]) this.props[key] = restored
+    this.onChanges({ property, name: key, previousValue, currentValue })
   }
 
   #handler(setter, meta) {

test/WebComponent.test.mjs

@@ -449,3 +449,92 @@ describe('prop type handling (derive from defaults, log not throw)', () => {
     expect(el.querySelector('h1').textContent).toBe('Hello Ayo')
   })
 })
+
+/**
+ * Attribute value & removal handling in attributeChangedCallback: empty
+ * string stays a string (not coerced to `true`), removal resets to the
+ * declared default, malformed typed values don't skip render, and prop
+ * names that collide with native element properties (e.g. `title`) are no
+ * longer written to the instance.
+ */
+describe('attribute value & removal handling', () => {
+  it('keeps an empty-string attribute as "" (not coerced to true)', () => {
+    const changes = []
+    class Emptyable extends WebComponent {
+      static props = { label: 'hi' }
+      onChanges(c) {
+        changes.push(c)
+      }
+    }
+    window.customElements.define('attr-emptyable', Emptyable)
+    const el = document.createElement('attr-emptyable')
+    document.body.appendChild(el)
+
+    el.setAttribute('label', '')
+    expect(el.props.label).toBe('')
+    // regression: must NOT echo back as "true"
+    expect(el.getAttribute('label')).toBe('')
+    expect(changes.at(-1)).toMatchObject({ name: 'label', currentValue: '' })
+  })
+
+  it('resets a prop to its static default when the attribute is removed', () => {
+    const changes = []
+    class Removable extends WebComponent {
+      static props = { myName: 'World' }
+      onChanges(c) {
+        changes.push(c)
+      }
+    }
+    window.customElements.define('attr-removable', Removable)
+    const el = document.createElement('attr-removable')
+    document.body.appendChild(el)
+
+    el.setAttribute('my-name', 'Ayo')
+    expect(el.props.myName).toBe('Ayo')
+
+    expect(() => el.removeAttribute('my-name')).not.toThrow()
+    expect(el.props.myName).toBe('World')
+    // onChanges fired for the removal with currentValue null
+    expect(changes.some((c) => c.currentValue === null)).toBe(true)
+  })
+
+  it('does not skip render when a typed attribute value is malformed', () => {
+    const error = vi.spyOn(console, 'error').mockImplementation(() => {})
+    const changes = []
+    class Counter extends WebComponent {
+      static props = { count: 0 }
+      onChanges(c) {
+        changes.push(c)
+      }
+    }
+    window.customElements.define('attr-counter', Counter)
+    const el = document.createElement('attr-counter')
+    document.body.appendChild(el)
+
+    // valid value round-trips through the number type
+    el.setAttribute('count', '5')
+    expect(el.props.count).toBe(5)
+
+    // malformed value must not throw and must still fire onChanges/render
+    expect(() => el.setAttribute('count', 'abc')).not.toThrow()
+    expect(changes.at(-1)).toMatchObject({ name: 'count', currentValue: 'abc' })
+
+    error.mockRestore()
+  })
+
+  it('does not touch the native property for a prop named "title"', () => {
+    class Titled extends WebComponent {
+      static props = { title: 'original' }
+    }
+    window.customElements.define('attr-titled', Titled)
+    const el = document.createElement('attr-titled')
+    document.body.appendChild(el)
+
+    el.setAttribute('title', 'changed')
+    expect(el.props.title).toBe('changed')
+    // the callback no longer writes `this.title = true`; the native property
+    // reflects the real string value, not a boolean-coerced "true"
+    expect(el.title).not.toBe('true')
+    expect(el.getAttribute('title')).toBe('changed')
+  })
+})