diff --git a/size-change-log.md b/size-change-log.md index a3ebc8e..27da8df 100644 --- a/size-change-log.md +++ b/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. | diff --git a/src/WebComponent.js b/src/WebComponent.js index 77affd3..257af5c 100644 --- a/src/WebComponent.js +++ b/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]) - - this.render() - this.onChanges({ property, previousValue, currentValue }) + 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 } - } - #handleUpdateProp(key, stringifiedValue) { - const restored = deserialize(stringifiedValue, this.#typeMap[key]) - if (restored !== this.props[key]) this.props[key] = restored + // write through the proxy; item 25 makes this log-not-throw by default + this.props[key] = next + this.render() + this.onChanges({ property, name: key, previousValue, currentValue }) } #handler(setter, meta) { diff --git a/test/WebComponent.test.mjs b/test/WebComponent.test.mjs index 112d740..d205a7c 100644 --- a/test/WebComponent.test.mjs +++ b/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') + }) +})