Skip to content

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

@DmitrySharabin

Description

@DmitrySharabin

Context

This is a follow-up to the discussion on the proposed fix for #98 (PR #109, not yet merged and possibly still subject to change). If the approach in #109 lands roughly as-is, it shifts where attributeChangedCallback lives:

  • Before: ACB was installed inside first_constructor_static, which fires on first instance construction. The hook unconditionally wrapped any existing ACB on the prototype chain, so a subclass that declared its own attributeChangedCallback still got the plugin's reflected-prop dispatch fired automatically.
  • Proposed in Fix setAttribute() after mount not updating reflected props (closes #98) #109: ACB moves to the props plugin's provides, so it lives on NudeElement.prototype and is reachable through the inherited prototype chain at customElements.define time. This is unavoidable for spec correctness — customElements.define reads ACB from the prototype chain at registration, before any first-instance logic runs, and uses it to gate the observedAttributes read.

The side effect of the shift: the previous unconditional auto-wrap goes away. A subclass that overrides attributeChangedCallback would have to call super.attributeChangedCallback(...) to compose with the plugin's reflected-prop dispatch — otherwise post-mount setAttribute would silently stop updating reflected props for that subclass (the exact #98 symptom, isolated to the offending subclass).

This is the worst lifecycle hook to require manual super-chaining. Other hooks (connectedCallback / disconnectedCallback) already have the same explicit-super expectation in this codebase, but their failure modes are less visible — e.g., a connectedCallback override that forgets super means the connected hook doesn't fire, which mostly affects the propchange queue drain on reconnect.

This issue exists to discuss whether a registration helper is the right way to restore auto-wrap without re-introducing the spec-correctness problem.

Proposal

Add a static define() method to NudeElement that subclasses would call instead of customElements.define(name, Class). It would wrap any own-property lifecycle callbacks on the subclass prototype so they compose with the inherited plugin methods, then delegate to native customElements.define.

class MyElement extends NudeElement {
    static props = { v: { type: Number, reflect: true } };

    attributeChangedCallback (name, oldValue, value) {
        console.log("custom logic — no super needed");
    }
}

MyElement.define("my-element");
// vs. customElements.define("my-element", MyElement)
// which would still work but lose the auto-wrap.

Sketch

const WRAPPED = Symbol("nude-element/wrapped");
const LIFECYCLES = [
    "connectedCallback",
    "disconnectedCallback",
    "adoptedCallback",
    "attributeChangedCallback",
];

static define (name, options) {
    for (let cb of LIFECYCLES) {
        let descriptor = Object.getOwnPropertyDescriptor(this.prototype, cb);
        if (!descriptor || typeof descriptor.value !== "function" || descriptor.value[WRAPPED]) {
            continue;
        }
        let userFn = descriptor.value;
        let inheritedFn = Object.getPrototypeOf(this.prototype)[cb];
        let wrapped = function (...args) {
            inheritedFn?.call(this, ...args);
            userFn.call(this, ...args);
        };
        wrapped[WRAPPED] = true;
        Object.defineProperty(this.prototype, cb, { ...descriptor, value: wrapped });
    }
    customElements.define(name, this, options);
}

Why a helper, not implicit auto-wrap

JavaScript class-method syntax installs subclass methods via internal DefineOwnProperty, which bypasses any setter trap that could be installed on the base prototype — so there is no language-level way to intercept "subclass just declared a method" without consumer cooperation. The spec also snapshots attributeChangedCallback at customElements.define time and never re-reads it, so wrapping later (eager getter, first construction, etc.) is invisible to the cached callback. The pre-#109 wrapping landed at first construction — which is precisely why the spec snapshot was already cached with null by then; the auto-wrap was the #98 bug.

A registration helper is the only path that gives auto-wrap + spec correctness, because it interposes at the moment the spec actually reads the prototype.

Open questions

  1. Wrap order — plugin first, then user; or user first, then plugin? Pre-Fix setAttribute() after mount not updating reflected props (closes #98) #109's wrap was plugin-first. Standard JS super.method(); …own logic… is also conventionally super-first. Plugin-first matches both, but worth confirming no scenario benefits from observing pre-plugin state.

  2. Wrap all four lifecycle callbacks, or just attributeChangedCallback? Uniform is one rule to remember; selective is less invasive. Uniform also nudges removing the explicit super.connectedCallback?.() chains scattered across consumers. The WRAPPED sentinel keeps the wrap idempotent so re-registrations don't double-fire.

  3. Conflict with consumer-side static define(). ColorElement.define() in color-elements already wraps customElements.define for dependency resolution and deferred-ready bookkeeping. With this proposal, consumers would either super.define(name) to opt into the wrap, or the wrap would need to install at a layer below the consumer's override.

  4. Co-existence with native customElements.define(name, Class). Native registration should keep working for backward compat. Documentation would need to make clear that the helper is the supported path; using native define keeps the explicit-super contract.

  5. Double-call risk. A consumer who already calls super.attributeChangedCallback(...) in their override AND switches to Class.define(name) would get the plugin's behavior twice (once via the wrap's inheritedFn?.call, once via the user's super.method(...)). Mitigation options: document "don't call super when using Class.define"; add a runtime guard that suppresses re-entry within a single dispatch; or detect a super call statically (not possible without AST inspection — punt).

  6. Multi-level inheritance. Sub extends Mid extends NudeElement, where both Sub and Mid declare own attributeChangedCallback. If Mid was registered via Mid.define("x-mid"), its ACB is wrapped — Sub.define("x-sub")'s wrap captures the wrapped Mid as inheritedFn, chain works. If Mid was registered via native customElements.define, Mid's ACB is unwrapped — the plugin's behavior is bypassed unless Mid manually called super. Edge case; worth deciding whether the helper detects and warns.

  7. Naming. Class.define(name) (mirrors what color-elements already calls it; reads as "register this class") vs NudeElement.define(name, Class) (explicit, dispatched from the base). Leaning toward the former.

Not proposed here

  • Removing the explicit-super expectation from connectedCallback / disconnectedCallback. Those would continue to work as before via standard JS inheritance. The helper would be purely additive.
  • Static auto-magic via a class decorator. JS decorators are still stabilizing and the helper API avoids the moving target.
  • Auto-registration on import. The helper would be invoked explicitly by the consumer.

Also note: this whole proposal is conditional on PR #109 landing in roughly its current shape. If that PR takes a different direction (e.g., the alternative registration-helper approaches the reviewer also suggested on #109), the motivation for this issue may change or disappear.

Filed for discussion, not as a decided plan.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions