Fix `setAttribute()` after mount not updating reflected props (closes #98) by DmitrySharabin · Pull Request #109 · nudeui/element

DmitrySharabin · 2026-05-12T19:36:56Z

TL;DR

Calling el.setAttribute("v", "100") after mount on a reflect: true prop did not update el.v, because customElements.define cached an empty observedAttributes list and a null attributeChangedCallback at registration time. This PR makes ACB always present on the prototype chain via the props plugin's provides, which lets the spec read observedAttributes and triggers the eager static getter that populates it.

Why

Per HTML spec §CustomElementRegistry.define:

For each lifecycle-callback name, ? Get(prototype, callbackName).
observedAttributes is initialized to an empty sequence.
Only if lifecycleCallbacks["attributeChangedCallback"] is non-null, ? Get(constructor, "observedAttributes").

Pre-this-PR, attributeChangedCallback was wired inside first_constructor_static (which fires on first instance construction, after registration). The browser snapshotted a null ACB at define time and so never read observedAttributes either. Pre-set attributes worked only because Props.initializeFor re-walks observedAttributes on mount.

The natural fix is to make sure ACB is reachable through the prototype chain by the time customElements.define runs.

What

`src/plugins/props/index.js`

attributeChangedCallback becomes a regular entry in the plugin's provides. addPlugins lands it on NudeElement.prototype once, and every subclass — at any depth of inheritance — inherits it through the chain. So Get(Class.prototype, "attributeChangedCallback") returns it during define, which causes the spec to read Class.observedAttributes, which triggers the eager static getter and populates the cache.

Chaining to a user-declared override follows the same pattern as connectedCallback in src/element/members.js: getSuperMethod(this, provides.attributeChangedCallback)?.call(this, ...) then the plugin's dispatch into Props.attributeChanged. A subclass that overrides attributeChangedCallback must call super.attributeChangedCallback(...) to compose, which is standard JS — see "Known caveat" below.

The eager static observedAttributes getter on providesStatic is kept — its only job now is to lazily call defineProps() and return the list. The previous ACB install block inside the getter is gone (it was the chicken-and-egg: ACB-on-prototype is the precondition for the getter ever firing).

The cache is still reserved as [] before defineProps runs, so a define-props hook that reads Class.observedAttributes hits the cache instead of recursing — same re-entry-guard shape as attachShadow in src/plugins/shadow/index.js.

The props plugin's setup hook still skips defineProps() when the static getter has already cached observedAttributes, so plugin-contributed additive calls (e.g. events/onprops appending synthetic on* props via this.defineProps(newProps)) keep working after the static install.

`test/util/FakeElement.js`

FakeElement.with now mirrors the spec faithfully when building its per-class CustomElementDefinition-shaped snapshot:

Read attributeChangedCallback from the prototype chain first.
Only if non-null, read observedAttributes from the constructor.

This closes the loophole the previous test harness had: it used to read Class.observedAttributes unconditionally, which triggered the eager install regardless of whether ACB was actually on the prototype — making the unit test pass even when the real-browser path was broken (the central complaint of the PR review). With this conditional, the unit suite is now load-bearing on the spec path.

The snapshot logic was also extracted into a new FakeElement.define(Class) static method, so a test can swap a class's prototype ACB and re-snapshot — modelling how customElements.define snapshots each registered class independently. Used by the subclass-override regression test below.

setAttribute / removeAttribute continue to dispatch through the snapshot, the same path a real browser takes via lifecycleCallbacks.

`test/Props.js`

The existing "Post-mount setAttribute updates the property" test now exercises the real registration path end-to-end. Without provides.attributeChangedCallback, it fails with Got undefined, expected 100 — the literal symptom from #98.

The Class.defineProps() additive-contract test (added in an earlier commit on this branch) still pins down that defineProps({ bar: {} }) after the registration-time install must still register bar.

A new Subclass attributeChangedCallback override group pins both halves of the new contract (see "Known caveat" below):

Override that calls super updates the reflected prop (expect 100) — verifies that chaining via super.attributeChangedCallback(...) still flows through Props.attributeChanged.
Override that omits super leaves the reflected prop unchanged (expect undefined) — verifies that forgetting super does silently drop the reflection, so the contract is observable in tests.

Both halves were sanity-checked: inverting either branch causes exactly the opposite test to fail with the expected symptom.

Known caveat — behavior change vs pre-PR-109

Pre-PR-109, first_constructor_static unconditionally wrapped any existing ACB on the prototype chain. A subclass could declare its own attributeChangedCallback and still get the plugin's reflected-prop dispatch fired automatically, without calling super. PR-109 partially broke that auto-wrap (its !Object.hasOwn(this.prototype, "attributeChangedCallback") guard skipped wrapping for any subclass that had an own ACB). This PR drops the auto-wrap entirely.

The new contract: any subclass that overrides attributeChangedCallback must call super.attributeChangedCallback(name, oldValue, value) to compose with the plugin. The same convention already governs connectedCallback / disconnectedCallback in this codebase. Pinned by the new regression test in test/Props.js.

Why auto-wrap can't be restored without an API change. JavaScript class-method syntax installs a data property on the subclass prototype via internal DefineOwnProperty — that path bypasses any setter trap we could install on the base prototype, so there is no language-level way to intercept "subclass just defined an ACB" without consumer cooperation. The spec snapshots ACB at customElements.define time and never looks at the prototype again, so wrapping later (eager getter, first construction, etc.) is invisible to the cached callback. The pre-PR-109 wrapping landed at first construction — which is precisely why the spec snapshot was already cached (with null) by then, i.e. the auto-wrap was the bug. You can have auto-wrap or spec-correct registration, not both, without introducing a NudeElement.define(name) registration helper.

Footgun severity. This is the worst lifecycle hook to require manual super-chaining on, from a footgun-cost perspective: forgetting super silently re-introduces #98 for that one subclass. By contrast, forgetting super.connectedCallback?.() only breaks the connected hook (propchange queue drain on reconnect — obscure). A registration helper that would restore auto-wrap without re-introducing the spec problem is proposed for discussion in #110.

Out of scope

// FIXME how to combine with existing observedAttributes? (src/plugins/props/index.js) — preserved. Merging a user-declared static observedAttributes with derived ones is an open question; current shadowing semantics are kept.
ACB inheritance edge cases — tracked separately under [Props] static props not inherited from parent class #104.
A registration helper (NudeElement.define(name)) that auto-wraps lifecycle callbacks uniformly — not introduced here. Proposed for discussion in RFC: registration helper to auto-wrap lifecycle callbacks (follow-up to #98 / #109) #110.

Test plan

npm test — 102/103 pass (1 pre-existing skip in split()).
Negative control: commenting out the new provides.attributeChangedCallback → "Post-mount setAttribute updates the property" fails with Got undefined, expected 100, proving the prototype-resident ACB is what enables the spec path.
Negative control: removing the acb ? conditional in FakeElement.with → tests still pass but the harness stops mirroring the spec; confirms the conditional itself is load-bearing on the regression check.
Negative control on the new override-contract test: unconditionally chaining to super → "Override that omits super leaves the reflected prop unchanged" fails with Got 100, expected undefined; never chaining → "Override that calls super updates the reflected prop" fails with Got undefined, expected 100. Both halves are load-bearing.
Real-browser spot check (manual): in the deploy preview, on <color-picker> (which inherits two levels — ColorPicker → ColorElement → NudeElement), call setAttribute on a reflected prop from DevTools and confirm the JS property updates.

netlify · 2026-05-12T19:37:02Z

✅ Deploy Preview for nude-element ready!

Name	Link
🔨 Latest commit	`eedd2dc`
🔍 Latest deploy log	https://app.netlify.com/projects/nude-element/deploys/6a04b77814dcc80008b89315
😎 Deploy Preview	https://deploy-preview-109--nude-element.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

`Class.observedAttributes` was wired inside `first_constructor_static`, which only runs on first instance construction. But `customElements.define(name, Class)` reads it at registration time — before any instance exists — so the browser caches the empty list and never fires `attributeChangedCallback` for subsequent attribute mutations. Move installation to a static getter on `providesStatic` keyed by `symbols.known.observedAttributes`. The cache is reserved as `[]` before `this.setup()` runs so a setup hook reading `Class.observedAttributes` hits the cache instead of recursing — same shape as `attachShadow`'s re-entry guard in shadow/index.js. Tests: - `FakeElement` now simulates the browser's observedAttributes cache in `setAttribute` / `removeAttribute`. With plugins passed through `with(...)` it reads the plugin's static getter (the `customElements.define` path); without plugins it falls back to the `Props` instance, preserving old test semantics. - The existing `"Post-mount setAttribute updates the property"` test now imports plugins dynamically and exercises the static-getter path — without this fix it fails with `Got undefined, expected 100`, the issue's literal symptom.

…98) The static observedAttributes getter previously called this.setup(), which fires every installed plugin's setup hook — implicitly moving setup() from first-construct to customElements.define time for *all* plugins, not just props. Switch to this.defineProps(): only the props plugin's own initialization fires early. Other plugins' setup timing is unchanged. define-props is the extension point for contributing observed attributes, so plugins that want to inject them aren't regressed. Add an idempotency guard (this[props].size > 0) to defineProps so the still-firing props setup hook at first construct is a no-op when the getter already populated this[props]. FakeElement: convert the parallel `defined` WeakMap to a per-class Symbol slot, captured once at FakeElement.with() time — mirrors customElements.define's one-time read at registration. setAttribute / removeAttribute consult the snapshot, not Class.observedAttributes live, so the test util truly reproduces the registration-time semantics the bug is about.

2ad9110 added `|| this[props].size > 0` to defineProps so the still-firing props setup hook would no-op after the observedAttributes getter populated this[props]. But the guard fires on *any* later call too, blocking plugin- contributed additive props — e.g. events/onprops appends synthetic on* props via `this.defineProps(newProps)`, which silently no-ops once the static install has run. Move the idempotency to the setup hook, where the asymmetry actually lives: only the props plugin has a registration-time install path, and the observedAttributes cache symbol is the natural marker for "the early path already ran." defineProps reverts to its original purely-additive shape — no size guard, no overloaded default-arg-means-install semantics. Regression test in test/Props.js asserts that defineProps remains additive after the registration-time install — pinning down the contract that 2ad9110's guard violated.

Move the patch from `first_constructor_static` into the eager `observedAttributes` getter so it lands on the prototype before `customElements.define` snapshots `lifecycleCallbacks`. Without this, post-mount `setAttribute` on a reflected prop did not update the prop in real browsers — the second half of #98 left open by PR #109. Strengthen FakeElement to dispatch `setAttribute`/`removeAttribute` through a snapshotted `CustomElementDefinition`-shaped record so the existing `Post-mount setAttribute updates the property` test exercises the full ACB chain.

DmitrySharabin · 2026-05-13T14:51:05Z

Posted on behalf of @DmitrySharabin. Investigation and writeup by Claude (Anthropic).

Empirical check in a real browser (Chromium 150) on color-elements (the primary consumer of nude-element) with this PR applied: the eager observedAttributes getter is never invoked by customElements.define for the registered classes. The registration-time snapshot of attributeChangedCallback is therefore still null, and post-mount setAttribute does not update reflected props — the very symptom #98 reports. The unit tests in this PR pass because FakeElement.setAttribute calls Props.attributeChanged directly, which bypasses the customElements.define-snapshot path the bug is actually about — so they certify nothing about real-browser behaviour.

Reduction (same page where nude-element is loaded)

class Direct extends NudeElement { static props = { foo: { reflect: { from: true } } } };
customElements.define("x-direct", Direct);
// ✅ eager observedAttributes getter fires; ACB patch installed; post-mount
//    setAttribute updates the prop.

class Mid extends NudeElement {};                    // empty intermediate
class Leaf extends Mid { static props = { foo: { reflect: { from: true } } } };
customElements.define("x-leaf", Leaf);
// ❌ eager getter never fires (verified with console.warn at the top of the
//    getter — silent during define). ACB patch never installed at registration.
//    Post-mount setAttribute does NOT update the prop.

Mid doesn't need static props, static plugins, or anything else — the mere presence of an intermediate class between the registered class and NudeElement is enough to break the lookup. Every color-element matches the bottom shape (ColorPicker extends ColorElement extends NudeElement), so in practice this PR doesn't fix #98 for the primary consumer.

What I verified

Minimal HTML page that imports only <color-picker> via the importmap, no docs-page scaffolding, no html-demo, no wrappers around customElements.define, no instrumentation in the loaded props/index.js module path.
console.warn injected at the top of the eager getter is silent during page load (even after 2.5s settle).
The class is registered (customElements.get("color-picker") returns it), so customElements.define was called.
After registration, Object.getOwnPropertyDescriptor(ColorPicker.prototype, "attributeChangedCallback") is undefined.
Manually reading ColorPicker.observedAttributes from DevTools does fire the getter — confirming the getter is on the chain and reachable via ECMAScript `Get`, just not via the path customElements.define takes here.

A hand-rolled minimal mimic of the same shape (3- and 4-level inheritance with observedAttributes installed via Object.defineProperty inside a static {…} block) does not reproduce. So the failure isn't generic Chromium behaviour with inherited static getters — it's something specific to nude-element's class-construction chain that I couldn't fully isolate. The failure persists with an empty intermediate class (no static props, no static plugins, no static {…} block) declared in the same script as the leaf class.

Direction

For this PR to actually close #98, the eager install needs to land as an own property of the registered subclass before customElements.define runs — not via prototype-chain Get reaching the getter on NudeElement. Two shapes that work:

In nude-element. Ship a Class.prepareForDefine() helper (or fold into the api plugin's static side) that subclasses call right before customElements.define. Reading this.observedAttributes inside it triggers the existing eager getter, which installs the cache and the attributeChangedCallback patch on the registered subclass as own properties. The existing setup hook is too late — it fires from constructed, after registration.
In consumer code. A define wrapper (e.g. ColorElement.define in color-elements) reads Class.observedAttributes before calling native customElements.define. This is what an earlier diagnostic wrapper I added effectively did — which is why the tests looked clean before I removed the wrapper, and why I initially (incorrectly) reported the fix as verified. It's a real workaround, but the proper fix belongs here.

Mea culpa

My implementation-notes comment on #98 called the unit-test approach "load-bearing" with a real-DOM smoke test as a complement. The complement was never actually exercised; without it, the unit suite certifies nothing about the snapshot path. A no-wrapper Playwright load of /src/color-picker/ (or anything that registers a class via two-step inheritance from NudeElement) would have caught this.

…#98) Per the HTML spec, `customElements.define` only reads observedAttributes if `attributeChangedCallback` is non-null on the prototype chain at the moment `define` runs. The previous fix installed ACB inside the eager `observedAttributes` getter — chicken-and-egg: the spec never reaches the getter because ACB hasn't been installed yet. Move ACB into the props plugin's `provides`, so `addPlugins` lands it on NudeElement.prototype where every subclass inherits it. Spec finds ACB via the chain → reads observedAttributes → eager getter fires → cache populated. Works at any depth of inheritance. Strip the now-redundant ACB install from the eager getter; its only job is to populate this[props] and return the list. Tighten FakeElement.with to mirror the spec faithfully: read attributeChangedCallback from the prototype chain first, and only read observedAttributes if ACB is non-null. The unit suite now exercises the spec path it was meant to certify — confirmed via negative control (removing the new provides.attributeChangedCallback makes \"Post-mount setAttribute updates the property\" fail with the literal symptom from #98).

DmitrySharabin · 2026-05-13T15:40:31Z

Posted on behalf of @DmitrySharabin. Verification by Claude (Anthropic).

Verified a3539c5 works end-to-end in color-elements against the docs page (/src/color-picker/), fresh load in Chromium 150, no instrumentation, no customElements.define wrappers, no other probe trickery.

What I checked

acbDescOnSubclass: false              ← ColorPicker doesn't shadow; inherits
acbOwner:          "NudeElement"      ← lands on the parent prototype, as designed
inMarkup setAttribute("space","p3"): "oklch" → "p3"  ✓
fresh   setAttribute("space","p3"): "oklch" → "p3"  ✓

Cross-component smoke (programmatically created, mounted, then setAttribute):

Case	Result	Pass
`color-swatch.value` (`reflect:{from:true}`)	`""` → `"rebeccapurple"`	✓
`color-picker.color` (`reflect:{from:"color"}`)	`oklch(60% 0.12 180)` → `oklch(80% 0.08 60)`	✓
`channel-slider.value` (Number, `reflect:{from:"value"}`)	`50` → `75`	✓
`color-chart.xMin` (attr `xmin`, prop `xMin`)	`"auto"` → `"25"`	✓

Note on the approach

Putting attributeChangedCallback in provides is cleaner than the earlier eager-getter install I had floated in #98 and on this PR — it sidesteps the chicken-and-egg the spec creates ("only read observedAttributes if ACB is non-null on the chain"), works at any inheritance depth without per-subclass coordination, and folds chain delegation into a single getSuperMethod call instead of a per-subclass closure capture. The strengthened FakeElement.with (ACB-first lookup, observedAttributes only if non-null) now exercises the same path real browsers do, so the unit suite actually certifies the spec contract.

LGTM from my end.

The plugin's ACB now lives on the inherited prototype (NudeElement.prototype via provides). A subclass that declares its own attributeChangedCallback shadows it: forgetting `super.attributeChangedCallback(...)` silently re-introduces #98 for that subclass. Add a regression test pinning both halves: override that calls super updates the reflected prop (expect 100); override that omits super leaves the prop unchanged (expect undefined). Verified each test fails in isolation when its branch is inverted. Extract `FakeElement.define(Class)` from `FakeElement.with` so the test can swap a class's ACB and re-snapshot — mirroring how `customElements.define` snapshots each registered class.

…98) Port of #109 from `main`. Per the HTML spec, `customElements.define` only reads `Class.observedAttributes` if `attributeChangedCallback` is non-null on the prototype at registration time. The previous `first_constructor_static` hook wired ACB at *first instance construction*, which fires after registration — so the browser snapshotted a `null` ACB and never read `observedAttributes`. `setAttribute` after mount silently dropped its update. Fix: make ACB a regular entry in the props plugin's `provides`, so `addPlugins` lands it on `NudeElement.prototype` before any subclass calls `customElements.define`. Subclasses chain via super, matching the convention used for `connectedCallback` / `disconnectedCallback`. The eager static `observedAttributes` getter on `providesStatic` now runs `defineProps()` lazily — that's the registration-time path. The `setup` hook is gated by `!Object.hasOwn(this, observedAttributes)` so it doesn't re-install when the static getter has already done so. Bonus: this also flips "removeAttribute collapses a reflected prop to its default" green — same root cause. Baseline: 91 → 93 pass / 7 → 5 fail / 4 skip. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

DmitrySharabin force-pushed the fix-observed-attributes-pre-construction branch 3 times, most recently from 8e55503 to 7e541a5 Compare May 12, 2026 21:05

DmitrySharabin added 2 commits May 12, 2026 23:10

DmitrySharabin force-pushed the fix-observed-attributes-pre-construction branch 2 times, most recently from b0590be to a630133 Compare May 13, 2026 10:26

DmitrySharabin added 2 commits May 13, 2026 14:52

DmitrySharabin force-pushed the fix-observed-attributes-pre-construction branch from a630133 to 5da1eb1 Compare May 13, 2026 13:13

DmitrySharabin mentioned this pull request May 13, 2026

RFC: registration helper to auto-wrap lifecycle callbacks (follow-up to #98 / #109) #110

Open

LeaVerou merged commit 143fb38 into main May 14, 2026
4 checks passed

LeaVerou deleted the fix-observed-attributes-pre-construction branch May 14, 2026 01:53

DmitrySharabin mentioned this pull request May 15, 2026

Fix setAttribute() after mount not updating reflected props (closes #98) #117

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix `setAttribute()` after mount not updating reflected props (closes #98)#109

Fix `setAttribute()` after mount not updating reflected props (closes #98)#109
LeaVerou merged 6 commits into
mainfrom
fix-observed-attributes-pre-construction

DmitrySharabin commented May 12, 2026 •

edited

Loading

Uh oh!

netlify Bot commented May 12, 2026 •

edited

Loading

Uh oh!

DmitrySharabin commented May 13, 2026 •

edited

Loading

Uh oh!

DmitrySharabin commented May 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

DmitrySharabin commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TL;DR

Why

What

src/plugins/props/index.js

test/util/FakeElement.js

test/Props.js

Known caveat — behavior change vs pre-PR-109

Out of scope

Test plan

Uh oh!

netlify Bot commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for nude-element ready!

Uh oh!

DmitrySharabin commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reduction (same page where nude-element is loaded)

What I verified

Direction

Mea culpa

Uh oh!

DmitrySharabin commented May 13, 2026

What I checked

Note on the approach

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

DmitrySharabin commented May 12, 2026 •

edited

Loading

`src/plugins/props/index.js`

`test/util/FakeElement.js`

`test/Props.js`

netlify Bot commented May 12, 2026 •

edited

Loading

DmitrySharabin commented May 13, 2026 •

edited

Loading