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

[DmitrySharabin]

Closes #98 — setAttribute() after mount didn't update reflected props because attributeChangedCallback wasn't on the prototype at customElements.define time, so the spec never read observedAttributes.

Per Lea's review: NudeElement now owns attributeChangedCallback (in src/element/members.js, alongside connectedCallback / disconnectedCallback) and fires an attribute-changed hook with { name, oldValue, value }. The props plugin listens to that hook instead of overriding ACB — same shape it already uses for connected / disconnected.

Bonus: also flips "removeAttribute collapses a reflected prop to its default" green — same root cause.

Test plan

• npm run test — both regression tests pass: "Post-mount setAttribute updates the property (issue setAttribute() after mount does not update the property on plain reflect props #98)" and "removeAttribute collapses a reflected prop to its default"
• No regressions in unrelated tests

🤖 Generated with Claude Code

[LeaVerou]

What happens when this plugin is used on an element class with its own attributeChangedCallback?

[DmitrySharabin]

It should call super.attributeChangedCallback to make it work correctly. It's a trade-off of the change.

More on this here: #110

[LeaVerou]

That won't work. Try it

[LeaVerou]

How could that happen? There's nothing async here, so nothing will be executed between L88 and L89 (though I haven't checked defineProps, maybe there are async parts)

Also, this doesn't take parent classes into account at all (but maybe we're fixing that later?)

[DmitrySharabin]

How could that happen? There's nothing async here

It's synchronous re-entry, not async. defineProps() synchronously fires this.$hook("define-props", env) (props/index.js:71), and hook listeners run while we're still inside the getter. A hypothetical plugin that hooks define-props and peeks at the attr list before contributing its own:

const hooks = {
  define_props (env) {
    if (this.observedAttributes.includes("foo")) { /* … */ }
  },
};

Without the reservation:

1. Outer read → getter, cache empty, no Object.hasOwn hit.
2. Getter calls defineProps().
3. defineProps fires the define-props hook.
4. The listener above reads Class.observedAttributes.
5. Getter fires again. Cache is still empty (the assignment is after defineProps).
6. Step 5 → step 2 → step 3 → step 4 → step 5 …
7. 💥 stack overflow.

With the reservation, step 4's inner read hits Object.hasOwn and returns the in-flight [] immediately — same re-entry-guard shape as attachShadow in src/plugins/shadow/index.js.

Today no plugin actually hooks define-props, so the guard is purely defensive — but it's a public extension point and a third-party listener would legitimately reach for Class.observedAttributes.

doesn't take parent classes into account at all (but maybe we're fixing that later?)

Right — that's the // FIXME how to combine with existing observedAttributes? on L95. Out of scope for #117; merging derived attrs with user-declared/inherited static observedAttributes is the open question from #109.

[LeaVerou]

It's questionable whether an empty array is better in that case, but it's not a blocking issue

[DmitrySharabin]

After digging into Lea's multi-level inheritance question, I think this PR is fixing the wrong layer.

Lea's pushback holds. Subclass-calls-super works for one level but stack-overflows at depth ≥ 2. Reproduced against the current provides.attributeChangedCallback: two subclasses both override ACB and call super → RangeError: Maximum call stack size exceeded. The same shape applies to connectedCallback, disconnectedCallback, attachShadow, attachInternals — every provides-installed method that uses getSuperMethod(this, provides.foo). The bug isn't unique to ACB; this PR just makes a latent issue visible by adding ACB to the list.

The root cause is structural. User subclasses chain via native super (lexical, homeobject-anchored). Plugin provides methods can't use native super — object literals have no useful homeobject — so they reach for getSuperMethod, which is dynamically anchored on this.constructor.prototype and works correctly only when no subclass also overrides the same method. getSuperMethod is correct for its actual purpose (dynamic mixin composition — chains that aren't known in advance, where home-binding genuinely can't help). It just isn't the right tool for these specific call sites, where the method's install home is known statically.

The right fix is #82. That issue tracks exactly this pain ("doesn't play nicely with inheritance"). The prototype already exists at src/util/unused/composed.js, and the // ...composed({ attributeChangedCallback ... }) placeholder this PR removes was a sketch of the intended API. Finishing composed()/constituents gives plugin authors a declarative idiom for "method installed on a known prototype, compose with same-named methods up and down the chain." getSuperMethod stays available for the dynamic scenarios it was designed for — the two coexist.

Recommendation: close this PR and use #98 as the forcing function for #82. Any tactical patch here — home-anchored helper, drop the super call, registration helper — is throw-away work that gets deleted (or worse, entrenched) when composed() lands. The "subclass must call super" contract specifically has a footgun that doesn't compose at depth; landing it codifies a pattern we'd then have to migrate consumers off.

Concrete path:

1. Settle Nicer API around composed members #82's design — composition order, base-value semantics, interaction with native class methods, and static composition (which also closes #104 for observedAttributes).
2. Implement composed() in xtensible.
3. One nude-element PR migrates the five provides-method call sites — closes setAttribute() after mount does not update the property on plain reflect props #98 / Nicer API around composed members #82, possibly obviates #110.

More work than this PR. But #98 has been open since April; using it to drive the architectural fix is probably the right tradeoff vs. shipping a workaround we'd need to undo.

🤖 Drafted by Claude Code from a full investigation in this session.

[LeaVerou]

I think overridding lifecycle methods is not the answer in general. Nude Element should just call hooks from them, like it does for connected/disconnected etc. ACB is trickier because it has arguments, but not by a lot: we can just pass the arguments to the attribute-changed hook.

[DmitrySharabin]

I think overridding lifecycle methods is not the answer in general. Nude Element should just call hooks from them, like it does for connected/disconnected etc. ACB is trickier because it has arguments, but not by a lot: we can just pass the arguments to the attribute-changed hook.

Done in 55c67b9. attributeChangedCallback moved into src/element/members.js next to connectedCallback / disconnectedCallback; it fires an attribute-changed hook with { name, oldValue, value }. The props plugin now listens via that hook instead of providing its own ACB — same pattern as connected / disconnected.