From 6468c6ea937411a4e135b5692f8b9b7f122a7333 Mon Sep 17 00:00:00 2001 From: Lea Verou Date: Wed, 27 May 2026 12:32:38 -0400 Subject: [PATCH 01/18] Add batched `propschange` event MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fires once per microtask after a burst of `propchange` events settles, carrying the net first→last delta as a `Map` (round-trips filtered out). Subclasses can define an `updated(event)` method and it's auto-wired, mirroring Lit's `updated(changedProperties)`. `element.props.paused = true` now coalesces writes per-prop during the paused window and dispatches one rebased `propchange` per (event, prop) plus a single `propschange` for the net delta on resume — used by the disconnect/reconnect lifecycle. Mount fires a `propschange` with every prop in `changed` so initial values arrive as a single settled snapshot (`oldValue` is `undefined` for the mount drain). `ElementProp` gains a `source` field (`"default" | "property" | "attribute" | "get" | "convert"`) tracking where the current value came from. User-ownership is derived as `source === "property" || source === "attribute"` at the two callsites that need it; a non-user source (e.g. a `"get"` recompute after a property write) doesn't overwrite a prior user-owned source. `PropChangeEvent` now extends `Event` (not `CustomEvent`) with flat properties — `e.value`, `e.oldValue`, `e.source`, `e.attributeName`, etc. directly on the event — instead of nesting them under `e.detail`. The burst queue holds the event objects themselves keyed by `${eventName}::${name}`; coalescing mutates the queued event's `value`, and `event.target` (set by `dispatchEvent`) doubles as the "has been dispatched" signal. Caveat: the event is reused across dispatches in a burst, so listeners that stash it past their handler will see `value`/ `oldValue` mutate. --- src/plugins/events/onprops.js | 10 +- src/plugins/events/propchange.js | 8 +- src/plugins/props/README.md | 58 ++++++ src/plugins/props/index.js | 10 +- src/plugins/props/util/ElementProp.js | 122 +++++++---- src/plugins/props/util/ElementProps.js | 115 +++++++++-- src/plugins/props/util/PropChangeEvent.js | 78 +++++-- src/plugins/props/util/PropsChangeEvent.js | 21 ++ test/plugins/events/propchange.js | 8 +- test/plugins/props/index.js | 15 +- test/plugins/props/lifecycle.js | 2 +- test/plugins/props/propchange.js | 5 +- test/plugins/props/propschange.js | 223 +++++++++++++++++++++ test/util/dom.js | 5 +- 14 files changed, 589 insertions(+), 91 deletions(-) create mode 100644 src/plugins/props/util/PropsChangeEvent.js create mode 100644 test/plugins/props/propschange.js diff --git a/src/plugins/events/onprops.js b/src/plugins/events/onprops.js index 9a184247..77196f2a 100644 --- a/src/plugins/events/onprops.js +++ b/src/plugins/events/onprops.js @@ -79,14 +79,12 @@ const hooks = { let eventName = this.constructor[eventProps][event.name]; if (eventName) { // Implement onEventName attributes/properties - let change = event.detail; - - if (change.oldValue) { - this.removeEventListener(eventName, change.oldValue); + if (event.oldValue) { + this.removeEventListener(eventName, event.oldValue); } - if (change.value) { - this.addEventListener(eventName, change.value); + if (event.value) { + this.addEventListener(eventName, event.value); } } }); diff --git a/src/plugins/events/propchange.js b/src/plugins/events/propchange.js index a8159efa..3b5c6cff 100644 --- a/src/plugins/events/propchange.js +++ b/src/plugins/events/propchange.js @@ -51,8 +51,12 @@ const hooks = { } let prop = this.props.get(propName); - let detail = { source: "initial", value }; - this.dispatchEvent(new PropChangeEvent(eventName, { name: propName, prop, detail })); + this.dispatchEvent(new PropChangeEvent(eventName, { + name: propName, + prop, + source: "initial", + value, + })); } }, }; diff --git a/src/plugins/props/README.md b/src/plugins/props/README.md index 00c5f90d..3d4ee4c7 100644 --- a/src/plugins/props/README.md +++ b/src/plugins/props/README.md @@ -228,3 +228,61 @@ The `reflect` property takes the following values: - `to`: If `true`, reflect to the attribute with the same name as the prop. If a string, reflect to the attribute with the given name. By default, `reflect` is `true` **unless** `get` is also specified, in which case it defaults to `false`. + +## Reacting to changes + +Two events fire when props change. They have different timings and different shapes — pick the one that matches your use case. + +### `propchange` — fine-grained, synchronous + +Fires once per individual property change, synchronously inside the assignment. Best for per-write side effects (logging, validation, syncing to another store). + +```js +element.addEventListener("propchange", e => { + e.name; // prop name + e.value; // new stored value + e.oldValue; // previous stored value + e.source; // "property" | "attribute" | "get" | "default" | … +}); +``` + +The event object is reused across dispatches within a burst, so `value` and `oldValue` will mutate past your handler if you stash the event. Copy the fields you need. + +Subclasses that define a `propChangedCallback(event)` method are auto-wired to `propchange`. + +### `propschange` — coalesced, microtask-deferred + +Fires once per microtask after a burst of `propchange` events settles. Best for "re-render once after a batch of changes" work — the typical use case for a settled snapshot. + +```js +element.addEventListener("propschange", e => { + e.changed; // Map — net first→last delta across the burst + for (let [name, oldValue] of e.changed) { + let currentValue = this[name]; + // … + } +}); +``` + +Subclasses that define an `updated(event)` method are auto-wired to `propschange`, mirroring Lit's `updated(changedProperties)`. + +```js +class MyElement extends NudeElement { + static props = { /* … */ }; + + updated (event) { + // event.changed is a Map + this.render(); + } +} +``` + +#### Semantics + +- **Mount fires a `propschange`** with every prop in `changed` — initial values arrive as a single settled snapshot. `oldValue` is `undefined` for the mount drain. +- **Round-trips drop out.** Setting a prop and then setting it back to its previous value within the burst produces no entry in `changed`, even though `propchange` fired for each assignment. +- **`oldValue` is the stored previous value**, matching `propchange`'s `e.oldValue`. Resolved defaults are cached on first access, so for a prop sitting at its default, `oldValue` will be that resolved default (not `undefined`). Read `this[name]` inside the handler for the current value. + +### Pausing dispatch + +`element.props.paused = true` holds both `propchange` and `propschange` dispatch. Writes during the paused window are coalesced per property and dispatched as a single rebased `propchange` per (event, prop) on `paused = false`, followed by one `propschange` for the net delta. Used internally for the disconnect/reconnect lifecycle. diff --git a/src/plugins/props/index.js b/src/plugins/props/index.js index 4b7a1541..fe38c860 100644 --- a/src/plugins/props/index.js +++ b/src/plugins/props/index.js @@ -20,9 +20,17 @@ const hooks = { }, constructor () { - if (this.propChangedCallback && this.constructor[props]) { + if (!this.constructor[props]) { + return; + } + + if (this.propChangedCallback) { this.addEventListener("propchange", this.propChangedCallback); } + + if (this.updated) { + this.addEventListener("propschange", this.updated); + } }, connected () { diff --git a/src/plugins/props/util/ElementProp.js b/src/plugins/props/util/ElementProp.js index ef69b671..4abdc213 100644 --- a/src/plugins/props/util/ElementProp.js +++ b/src/plugins/props/util/ElementProp.js @@ -6,17 +6,20 @@ import { resolveValue } from "../util.js"; * per-element behavior (get, set, update, cascade) for that prop. */ export default class ElementProp { - /** - * Current stored value. `undefined` means no explicit value has been set, - * which triggers default resolution on read. - */ + /** Current stored value (cached resolved default, computed result, or explicit user write). */ value; + /** Value prior to the most recent change. */ + oldValue; + /** - * Last value that was different from the current one (i.e. the value - * before the most recent change that passed the equality check). + * Source of the current {@link value}: one of `"default"`, `"property"`, + * `"attribute"`, `"get"`, `"convert"`, or `undefined` before any value lands. + * `"property"` and `"attribute"` mark a user-owned value, which gates the + * defaultProp cascade (see {@link dependsOn}) and the reflect-on-first- + * explicit-write path in {@link set}. */ - oldValue; + source; /** * @param {ElementProps} props The owning per-element collection. @@ -49,10 +52,12 @@ export default class ElementProp { }); } else if (!defaultProp && !computed) { - // Plain prop (value-or-function default, no `get`, no `defaultProp`). - // Computed / defaultProp'd props don't need this — they receive their - // initial value via the cascade from their dependencies. - this.changed({ source: "default" }); + // Plain prop with a value-or-function default. Cache the resolved + // default into this.value so dependents reading it via the cascade + // see a real value, not undefined. + this.value = this.get(); + this.source = "default"; + this.changed({ source: "default", value: this.value, oldValue: undefined }); } } @@ -112,7 +117,7 @@ export default class ElementProp { * @param {{source?: string, name?: string, oldAttributeValue?: string | null}} [options] */ set (value, { source, name, oldAttributeValue } = {}) { - let { spec, element } = this; + let { spec } = this; let parsed; try { @@ -136,10 +141,30 @@ export default class ElementProp { parsed = this.convert(parsed); + let isUserWrite = source === "property" || source === "attribute"; + + let wasUserOwned = this.source === "property" || this.source === "attribute"; + if (spec.equals(parsed, this.value)) { + // Same value. First explicit write of the cached default still needs + // to reflect to the attribute (issue #105) and take ownership so the + // defaultProp cascade stops tracking us, but no propchange fires — + // nothing actually changed. + if (isUserWrite && !wasUserOwned) { + this.source = source; + if (source === "property") { + this.#reflectToAttribute(parsed); + } + } return; } + // Don't let a non-user source (e.g. a "get" recompute) silently strip + // user ownership established by a prior property/attribute write. + if (isUserWrite || !wasUserOwned) { + this.source = source; + } + this.oldValue = this.value; this.value = parsed; @@ -149,24 +174,10 @@ export default class ElementProp { oldValue: this.oldValue, }; - if (source === "property" && spec.reflect.to) { - let attributeName = spec.reflect.to; - let attributeValue = spec.stringify(parsed); - let oldAttributeValue = element.getAttribute(attributeName); - - if (oldAttributeValue !== attributeValue) { - // TODO what if another prop is reflected *from* this attribute? - Object.assign(change, { attributeName, attributeValue, oldAttributeValue }); - - let ignored = this.props.ignoredAttributes; - ignored.add(attributeName); - if (attributeValue === null) { - element.removeAttribute(attributeName); - } - else { - element.setAttribute(attributeName, attributeValue); - } - ignored.delete(attributeName); + if (source === "property") { + let reflected = this.#reflectToAttribute(parsed); + if (reflected) { + Object.assign(change, reflected); } } else if (source === "attribute") { @@ -180,6 +191,41 @@ export default class ElementProp { this.changed(change); } + /** + * Mirror a property write to its `reflect.to` attribute, guarded against + * the resulting attributeChangedCallback echo. No-op when reflection is + * disabled or the attribute already has the stringified value. + * + * TODO what if another prop is reflected *from* this attribute? + * @returns {{attributeName: string, attributeValue: string | null, oldAttributeValue: string | null} | null} + * Mirror metadata when the attribute changed, otherwise `null`. + */ + #reflectToAttribute (parsed) { + let { spec, element } = this; + let attributeName = spec.reflect.to; + if (!attributeName) { + return null; + } + + let attributeValue = spec.stringify(parsed); + let oldAttributeValue = element.getAttribute(attributeName); + if (oldAttributeValue === attributeValue) { + return null; + } + + let ignored = this.props.ignoredAttributes; + ignored.add(attributeName); + if (attributeValue === null) { + element.removeAttribute(attributeName); + } + else { + element.setAttribute(attributeName, attributeValue); + } + ignored.delete(attributeName); + + return { attributeName, attributeValue, oldAttributeValue }; + } + /** * Apply the spec's `convert` hook, or pass the value through if none. * @param {*} value @@ -206,9 +252,12 @@ export default class ElementProp { let { spec } = this; if (dependency && dependency.spec === spec.defaultProp) { - // The default prop changed; the resolved default may differ, but since - // we have no value of our own to update, just notify listeners. - this.changed({ source: "default" }); + let oldValue = this.value; + this.value = dependency.value === undefined + ? undefined + : this.convert(spec.parse(dependency.value)); + this.source = "default"; + this.changed({ source: "default", value: this.value, oldValue }); return; } @@ -224,7 +273,7 @@ export default class ElementProp { /** * Whether this prop currently depends on `dep`'s value. - * Includes the default-prop link only while this prop has no explicit value. + * Includes the default-prop link only while this prop is not user-owned. * @param {ElementProp} dep * @returns {boolean} */ @@ -237,10 +286,11 @@ export default class ElementProp { return true; } - let { spec } = this; + let { spec, source } = this; + let userOwned = source === "property" || source === "attribute"; return ( spec.dependencies.has(dep.name) || - (spec.defaultProp === dep.spec && this.value === undefined) + (spec.defaultProp === dep.spec && !userOwned) ); } } diff --git a/src/plugins/props/util/ElementProps.js b/src/plugins/props/util/ElementProps.js index adb49ed5..fb222c78 100644 --- a/src/plugins/props/util/ElementProps.js +++ b/src/plugins/props/util/ElementProps.js @@ -1,20 +1,20 @@ import { symbols } from "xtensible"; import ElementProp from "./ElementProp.js"; import PropChangeEvent from "./PropChangeEvent.js"; +import PropsChangeEvent from "./PropsChangeEvent.js"; const { props } = symbols.known; /** * Per-element collection of {@link ElementProp} wrappers. - * Owns per-element state: stored values, paused/queued events, and attribute - * echo suppression. */ export default class ElementProps extends Map { #paused = true; /** * Whether propchange-style event dispatch is currently held. While `true`, - * fired events are queued; flipping to `false` flushes the queue in order. + * fired events are coalesced per (eventName, propName) in the burst queue; + * flipping to `false` dispatches every undispatched event in queue order. */ get paused () { return this.#paused; @@ -29,26 +29,39 @@ export default class ElementProps extends Map { this.#paused = value; if (!value) { - let queue = this.#eventQueue; - this.#eventQueue = []; - for (let event of queue) { - this.element.dispatchEvent(event); + for (let event of this.#eventQueue.values()) { + // Stale consumer view: oldValue ≠ value means we haven't told + // the consumer about the current value yet. + if (!event.prop.spec.equals(event.oldValue, event.value)) { + this.element.dispatchEvent(event); + // Rebase: consumer now knows about the current value. + event.oldValue = event.value; + } + } + + // Drains scheduled while paused exited early without firing + // propschange. Queue a fresh one so the bunched event still fires + // after the resume flush settles. + if (this.#eventQueue.size > 0) { + queueMicrotask(() => this.#drain()); } } } - /** - * Attributes currently being written reflexively by an {@link ElementProp}; - * the resulting attributeChangedCallback should be ignored. - * @type {Set} - */ + /** @type {Set} Attributes mid-reflective-write; their ACB should be ignored. */ ignoredAttributes = new Set(); /** - * Queue of propchange-style events awaiting dispatch while paused. - * @type {PropChangeEvent[]} + * Coalesced propchange-style events keyed by `${eventName}::${propName}`. + * Each entry is the event that would be dispatched right now for that + * (event, prop) pair: `oldValue` is what was last told to the consumer + * (or the burst-start value if never dispatched), `value` is the current + * stored value, and `firstOldValue` is the sticky burst-start value used + * by the `propschange` drain. `event.target` is non-null once the event + * has been dispatched at least once. + * @type {Map} */ - #eventQueue = []; + #eventQueue = new Map(); /** * Construct the per-element collection and run the mount-time init pass. @@ -147,7 +160,7 @@ export default class ElementProps extends Map { this.#firePropChangeEvent(eventName, { name: ep.name, prop: ep, - detail: change, + ...change, }); } @@ -164,18 +177,76 @@ export default class ElementProps extends Map { } /** - * Dispatch (or queue, if paused) a propchange-style event. + * Coalesce a propchange-style event into the burst queue and, when unpaused, + * dispatch it synchronously. Reuses the queued event object across + * dispatches in the same burst — `value` updates to the latest stored + * value and `oldValue` rebases to what the consumer was last told. + * * @param {string} eventName - * @param {{name: string, prop: ElementProp, detail: Object}} eventProps + * @param {{name: string, prop: ElementProp, value: *, oldValue: *, source: string, attributeName?: string, attributeValue?: string | null, oldAttributeValue?: string | null}} eventProps */ #firePropChangeEvent (eventName, eventProps) { - let event = new PropChangeEvent(eventName, eventProps); - - if (this.#paused) { - this.#eventQueue.push(event); + let key = `${eventName}::${eventProps.name}`; + let event = this.#eventQueue.get(key); + + if (event) { + // Coalesce: keep firstOldValue + oldValue (last-told), update value. + event.value = eventProps.value; + // delete + set moves the entry to the end of insertion order. + this.#eventQueue.delete(key); } else { + event = new PropChangeEvent(eventName, eventProps); + } + + if (!this.#paused) { this.element.dispatchEvent(event); + // Rebase: consumer now knows about the current value. + event.oldValue = event.value; + } + + let { spec } = event.prop; + // Drop only when both axes are zero: nothing left to tell the + // consumer AND the burst's net effect is zero. If the consumer was + // told an intermediate value, the entry stays so resume can dispatch + // the revert (propschange still skips it — #drain filters by firstOldValue). + if ( + spec.equals(event.oldValue, event.value) + && spec.equals(event.value, event.firstOldValue) + ) { + return; + } + + this.#eventQueue.set(key, event); + queueMicrotask(() => this.#drain()); + } + + /** + * Build the net change map from the burst queue and fire `propschange`. + * No-op while paused or after another drain already cleared the queue — + * the {@link paused} setter queues a fresh drain on resume. + */ + #drain () { + if (this.#paused) { + return; + } + + let changed = new Map(); + for (let event of this.#eventQueue.values()) { + // Alias eventNames are listener-side conveniences; only canonical + // propchange feeds the bunch. Net-zero entries are dropped — they + // only exist so resume could dispatch a revert to the consumer. + if ( + event.type === "propchange" + && !event.prop.spec.equals(event.value, event.firstOldValue) + ) { + changed.set(event.name, event.firstOldValue); + } + } + this.#eventQueue.clear(); + + if (changed.size > 0) { + this.element.dispatchEvent(new PropsChangeEvent("propschange", { changed })); } } } diff --git a/src/plugins/props/util/PropChangeEvent.js b/src/plugins/props/util/PropChangeEvent.js index 9a6314f0..71d95371 100644 --- a/src/plugins/props/util/PropChangeEvent.js +++ b/src/plugins/props/util/PropChangeEvent.js @@ -1,9 +1,65 @@ -export default class PropChangeEvent extends CustomEvent { - constructor (type, { name, prop, ...options } = {}) { +/** + * Per-prop change event. Fires synchronously inside a property/attribute + * write, then again on resume after a paused burst settles. Fields are + * direct properties — no `detail` wrapper. + * + * The same event object is reused across dispatches within a burst: `value` + * tracks the latest stored value and `oldValue` rebases to whatever was last + * dispatched, so listeners that stash the event will see those fields mutate + * past their handler. Copy the fields if you need a snapshot. + */ +export default class PropChangeEvent extends Event { + /** @type {string} Prop name. */ + name; + + /** @type {import("./ElementProp.js").default} */ + prop; + + /** @type {*} Current stored value (parsed + converted). */ + value; + + /** + * Value the consumer was last told about: the stored value before this + * change for a fresh dispatch, or the previously-dispatched value when + * the event is re-dispatched after a paused burst. + * @type {*} + */ + oldValue; + + /** @type {"default" | "property" | "attribute" | "get" | "convert"} */ + source; + + /** @type {string | undefined} Set when `source === "attribute"` or a property write reflects to an attribute. */ + attributeName; + + /** @type {string | null | undefined} */ + attributeValue; + + /** @type {string | null | undefined} */ + oldAttributeValue; + + /** + * Stored value before the first change in the current burst. Stays sticky + * across rebasing of {@link oldValue} so the matching `propschange` drain + * can compute the net first→last delta. + * @type {*} + */ + firstOldValue; + + constructor (type, options = {}) { + // Event picks out its init-dict keys (bubbles, cancelable, composed) + // and silently ignores the rest. super(type, options); - this.name = name; - this.prop = prop; + // Assign the rest as own properties, skipping anything Event already + // owns (init dict, state like `target`/`defaultPrevented`, methods). + for (let [key, value] of Object.entries(options)) { + if (!(key in Event.prototype)) { + this[key] = value; + } + } + + this.firstOldValue ??= this.oldValue; } /** @@ -12,18 +68,16 @@ export default class PropChangeEvent extends CustomEvent { * @param {Element} target Element to apply the change to. */ applyTo (target) { - let { source, attributeName, attributeValue, value } = this.detail; - - if (source === "attribute") { - if (attributeValue === null) { - target.removeAttribute(attributeName); + if (this.source === "attribute") { + if (this.attributeValue === null) { + target.removeAttribute(this.attributeName); } else { - target.setAttribute(attributeName, attributeValue); + target.setAttribute(this.attributeName, this.attributeValue); } } - else if (source === "property") { - target[this.name] = value; + else if (this.source === "property") { + target[this.name] = this.value; } } } diff --git a/src/plugins/props/util/PropsChangeEvent.js b/src/plugins/props/util/PropsChangeEvent.js new file mode 100644 index 00000000..b7505e45 --- /dev/null +++ b/src/plugins/props/util/PropsChangeEvent.js @@ -0,0 +1,21 @@ +/** + * Event fired after a burst of {@link PropChangeEvent}s settles within a microtask. + * Carries the net first→last delta as a `changed` Map of `name → oldValue`. + * Current values are read via `this[name]` inside the handler. + * + * `oldValue` is the stored internal value (parsed + converted) before the + * first change in the burst, mirroring `propchange`'s `e.oldValue`. + */ +export default class PropsChangeEvent extends Event { + /** + * Net change map: prop name → the value before the first change in the burst. + * Round-trips (current value equals the recorded old value) are filtered out. + * @type {Map} + */ + changed; + + constructor (type, { changed, ...options } = {}) { + super(type, options); + this.changed = changed; + } +} diff --git a/test/plugins/events/propchange.js b/test/plugins/events/propchange.js index 50316b67..023a2742 100644 --- a/test/plugins/events/propchange.js +++ b/test/plugins/events/propchange.js @@ -7,7 +7,7 @@ export default { tests: [ { - name: "first_connected shortcut re-fire carries a detail", + name: "first_connected shortcut re-fire carries the value", description: "Late-bound on*= handlers only see the catch-up re-fire (issue #106)", run () { let tag = defineElement({ @@ -19,15 +19,15 @@ export default { // Declared via markup so the on*= handler binds late, like real // consumer usage — it then sees only the first_connected catch-up. let host = document.createElement("div"); - host.innerHTML = `<${tag} onvaluechange="(this._log ??= []).push(event.detail)">`; + host.innerHTML = `<${tag} onvaluechange="(this._log ??= []).push({value: event.value, source: event.source})">`; document.body.append(host); let element = host.firstElementChild; host.remove(); - return (element._log ?? []).map(detail => Object.keys(detail ?? {}).length > 0); + return element._log ?? []; }, - expect: [true], + expect: [{ value: "x", source: "initial" }], }, ], }; diff --git a/test/plugins/props/index.js b/test/plugins/props/index.js index ff8afcb1..5ad55842 100644 --- a/test/plugins/props/index.js +++ b/test/plugins/props/index.js @@ -4,6 +4,7 @@ import reflection from "./reflection.js"; import defaults from "./defaults.js"; import computed from "./computed.js"; import propchange from "./propchange.js"; +import propschange from "./propschange.js"; import lifecycle from "./lifecycle.js"; import inheritance from "./inheritance.js"; import install from "./install.js"; @@ -17,8 +18,8 @@ export default { name: "Behavior", beforeEach () { - let { props, attributes } = this.arg; - let tag = defineElement({ plugins: [propsPlugin], props }); + let { props, attributes, mixin } = this.arg; + let tag = defineElement({ plugins: [propsPlugin], props, mixin }); let element = document.createElement(tag); let events = []; @@ -27,20 +28,26 @@ export default { element.addEventListener("propchange", e => events.push([e.name, e.target[e.name]])); + // Snapshot as [name, oldValue] tuples so tests can compare with + // plain array equality. + let propsEvents = []; + element.addEventListener("propschange", e => + propsEvents.push([...e.changed])); + for (let [name, value] of Object.entries(attributes ?? {})) { element.setAttribute(name, value); } document.body.append(element); - Object.assign(this.data, { element, events }); + Object.assign(this.data, { element, events, propsEvents }); }, afterEach () { this.data.element.remove(); }, - tests: [reflection, defaults, computed, propchange, lifecycle], + tests: [reflection, defaults, computed, propchange, propschange, lifecycle], }, inheritance, install, diff --git a/test/plugins/props/lifecycle.js b/test/plugins/props/lifecycle.js index df7ec396..94acb091 100644 --- a/test/plugins/props/lifecycle.js +++ b/test/plugins/props/lifecycle.js @@ -31,7 +31,7 @@ export default { run () { let { element } = this.data; let log = []; - element.addEventListener("propchange", e => log.push([e.name, e.detail.value])); + element.addEventListener("propchange", e => log.push([e.name, e.value])); element.a = 1; // fires immediately let beforePause = log.length; diff --git a/test/plugins/props/propchange.js b/test/plugins/props/propchange.js index bdcb7da7..28953d6e 100644 --- a/test/plugins/props/propchange.js +++ b/test/plugins/props/propchange.js @@ -145,7 +145,7 @@ export default { run () { let { element } = this.data; let log = []; - element.addEventListener("propchange", e => log.push([e.name, e.detail.value])); + element.addEventListener("propchange", e => log.push([e.name, e.value])); element.v = "foo"; element.v = "bar"; @@ -162,6 +162,8 @@ export default { }, { name: "Writes that collapse to the same value after convert do not fire propchange", + description: + "v has no default — its mount-time state is genuinely undefined, so no mount propchange fires. Only the first write that produces a new floored value triggers an event.", run () { let { element } = this.data; element.v = 5; @@ -180,7 +182,6 @@ export default { }, }, expect: [ - ["v", undefined], ["v", 5], // the 5.4 and 5.9 writes do not fire ], diff --git a/test/plugins/props/propschange.js b/test/plugins/props/propschange.js new file mode 100644 index 00000000..c0d49ecd --- /dev/null +++ b/test/plugins/props/propschange.js @@ -0,0 +1,223 @@ +/** + * `propschange` is dispatched once per microtask after a burst of `propchange` + * events settles. Tests await `Promise.resolve()` (or a microtask) to let the + * drain run before asserting. + */ +const flush = () => Promise.resolve(); + +export default { + name: "propschange events", + + tests: [ + { + name: "Mount fires one propschange with every prop → undefined", + async run () { + await flush(); + return this.data.propsEvents; + }, + arg: { + props: { + a: { type: Number, default: 1 }, + b: { type: String, default: "x" }, + }, + }, + expect: [ + [ + ["a", undefined], + ["b", undefined], + ], + ], + }, + { + name: "Three writes to the same prop collapse to one propschange entry", + description: + "oldValue is the stored previous value (the cached resolved default 0 here), mirroring propchange's e.oldValue.", + async run () { + let { element } = this.data; + await flush(); // drain mount-time propschange first + let mountCount = this.data.propsEvents.length; + + element.v = 1; + element.v = 2; + element.v = 3; + + await flush(); + return this.data.propsEvents.slice(mountCount); + }, + arg: { props: { v: { type: Number, default: 0 } } }, + expect: [[["v", 0]]], + }, + { + name: "Round-trip drops out: x=stored; x=stored fires propchange twice, no propschange", + description: + "Round-trip is detected against the stored previous value. With no explicit prior write, the stored old is undefined — so a write to a non-undefined value then back to undefined would be the round-trip, not back to the default.", + async run () { + let { element } = this.data; + element.v = 1; // establish a stored value via property write + await flush(); + let propchangeCountBefore = this.data.events.length; + let propschangeCountBefore = this.data.propsEvents.length; + + element.v = 2; + element.v = 1; // back to stored value + + await flush(); + return { + propchangeAfter: this.data.events.length - propchangeCountBefore, + propschangeAfter: this.data.propsEvents.length - propschangeCountBefore, + }; + }, + arg: { props: { v: { type: Number, default: 0 } } }, + expect: { propchangeAfter: 2, propschangeAfter: 0 }, + }, + { + name: "Cascade: dependents land in the same propschange", + description: + "Both plain and computed cache their mount-time values, so oldValue is the resolved default for each.", + async run () { + let { element } = this.data; + await flush(); + let mountCount = this.data.propsEvents.length; + + element.plain = 5; + + await flush(); + return this.data.propsEvents.slice(mountCount); + }, + arg: { + props: { + plain: { type: Number, default: 1 }, + computed: { + get () { + return this.plain + 10; + }, + }, + }, + }, + expect: [ + [ + ["plain", 1], + ["computed", 11], + ], + ], + }, + { + name: "Disconnect/reconnect: revert propchange dispatched, no propschange (dispatched round-trip)", + description: + "v=2 sync; disconnect; v=1 (the burst's stored old); reconnect → consumer hears the revert via propchange, but propschange's net effect is zero.", + async run () { + let { element } = this.data; + element.v = 1; // establish a stored value + await flush(); + let propchangeLog = []; + element.addEventListener("propchange", e => + propchangeLog.push([e.name, e.oldValue, e.value])); + let propschangeCountBefore = this.data.propsEvents.length; + + element.v = 2; // connected: dispatched immediately + element.remove(); // pause + element.v = 1; // back to stored old while paused + document.body.append(element); // resume + + await flush(); + return { + propchanges: propchangeLog, + propschangesAfter: this.data.propsEvents.length - propschangeCountBefore, + }; + }, + arg: { props: { v: { type: Number, default: 0 } } }, + expect: { + propchanges: [ + ["v", 1, 2], + ["v", 2, 1], + ], + propschangesAfter: 0, + }, + }, + { + name: "Paused burst: one rebased propchange + one propschange on resume", + async run () { + let { element } = this.data; + element.v = 1; // establish a stored old value before the burst + await flush(); + let propchangeLog = []; + element.addEventListener("propchange", e => + propchangeLog.push([e.name, e.oldValue, e.value])); + let propschangeCountBefore = this.data.propsEvents.length; + + element.props.paused = true; + element.v = 2; + element.v = 3; + element.v = 4; + element.props.paused = false; + + await flush(); + return { + propchanges: propchangeLog, + propschange: this.data.propsEvents.slice(propschangeCountBefore), + }; + }, + arg: { props: { v: { type: Number, default: 0 } } }, + expect: { + propchanges: [["v", 1, 4]], + propschange: [[["v", 1]]], + }, + }, + { + name: "updated() method auto-wires to propschange", + async run () { + await flush(); + let { element } = this.data; + let updates = element.constructor.__updates; + element.v = 5; + await flush(); + return updates.map(changed => [...changed]); + }, + arg: { + props: { v: { type: Number, default: 0 } }, + mixin (Class) { + Class.__updates = []; + Class.prototype.updated = function (event) { + this.constructor.__updates.push(event.changed); + }; + }, + }, + expect: [ + [["v", undefined]], + [["v", 0]], + ], + }, + { + name: "Per-event-name aliases dedup independently and don't feed propschange", + async run () { + let { element } = this.data; + element.v = 1; // stored old before the burst + let aliasLog = []; + element.addEventListener("valuechange", e => + aliasLog.push([e.name, e.oldValue, e.value])); + await flush(); + let propschangeCountBefore = this.data.propsEvents.length; + + element.props.paused = true; + element.v = 2; + element.v = 3; + element.props.paused = false; + + await flush(); + return { + alias: aliasLog, + propschange: this.data.propsEvents.slice(propschangeCountBefore), + }; + }, + arg: { + props: { + v: { type: Number, default: 0, eventNames: ["valuechange"] }, + }, + }, + expect: { + alias: [["v", 1, 3]], + propschange: [[["v", 1]]], + }, + }, + ], +}; diff --git a/test/util/dom.js b/test/util/dom.js index 5bfabfee..812fcfd0 100644 --- a/test/util/dom.js +++ b/test/util/dom.js @@ -6,11 +6,14 @@ let counter = 0; * Define a one-off custom element for tests; returns the tag name. * `plugins` composes the element class; every other key becomes a static class * member (`props`, `states`, `events`, …) for the matching plugin to read. + * `mixin` (optional) is called with the class so tests can install instance + * methods (e.g. `updated`, `propChangedCallback`) before `customElements.define`. */ -export function defineElement ({ plugins, ...statics } = {}) { +export function defineElement ({ plugins, mixin, ...statics } = {}) { let tag = `nude-element-${counter++}`; let Class = class extends ElementFactory(HTMLElement, plugins) {}; Object.assign(Class, statics); + mixin?.(Class); customElements.define(tag, Class); return tag; } From defe4859f4984192358f43abd17c654cd94a3309 Mon Sep 17 00:00:00 2001 From: Lea Verou Date: Wed, 27 May 2026 13:31:50 -0400 Subject: [PATCH 02/18] Extract PropChangeEvent / PropsChangeEvent prop typedefs Replaces per-field `@type` declarations and the inline object type on `ElementProps#firePropChangeEvent` with shared JSDoc typedefs. --- src/plugins/props/util/ElementProps.js | 4 +- src/plugins/props/util/PropChangeEvent.js | 60 +++++++++------------- src/plugins/props/util/PropsChangeEvent.js | 18 +++++-- 3 files changed, 41 insertions(+), 41 deletions(-) diff --git a/src/plugins/props/util/ElementProps.js b/src/plugins/props/util/ElementProps.js index fb222c78..61123f2a 100644 --- a/src/plugins/props/util/ElementProps.js +++ b/src/plugins/props/util/ElementProps.js @@ -3,6 +3,8 @@ import ElementProp from "./ElementProp.js"; import PropChangeEvent from "./PropChangeEvent.js"; import PropsChangeEvent from "./PropsChangeEvent.js"; +/** @import { PropChangeEventProps } from "./PropChangeEvent.js" */ + const { props } = symbols.known; /** @@ -183,7 +185,7 @@ export default class ElementProps extends Map { * value and `oldValue` rebases to what the consumer was last told. * * @param {string} eventName - * @param {{name: string, prop: ElementProp, value: *, oldValue: *, source: string, attributeName?: string, attributeValue?: string | null, oldAttributeValue?: string | null}} eventProps + * @param {PropChangeEventProps} eventProps */ #firePropChangeEvent (eventName, eventProps) { let key = `${eventName}::${eventProps.name}`; diff --git a/src/plugins/props/util/PropChangeEvent.js b/src/plugins/props/util/PropChangeEvent.js index 71d95371..b32ef9a9 100644 --- a/src/plugins/props/util/PropChangeEvent.js +++ b/src/plugins/props/util/PropChangeEvent.js @@ -1,3 +1,24 @@ +/** + * @typedef {object} PropChangeEventProps + * @property {string} name Prop name. + * @property {import("./ElementProp.js").default} prop + * @property {*} value Current stored value (parsed + converted). + * @property {*} oldValue Value the consumer was last told about: the stored + * value before this change for a fresh dispatch, or the previously-dispatched + * value when the event is re-dispatched after a paused burst. + * @property {"default" | "property" | "attribute" | "get" | "convert"} source + * @property {string} [attributeName] Set when `source === "attribute"` or a + * property write reflects to an attribute. + * @property {string | null} [attributeValue] + * @property {string | null} [oldAttributeValue] + * @property {*} [firstOldValue] Stored value before the first change in the + * current burst. Stays sticky across rebasing of `oldValue` so the matching + * `propschange` drain can compute the net first→last delta. Defaults to + * `oldValue` on construction. + * + * @typedef {EventInit & PropChangeEventProps} PropChangeEventInit + */ + /** * Per-prop change event. Fires synchronously inside a property/attribute * write, then again on resume after a paused burst settles. Fields are @@ -7,45 +28,14 @@ * tracks the latest stored value and `oldValue` rebases to whatever was last * dispatched, so listeners that stash the event will see those fields mutate * past their handler. Copy the fields if you need a snapshot. + * + * @implements {PropChangeEventProps} */ export default class PropChangeEvent extends Event { - /** @type {string} Prop name. */ - name; - - /** @type {import("./ElementProp.js").default} */ - prop; - - /** @type {*} Current stored value (parsed + converted). */ - value; - /** - * Value the consumer was last told about: the stored value before this - * change for a fresh dispatch, or the previously-dispatched value when - * the event is re-dispatched after a paused burst. - * @type {*} + * @param {string} type + * @param {PropChangeEventInit} options */ - oldValue; - - /** @type {"default" | "property" | "attribute" | "get" | "convert"} */ - source; - - /** @type {string | undefined} Set when `source === "attribute"` or a property write reflects to an attribute. */ - attributeName; - - /** @type {string | null | undefined} */ - attributeValue; - - /** @type {string | null | undefined} */ - oldAttributeValue; - - /** - * Stored value before the first change in the current burst. Stays sticky - * across rebasing of {@link oldValue} so the matching `propschange` drain - * can compute the net first→last delta. - * @type {*} - */ - firstOldValue; - constructor (type, options = {}) { // Event picks out its init-dict keys (bubbles, cancelable, composed) // and silently ignores the rest. diff --git a/src/plugins/props/util/PropsChangeEvent.js b/src/plugins/props/util/PropsChangeEvent.js index b7505e45..1cd0b2fe 100644 --- a/src/plugins/props/util/PropsChangeEvent.js +++ b/src/plugins/props/util/PropsChangeEvent.js @@ -1,3 +1,12 @@ +/** + * @typedef {object} PropsChangeEventProps + * @property {Map} changed Net change map: prop name → the value + * before the first change in the burst. Round-trips (current value equals + * the recorded old value) are filtered out. + * + * @typedef {EventInit & PropsChangeEventProps} PropsChangeEventInit + */ + /** * Event fired after a burst of {@link PropChangeEvent}s settles within a microtask. * Carries the net first→last delta as a `changed` Map of `name → oldValue`. @@ -5,15 +14,14 @@ * * `oldValue` is the stored internal value (parsed + converted) before the * first change in the burst, mirroring `propchange`'s `e.oldValue`. + * + * @implements {PropsChangeEventProps} */ export default class PropsChangeEvent extends Event { /** - * Net change map: prop name → the value before the first change in the burst. - * Round-trips (current value equals the recorded old value) are filtered out. - * @type {Map} + * @param {string} type + * @param {PropsChangeEventInit} options */ - changed; - constructor (type, { changed, ...options } = {}) { super(type, options); this.changed = changed; From df9be1240a1227313a4914c86b3265d8470240d9 Mon Sep 17 00:00:00 2001 From: Lea Verou Date: Wed, 27 May 2026 13:43:39 -0400 Subject: [PATCH 03/18] Move propchange alias dispatch into events/propchange plugin Aliases (events declared as `{eventName: {propchange: propName}}`) used to piggyback on ElementProps via a `prop.eventNames` array, with ElementProps looping over `["propchange", ...eventNames]` and keying its coalescing queue on `${eventName}::${propName}`. That cross-plugin seam complicated the props core for one specific consumer. Aliases now live entirely in the events/propchange plugin: a `propchange` listener installed in `first_connected` re-dispatches each alias whenever the canonical event fires, inheriting coalescing and pause/resume for free. ElementProps drops `eventNames`, the type guard in `#drain`, and the composite key (now just the prop name); `#firePropChangeEvent` no longer needs the event-name parameter. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/plugins/events/propchange.js | 74 +++++++++++++++++--------- src/plugins/props/util/ElementProps.js | 45 +++++++--------- src/plugins/props/util/Prop.js | 1 - test/plugins/events/propchange.js | 41 ++++++++++++++ test/plugins/props/propschange.js | 32 ----------- 5 files changed, 109 insertions(+), 84 deletions(-) diff --git a/src/plugins/events/propchange.js b/src/plugins/events/propchange.js index 3b5c6cff..67892c22 100644 --- a/src/plugins/events/propchange.js +++ b/src/plugins/events/propchange.js @@ -18,32 +18,56 @@ const hooks = { return; } - let propchangeEvents = Object.entries(this[events]) - .filter(([name, options]) => options.propchange) - .map(([eventName, options]) => [eventName, options.propchange]); + // Invert the user's `{eventName: {propchange: propName}}` declaration + // into `{propName: [eventName, ...]}` — the shape we need at dispatch + // time, when we have the prop name and want every alias that fires for it. + let aliases = {}; + for (let [eventName, options] of Object.entries(this[events])) { + if (!options.propchange) { + continue; + } - if (propchangeEvents.length > 0) { - // Shortcut for events that fire when a specific prop changes - this[propchange] = Object.fromEntries(propchangeEvents); + let propName = options.propchange; + if (!this[props].get(propName)) { + throw new TypeError(`No prop named ${propName} in ${this.name}`); + } - for (let eventName in this[propchange]) { - let propName = this[propchange][eventName]; - let prop = this[props].get(propName); + (aliases[propName] ??= []).push(eventName); + } - if (prop) { - (prop.eventNames ??= []).push(eventName); - } - else { - throw new TypeError(`No prop named ${propName} in ${this.name}`); - } - } + if (Object.keys(aliases).length > 0) { + this[propchange] = aliases; } }, first_connected () { + let aliases = this.constructor[propchange]; + if (!aliases) { + return; + } + + // Re-dispatch every propchange as its declared alias event(s). The + // canonical event already inherits coalescing / pause-resume from + // ElementProps, so the alias rides along for free. + this.addEventListener("propchange", event => { + let aliasNames = aliases[event.name]; + if (!aliasNames) { + return; + } + + for (let aliasName of aliasNames) { + this.dispatchEvent(new PropChangeEvent(aliasName, { + name: event.name, + prop: event.prop, + source: event.source, + value: event.value, + oldValue: event.oldValue, + })); + } + }); + // Often propchange events have already fired by the time the event handlers are added - for (let eventName in this.constructor[propchange]) { - let propName = this.constructor[propchange][eventName]; + for (let propName in aliases) { let value = this[propName]; if (value === undefined) { @@ -51,12 +75,14 @@ const hooks = { } let prop = this.props.get(propName); - this.dispatchEvent(new PropChangeEvent(eventName, { - name: propName, - prop, - source: "initial", - value, - })); + for (let aliasName of aliases[propName]) { + this.dispatchEvent(new PropChangeEvent(aliasName, { + name: propName, + prop, + source: "initial", + value, + })); + } } }, }; diff --git a/src/plugins/props/util/ElementProps.js b/src/plugins/props/util/ElementProps.js index 61123f2a..6c568881 100644 --- a/src/plugins/props/util/ElementProps.js +++ b/src/plugins/props/util/ElementProps.js @@ -54,13 +54,12 @@ export default class ElementProps extends Map { ignoredAttributes = new Set(); /** - * Coalesced propchange-style events keyed by `${eventName}::${propName}`. - * Each entry is the event that would be dispatched right now for that - * (event, prop) pair: `oldValue` is what was last told to the consumer - * (or the burst-start value if never dispatched), `value` is the current - * stored value, and `firstOldValue` is the sticky burst-start value used - * by the `propschange` drain. `event.target` is non-null once the event - * has been dispatched at least once. + * Coalesced `propchange` events keyed by prop name. Each entry is the + * event that would be dispatched right now for that prop: `oldValue` is + * what was last told to the consumer (or the burst-start value if never + * dispatched), `value` is the current stored value, and `firstOldValue` + * is the sticky burst-start value used by the `propschange` drain. + * `event.target` is non-null once the event has been dispatched at least once. * @type {Map} */ #eventQueue = new Map(); @@ -157,14 +156,11 @@ export default class ElementProps extends Map { */ propChanged (ep, change) { // Source-first: fire before cascading so listeners hear the written prop first. - let eventNames = ["propchange", ...(ep.spec.eventNames ?? [])]; - for (let eventName of eventNames) { - this.#firePropChangeEvent(eventName, { - name: ep.name, - prop: ep, - ...change, - }); - } + this.#firePropChangeEvent({ + name: ep.name, + prop: ep, + ...change, + }); // Update all props that depend on this one. {@link get} materializes a // dependent that hasn't been wrapped yet (e.g. during the constructor's @@ -179,16 +175,15 @@ export default class ElementProps extends Map { } /** - * Coalesce a propchange-style event into the burst queue and, when unpaused, + * Coalesce a `propchange` event into the burst queue and, when unpaused, * dispatch it synchronously. Reuses the queued event object across * dispatches in the same burst — `value` updates to the latest stored * value and `oldValue` rebases to what the consumer was last told. * - * @param {string} eventName * @param {PropChangeEventProps} eventProps */ - #firePropChangeEvent (eventName, eventProps) { - let key = `${eventName}::${eventProps.name}`; + #firePropChangeEvent (eventProps) { + let key = eventProps.name; let event = this.#eventQueue.get(key); if (event) { @@ -198,7 +193,7 @@ export default class ElementProps extends Map { this.#eventQueue.delete(key); } else { - event = new PropChangeEvent(eventName, eventProps); + event = new PropChangeEvent("propchange", eventProps); } if (!this.#paused) { @@ -235,13 +230,9 @@ export default class ElementProps extends Map { let changed = new Map(); for (let event of this.#eventQueue.values()) { - // Alias eventNames are listener-side conveniences; only canonical - // propchange feeds the bunch. Net-zero entries are dropped — they - // only exist so resume could dispatch a revert to the consumer. - if ( - event.type === "propchange" - && !event.prop.spec.equals(event.value, event.firstOldValue) - ) { + // Net-zero entries are dropped — they only exist so resume could + // dispatch a revert to the consumer. + if (!event.prop.spec.equals(event.value, event.firstOldValue)) { changed.set(event.name, event.firstOldValue); } } diff --git a/src/plugins/props/util/Prop.js b/src/plugins/props/util/Prop.js index 83d6db48..a89e6202 100644 --- a/src/plugins/props/util/Prop.js +++ b/src/plugins/props/util/Prop.js @@ -30,7 +30,6 @@ let Self = class Prop { this.set = spec.set; this.convert = spec.convert; this.changed = spec.changed; - this.eventNames = spec.eventNames; this.enumerable = spec.enumerable ?? true; this.default = spec.default; diff --git a/test/plugins/events/propchange.js b/test/plugins/events/propchange.js index 023a2742..ccb50aad 100644 --- a/test/plugins/events/propchange.js +++ b/test/plugins/events/propchange.js @@ -2,6 +2,8 @@ import { defineElement } from "../../util/dom.js"; import { default as propsPlugin } from "../../../src/plugins/props/index.js"; import { default as eventsPlugin } from "../../../src/plugins/events/index.js"; +const flush = () => Promise.resolve(); + export default { name: "Shortcut events", @@ -29,5 +31,44 @@ export default { }, expect: [{ value: "x", source: "initial" }], }, + { + name: "Shortcut events ride along on coalesced propchange and don't feed propschange", + description: "Burst dispatches one propchange (old=1, value=3); the alias mirrors it, propschange sees only [v, 1].", + async run () { + let tag = defineElement({ + plugins: [propsPlugin, eventsPlugin], + props: { v: { type: Number, default: 0 } }, + events: { valuechange: { propchange: "v" } }, + }); + let element = document.createElement(tag); + document.body.append(element); + + let propsEvents = []; + element.addEventListener("propschange", e => propsEvents.push([...e.changed])); + + element.v = 1; // stored old before the burst + let aliasLog = []; + element.addEventListener("valuechange", e => + aliasLog.push([e.name, e.oldValue, e.value])); + await flush(); + let propschangeCountBefore = propsEvents.length; + + element.props.paused = true; + element.v = 2; + element.v = 3; + element.props.paused = false; + + await flush(); + element.remove(); + return { + alias: aliasLog, + propschange: propsEvents.slice(propschangeCountBefore), + }; + }, + expect: { + alias: [["v", 1, 3]], + propschange: [[["v", 1]]], + }, + }, ], }; diff --git a/test/plugins/props/propschange.js b/test/plugins/props/propschange.js index c0d49ecd..e1fc7f00 100644 --- a/test/plugins/props/propschange.js +++ b/test/plugins/props/propschange.js @@ -187,37 +187,5 @@ export default { [["v", 0]], ], }, - { - name: "Per-event-name aliases dedup independently and don't feed propschange", - async run () { - let { element } = this.data; - element.v = 1; // stored old before the burst - let aliasLog = []; - element.addEventListener("valuechange", e => - aliasLog.push([e.name, e.oldValue, e.value])); - await flush(); - let propschangeCountBefore = this.data.propsEvents.length; - - element.props.paused = true; - element.v = 2; - element.v = 3; - element.props.paused = false; - - await flush(); - return { - alias: aliasLog, - propschange: this.data.propsEvents.slice(propschangeCountBefore), - }; - }, - arg: { - props: { - v: { type: Number, default: 0, eventNames: ["valuechange"] }, - }, - }, - expect: { - alias: [["v", 1, 3]], - propschange: [[["v", 1]]], - }, - }, ], }; From a42fdbc285b13a63fa57a144235ba2977859426c Mon Sep 17 00:00:00 2001 From: Lea Verou Date: Wed, 27 May 2026 13:47:31 -0400 Subject: [PATCH 04/18] Inline #firePropChangeEvent into propChanged Single-callsite helper. Folding it in lets the net-zero check read as a positive "keep if work left" branch instead of an early return. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/plugins/props/util/ElementProps.js | 73 +++++++++++--------------- 1 file changed, 31 insertions(+), 42 deletions(-) diff --git a/src/plugins/props/util/ElementProps.js b/src/plugins/props/util/ElementProps.js index 6c568881..bcaa4053 100644 --- a/src/plugins/props/util/ElementProps.js +++ b/src/plugins/props/util/ElementProps.js @@ -3,8 +3,6 @@ import ElementProp from "./ElementProp.js"; import PropChangeEvent from "./PropChangeEvent.js"; import PropsChangeEvent from "./PropsChangeEvent.js"; -/** @import { PropChangeEventProps } from "./PropChangeEvent.js" */ - const { props } = symbols.known; /** @@ -150,50 +148,32 @@ export default class ElementProps extends Map { } /** - * Fire propchange events for `ep` and cascade updates to any dependents. + * Fire a `propchange` event for `ep` and cascade updates to any dependents. + * + * The event is coalesced into the burst queue keyed by prop name: a + * queued entry has its `value` updated and is moved to the end of + * insertion order; otherwise a fresh event is created. When unpaused, + * the event is dispatched synchronously and its `oldValue` rebases to + * what the consumer was just told. The queue only retains entries with + * work left — net-zero entries (nothing to tell the consumer AND no net + * delta for the `propschange` drain) are dropped. + * * @param {ElementProp} ep The prop that changed. * @param {Object} change Change descriptor (see {@link ElementProp#set}). */ propChanged (ep, change) { // Source-first: fire before cascading so listeners hear the written prop first. - this.#firePropChangeEvent({ - name: ep.name, - prop: ep, - ...change, - }); - - // Update all props that depend on this one. {@link get} materializes a - // dependent that hasn't been wrapped yet (e.g. during the constructor's - // initial iteration) on demand. - let dependentSpecs = this.spec.dependents[ep.name] ?? []; - for (let depSpec of dependentSpecs) { - let dep = this.get(depSpec.name); - if (dep.dependsOn(ep)) { - dep.update(ep); - } - } - } - - /** - * Coalesce a `propchange` event into the burst queue and, when unpaused, - * dispatch it synchronously. Reuses the queued event object across - * dispatches in the same burst — `value` updates to the latest stored - * value and `oldValue` rebases to what the consumer was last told. - * - * @param {PropChangeEventProps} eventProps - */ - #firePropChangeEvent (eventProps) { - let key = eventProps.name; + let key = ep.name; let event = this.#eventQueue.get(key); if (event) { // Coalesce: keep firstOldValue + oldValue (last-told), update value. - event.value = eventProps.value; + event.value = change.value; // delete + set moves the entry to the end of insertion order. this.#eventQueue.delete(key); } else { - event = new PropChangeEvent("propchange", eventProps); + event = new PropChangeEvent("propchange", { name: ep.name, prop: ep, ...change }); } if (!this.#paused) { @@ -202,20 +182,29 @@ export default class ElementProps extends Map { event.oldValue = event.value; } - let { spec } = event.prop; - // Drop only when both axes are zero: nothing left to tell the - // consumer AND the burst's net effect is zero. If the consumer was - // told an intermediate value, the entry stays so resume can dispatch + // Keep only if there's work left: something still to tell the consumer, + // OR a non-zero net effect for the `propschange` drain. If the consumer + // was told an intermediate value, the entry stays so resume can dispatch // the revert (propschange still skips it — #drain filters by firstOldValue). + let { spec } = ep; if ( - spec.equals(event.oldValue, event.value) - && spec.equals(event.value, event.firstOldValue) + !spec.equals(event.oldValue, event.value) + || !spec.equals(event.value, event.firstOldValue) ) { - return; + this.#eventQueue.set(key, event); + queueMicrotask(() => this.#drain()); } - this.#eventQueue.set(key, event); - queueMicrotask(() => this.#drain()); + // Update all props that depend on this one. {@link get} materializes a + // dependent that hasn't been wrapped yet (e.g. during the constructor's + // initial iteration) on demand. + let dependentSpecs = this.spec.dependents[ep.name] ?? []; + for (let depSpec of dependentSpecs) { + let dep = this.get(depSpec.name); + if (dep.dependsOn(ep)) { + dep.update(ep); + } + } } /** From 0510f89b2b50d2af8f856d05b8010ae2e48a04ac Mon Sep 17 00:00:00 2001 From: Lea Verou Date: Wed, 27 May 2026 15:37:27 -0400 Subject: [PATCH 05/18] Rework ElementProps burst handling around an ordered event log MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The burst queue used to be a Map where each queued event carried a `firstOldValue` field separate from `oldValue`, mutated `value`/`oldValue` across dispatches, and required a "keep if work left" net-zero check to decide whether to retain it. This conflated the per-write `propchange` dispatch with the per-burst `propschange` delta, and exposed the same event object — reused across dispatches — to listeners. Restructure around an ordered array of fresh event objects: - `#eventQueue` is now a `PropChangeEvent[]` populated only while paused. Live (unpaused) writes go straight through `#firePropChangeEvent` and never touch the queue. - `#firePropChangeEvent(event)` is the single dispatch site for both live writes and resume flush. It also owns `propschange` tracking — on every dispatch it updates a per-prop `#propschangeEntries` Map keyed by name with `{firstOldValue, value, spec}`, and schedules the microtask drain on the first entry of a burst. - `#flushPending()` (sync, on `paused = false`) walks the queue in write order, in-place coalesces consecutive same-prop entries by mutating the run's first event with the latest fields (keeping its burst-start `oldValue`), then dispatches the surviving entries via `#firePropChangeEvent`. A sequence like `A A B A` produces three dispatches, not two — non-adjacent runs of the same prop stay separate. - `#drain()` (microtask) only fires `propschange`, reading first-old / last-value from `#propschangeEntries`. No queue walk needed. Drops `firstOldValue` from the public `PropChangeEvent` shape and the "same event object reused across a burst" README caveat — each dispatch gets a fresh event, so stashing fields is safe. Adds a test pinning the `A A B A` sequential-coalesce behavior on resume. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/plugins/props/README.md | 2 - src/plugins/props/util/ElementProps.js | 173 +++++++++++++--------- src/plugins/props/util/PropChangeEvent.js | 21 +-- test/plugins/props/propschange.js | 44 ++++++ 4 files changed, 148 insertions(+), 92 deletions(-) diff --git a/src/plugins/props/README.md b/src/plugins/props/README.md index 3d4ee4c7..49e2a4ee 100644 --- a/src/plugins/props/README.md +++ b/src/plugins/props/README.md @@ -246,8 +246,6 @@ element.addEventListener("propchange", e => { }); ``` -The event object is reused across dispatches within a burst, so `value` and `oldValue` will mutate past your handler if you stash the event. Copy the fields you need. - Subclasses that define a `propChangedCallback(event)` method are auto-wired to `propchange`. ### `propschange` — coalesced, microtask-deferred diff --git a/src/plugins/props/util/ElementProps.js b/src/plugins/props/util/ElementProps.js index bcaa4053..a0901ac7 100644 --- a/src/plugins/props/util/ElementProps.js +++ b/src/plugins/props/util/ElementProps.js @@ -12,9 +12,10 @@ export default class ElementProps extends Map { #paused = true; /** - * Whether propchange-style event dispatch is currently held. While `true`, - * fired events are coalesced per (eventName, propName) in the burst queue; - * flipping to `false` dispatches every undispatched event in queue order. + * Whether `propchange` dispatch is currently held. While `true`, writes + * append to the burst queue without dispatching; flipping to `false` + * sequentially coalesces undispatched runs (same prop in a row) and + * dispatches one event per run. */ get paused () { return this.#paused; @@ -29,22 +30,7 @@ export default class ElementProps extends Map { this.#paused = value; if (!value) { - for (let event of this.#eventQueue.values()) { - // Stale consumer view: oldValue ≠ value means we haven't told - // the consumer about the current value yet. - if (!event.prop.spec.equals(event.oldValue, event.value)) { - this.element.dispatchEvent(event); - // Rebase: consumer now knows about the current value. - event.oldValue = event.value; - } - } - - // Drains scheduled while paused exited early without firing - // propschange. Queue a fresh one so the bunched event still fires - // after the resume flush settles. - if (this.#eventQueue.size > 0) { - queueMicrotask(() => this.#drain()); - } + this.#flushPending(); } } @@ -52,15 +38,21 @@ export default class ElementProps extends Map { ignoredAttributes = new Set(); /** - * Coalesced `propchange` events keyed by prop name. Each entry is the - * event that would be dispatched right now for that prop: `oldValue` is - * what was last told to the consumer (or the burst-start value if never - * dispatched), `value` is the current stored value, and `firstOldValue` - * is the sticky burst-start value used by the `propschange` drain. - * `event.target` is non-null once the event has been dispatched at least once. - * @type {Map} + * Paused-pending `propchange` events in write order. Empty during + * unpaused operation — live writes go straight through + * {@link #firePropChangeEvent}. + * @type {PropChangeEvent[]} + */ + #eventQueue = []; + + /** + * Per-prop burst tracking for `propschange`. Each entry holds the prop's + * burst-start `oldValue` (sticky), latest dispatched `value`, and spec. + * Populated by {@link #firePropChangeEvent} on every dispatch; consumed + * and cleared by {@link #drain} when the microtask fires `propschange`. + * @type {Map} */ - #eventQueue = new Map(); + #propschangeEntries = new Map(); /** * Construct the per-element collection and run the mount-time init pass. @@ -148,51 +140,20 @@ export default class ElementProps extends Map { } /** - * Fire a `propchange` event for `ep` and cascade updates to any dependents. - * - * The event is coalesced into the burst queue keyed by prop name: a - * queued entry has its `value` updated and is moved to the end of - * insertion order; otherwise a fresh event is created. When unpaused, - * the event is dispatched synchronously and its `oldValue` rebases to - * what the consumer was just told. The queue only retains entries with - * work left — net-zero entries (nothing to tell the consumer AND no net - * delta for the `propschange` drain) are dropped. - * + * Either dispatch a `propchange` synchronously or queue it for resume, + * then cascade updates to any dependents. * @param {ElementProp} ep The prop that changed. * @param {Object} change Change descriptor (see {@link ElementProp#set}). */ propChanged (ep, change) { // Source-first: fire before cascading so listeners hear the written prop first. - let key = ep.name; - let event = this.#eventQueue.get(key); + let event = new PropChangeEvent("propchange", { name: ep.name, prop: ep, ...change }); - if (event) { - // Coalesce: keep firstOldValue + oldValue (last-told), update value. - event.value = change.value; - // delete + set moves the entry to the end of insertion order. - this.#eventQueue.delete(key); + if (this.#paused) { + this.#eventQueue.push(event); } else { - event = new PropChangeEvent("propchange", { name: ep.name, prop: ep, ...change }); - } - - if (!this.#paused) { - this.element.dispatchEvent(event); - // Rebase: consumer now knows about the current value. - event.oldValue = event.value; - } - - // Keep only if there's work left: something still to tell the consumer, - // OR a non-zero net effect for the `propschange` drain. If the consumer - // was told an intermediate value, the entry stays so resume can dispatch - // the revert (propschange still skips it — #drain filters by firstOldValue). - let { spec } = ep; - if ( - !spec.equals(event.oldValue, event.value) - || !spec.equals(event.value, event.firstOldValue) - ) { - this.#eventQueue.set(key, event); - queueMicrotask(() => this.#drain()); + this.#firePropChangeEvent(event); } // Update all props that depend on this one. {@link get} materializes a @@ -208,24 +169,88 @@ export default class ElementProps extends Map { } /** - * Build the net change map from the burst queue and fire `propschange`. - * No-op while paused or after another drain already cleared the queue — - * the {@link paused} setter queues a fresh drain on resume. + * Dispatch a single `propchange` event and update `propschange` tracking. + * Skips no-op events (oldValue === value), which only arise as the result + * of coalescing a paused round-trip; live writes are already filtered + * upstream by {@link ElementProp#set}. + * + * @param {PropChangeEvent} event + */ + #firePropChangeEvent (event) { + let { spec } = event.prop; + if (spec.equals(event.oldValue, event.value)) { + return; + } + + this.element.dispatchEvent(event); + + let entry = this.#propschangeEntries.get(event.name); + if (entry) { + entry.value = event.value; + return; + } + + // First entry of a fresh burst schedules the propschange drain. + if (this.#propschangeEntries.size === 0) { + queueMicrotask(() => this.#drain()); + } + this.#propschangeEntries.set(event.name, { + firstOldValue: event.oldValue, + value: event.value, + spec, + }); + } + + /** + * Walk the paused queue in write order, in-place coalescing consecutive + * same-prop events (a run like `A A B A` keeps three entries, not two), + * then dispatch each surviving entry. Called synchronously from the + * `paused` setter on resume. + */ + #flushPending () { + // In-place coalesce consecutive same-prop entries: collapse later + // events into the run's first one, keeping its burst-start oldValue. + let writeIdx = 0; + let lastName = null; + for (let event of this.#eventQueue) { + if (event.name === lastName) { + let prev = this.#eventQueue[writeIdx - 1]; + let { oldValue } = prev; + Object.assign(prev, event); + prev.oldValue = oldValue; + } + else { + this.#eventQueue[writeIdx++] = event; + lastName = event.name; + } + } + this.#eventQueue.length = writeIdx; + + // Dispatch the coalesced events. Detach first so listener-induced + // writes during dispatch don't get re-coalesced into this pass. + let queue = this.#eventQueue; + this.#eventQueue = []; + for (let event of queue) { + this.#firePropChangeEvent(event); + } + } + + /** + * Fire `propschange` with the net first→last delta per prop. No-op while + * paused or when no propchange dispatched during the burst. */ #drain () { - if (this.#paused) { + if (this.#paused || this.#propschangeEntries.size === 0) { return; } let changed = new Map(); - for (let event of this.#eventQueue.values()) { - // Net-zero entries are dropped — they only exist so resume could - // dispatch a revert to the consumer. - if (!event.prop.spec.equals(event.value, event.firstOldValue)) { - changed.set(event.name, event.firstOldValue); + for (let [name, { firstOldValue, value, spec }] of this.#propschangeEntries) { + if (!spec.equals(firstOldValue, value)) { + changed.set(name, firstOldValue); } } - this.#eventQueue.clear(); + this.#propschangeEntries.clear(); if (changed.size > 0) { this.element.dispatchEvent(new PropsChangeEvent("propschange", { changed })); diff --git a/src/plugins/props/util/PropChangeEvent.js b/src/plugins/props/util/PropChangeEvent.js index b32ef9a9..66ca8e2b 100644 --- a/src/plugins/props/util/PropChangeEvent.js +++ b/src/plugins/props/util/PropChangeEvent.js @@ -3,31 +3,22 @@ * @property {string} name Prop name. * @property {import("./ElementProp.js").default} prop * @property {*} value Current stored value (parsed + converted). - * @property {*} oldValue Value the consumer was last told about: the stored - * value before this change for a fresh dispatch, or the previously-dispatched - * value when the event is re-dispatched after a paused burst. + * @property {*} oldValue Stored value before this change, or — for a coalesced + * resume dispatch — the burst-start value the consumer was last told. * @property {"default" | "property" | "attribute" | "get" | "convert"} source * @property {string} [attributeName] Set when `source === "attribute"` or a * property write reflects to an attribute. * @property {string | null} [attributeValue] * @property {string | null} [oldAttributeValue] - * @property {*} [firstOldValue] Stored value before the first change in the - * current burst. Stays sticky across rebasing of `oldValue` so the matching - * `propschange` drain can compute the net first→last delta. Defaults to - * `oldValue` on construction. * * @typedef {EventInit & PropChangeEventProps} PropChangeEventInit */ /** * Per-prop change event. Fires synchronously inside a property/attribute - * write, then again on resume after a paused burst settles. Fields are - * direct properties — no `detail` wrapper. - * - * The same event object is reused across dispatches within a burst: `value` - * tracks the latest stored value and `oldValue` rebases to whatever was last - * dispatched, so listeners that stash the event will see those fields mutate - * past their handler. Copy the fields if you need a snapshot. + * write, or on resume from a paused burst (sequentially coalesced). Fields + * are direct properties — no `detail` wrapper. Each dispatch gets a fresh + * event object; fields don't mutate past the handler. * * @implements {PropChangeEventProps} */ @@ -48,8 +39,6 @@ export default class PropChangeEvent extends Event { this[key] = value; } } - - this.firstOldValue ??= this.oldValue; } /** diff --git a/test/plugins/props/propschange.js b/test/plugins/props/propschange.js index e1fc7f00..d4e83527 100644 --- a/test/plugins/props/propschange.js +++ b/test/plugins/props/propschange.js @@ -134,6 +134,50 @@ export default { propschangesAfter: 0, }, }, + { + name: "Paused burst: A,A,A,B,B,A coalesces sequentially into three dispatches", + description: + "Consecutive runs for the same prop merge; B between A-runs splits them, so the consumer hears A then B then A again.", + async run () { + let { element } = this.data; + element.a = 0; // establish stored values before the burst + element.b = 0; + await flush(); + let propchangeLog = []; + element.addEventListener("propchange", e => + propchangeLog.push([e.name, e.oldValue, e.value])); + let propschangeCountBefore = this.data.propsEvents.length; + + element.props.paused = true; + element.a = 1; + element.a = 2; + element.a = 3; + element.b = 10; + element.b = 20; + element.a = 7; + element.props.paused = false; + + await flush(); + return { + propchanges: propchangeLog, + propschange: this.data.propsEvents.slice(propschangeCountBefore), + }; + }, + arg: { + props: { + a: { type: Number, default: 0 }, + b: { type: Number, default: 0 }, + }, + }, + expect: { + propchanges: [ + ["a", 0, 3], + ["b", 0, 20], + ["a", 3, 7], + ], + propschange: [[["a", 0], ["b", 0]]], + }, + }, { name: "Paused burst: one rebased propchange + one propschange on resume", async run () { From c1dab591538479cc0305525cecf5826353ef3a1a Mon Sep 17 00:00:00 2001 From: Lea Verou Date: Wed, 27 May 2026 15:37:40 -0400 Subject: [PATCH 06/18] Attach on*/alias listeners early so no catch-up is needed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both the propchange-alias plugin and the on* prop plugin used to late-bind their listeners (events/propchange in `first_connected`, onprops in `constructed`), then synthesize "initial" events to deliver the mount-time state that those late listeners had missed. The two catch-ups depended on each other's order: events/propchange's `first_connected` re-dispatch ran *after* onprops' `constructed` so the re-dispatch would actually reach the freshly-attached on* handler. Restructure so both attach during prop init / element construction: - onprops moves the addEventListener/removeEventListener swap into the on* prop's spec `changed` callback. `spec.changed` runs synchronously inside `ElementProp.changed` — before `propChanged` fires — so on* handlers are bound during the prop's own init pass, in time to catch the mount-time propchanges (and aliases) that flush afterwards. - events/propchange moves the alias re-dispatch listener from `first_connected` to the `constructor` hook, so it's attached during element construction and naturally catches mount propchanges. No synthesized "initial" re-fire needed. Observable change: `on*=` handlers in markup now receive the mount-time alias with its real source (e.g. `"default"`) instead of the synthesized `"initial"`. Test updated accordingly. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/plugins/events/onprops.js | 46 ++++++++++--------------------- src/plugins/events/propchange.js | 25 +++-------------- test/plugins/events/propchange.js | 8 ++---- 3 files changed, 21 insertions(+), 58 deletions(-) diff --git a/src/plugins/events/onprops.js b/src/plugins/events/onprops.js index 77196f2a..76df81b8 100644 --- a/src/plugins/events/onprops.js +++ b/src/plugins/events/onprops.js @@ -39,6 +39,12 @@ const hooks = { continue; } + // Capture the event name in the closure so each on* prop knows + // which event to attach/detach when its value changes. The + // `changed` callback runs synchronously inside the write — before + // `propchange` fires — so handlers set during prop init catch + // the mount-time propchanges that follow. + let eventName = name; newProps[eventDef.onprop] = { type: { is: Function, @@ -47,6 +53,14 @@ const hooks = { reflect: { from: true, }, + changed ({ oldValue, value }) { + if (oldValue) { + this.removeEventListener(eventName, oldValue); + } + if (value) { + this.addEventListener(eventName, value); + } + }, }; this[eventProps] ??= {}; @@ -57,38 +71,6 @@ const hooks = { this.defineProps(newProps); } }, - - constructed () { - // Deal with existing values - if (!this.constructor[eventProps]) { - return; - } - - for (let name in this.constructor[eventProps]) { - // Read any existing on* prop value - let value = this[name]; - - if (typeof value === "function") { - let eventName = this.constructor[eventProps][name]; - this.addEventListener(eventName, value); - } - } - - // Listen for changes - this.addEventListener("propchange", event => { - let eventName = this.constructor[eventProps][event.name]; - if (eventName) { - // Implement onEventName attributes/properties - if (event.oldValue) { - this.removeEventListener(eventName, event.oldValue); - } - - if (event.value) { - this.addEventListener(eventName, event.value); - } - } - }); - }, }; const providesStatic = {}; diff --git a/src/plugins/events/propchange.js b/src/plugins/events/propchange.js index 67892c22..34cd49fb 100644 --- a/src/plugins/events/propchange.js +++ b/src/plugins/events/propchange.js @@ -40,7 +40,7 @@ const hooks = { } }, - first_connected () { + constructor () { let aliases = this.constructor[propchange]; if (!aliases) { return; @@ -48,7 +48,9 @@ const hooks = { // Re-dispatch every propchange as its declared alias event(s). The // canonical event already inherits coalescing / pause-resume from - // ElementProps, so the alias rides along for free. + // ElementProps, so the alias rides along for free. Attaching in the + // `constructor` hook means we catch mount propchanges too — no + // synthesized catch-up needed. this.addEventListener("propchange", event => { let aliasNames = aliases[event.name]; if (!aliasNames) { @@ -65,25 +67,6 @@ const hooks = { })); } }); - - // Often propchange events have already fired by the time the event handlers are added - for (let propName in aliases) { - let value = this[propName]; - - if (value === undefined) { - continue; - } - - let prop = this.props.get(propName); - for (let aliasName of aliases[propName]) { - this.dispatchEvent(new PropChangeEvent(aliasName, { - name: propName, - prop, - source: "initial", - value, - })); - } - } }, }; diff --git a/test/plugins/events/propchange.js b/test/plugins/events/propchange.js index ccb50aad..0b116a6f 100644 --- a/test/plugins/events/propchange.js +++ b/test/plugins/events/propchange.js @@ -9,8 +9,8 @@ export default { tests: [ { - name: "first_connected shortcut re-fire carries the value", - description: "Late-bound on*= handlers only see the catch-up re-fire (issue #106)", + name: "Markup on*= handlers catch the mount-time alias dispatch", + description: "The on* prop's `changed` callback attaches the handler during prop init, before mount propchanges flush — so it receives the real mount alias with its original source (issue #106).", run () { let tag = defineElement({ plugins: [propsPlugin, eventsPlugin], @@ -18,8 +18,6 @@ export default { events: { valuechange: { propchange: "value" } }, }); - // Declared via markup so the on*= handler binds late, like real - // consumer usage — it then sees only the first_connected catch-up. let host = document.createElement("div"); host.innerHTML = `<${tag} onvaluechange="(this._log ??= []).push({value: event.value, source: event.source})">`; document.body.append(host); @@ -29,7 +27,7 @@ export default { return element._log ?? []; }, - expect: [{ value: "x", source: "initial" }], + expect: [{ value: "x", source: "default" }], }, { name: "Shortcut events ride along on coalesced propchange and don't feed propschange", From 32b0d18ad4493b607d3d9a0da9e8f9a008fe008d Mon Sep 17 00:00:00 2001 From: Lea Verou Date: Wed, 27 May 2026 15:45:11 -0400 Subject: [PATCH 07/18] Extract propschange into a separate plugin Co-Authored-By: Claude Opus 4.7 (1M context) --- src/plugins/index.js | 5 +- src/plugins/props/index.js | 4 -- src/plugins/props/propschange.js | 79 ++++++++++++++++++++++ src/plugins/props/util/ElementProps.js | 59 ++-------------- src/plugins/props/util/PropsChangeEvent.js | 29 -------- test/plugins/events/propchange.js | 3 +- test/plugins/props/index.js | 11 +-- test/plugins/props/propschange.js | 42 ++++++++++-- 8 files changed, 128 insertions(+), 104 deletions(-) create mode 100644 src/plugins/props/propschange.js delete mode 100644 src/plugins/props/util/PropsChangeEvent.js diff --git a/src/plugins/index.js b/src/plugins/index.js index 05f4708a..5eb6ba53 100644 --- a/src/plugins/index.js +++ b/src/plugins/index.js @@ -4,14 +4,15 @@ import elements from "./elements/index.js"; import props from "./props/index.js"; +import propschange from "./props/propschange.js"; import events from "./events/index.js"; import formBehavior from "./form-behavior/index.js"; import slots from "./slots/index.js"; import states from "./states/index.js"; import styles from "./styles/index.js"; -export { elements, slots, states, props, events, formBehavior, styles }; +export { elements, slots, states, props, propschange, events, formBehavior, styles }; export default { - dependencies: [elements, slots, states, props, events, formBehavior, styles], + dependencies: [elements, slots, states, props, propschange, events, formBehavior, styles], }; diff --git a/src/plugins/props/index.js b/src/plugins/props/index.js index fe38c860..5a0ef176 100644 --- a/src/plugins/props/index.js +++ b/src/plugins/props/index.js @@ -27,10 +27,6 @@ const hooks = { if (this.propChangedCallback) { this.addEventListener("propchange", this.propChangedCallback); } - - if (this.updated) { - this.addEventListener("propschange", this.updated); - } }, connected () { diff --git a/src/plugins/props/propschange.js b/src/plugins/props/propschange.js new file mode 100644 index 00000000..5a399b2b --- /dev/null +++ b/src/plugins/props/propschange.js @@ -0,0 +1,79 @@ +/** + * Implement `propschange`: a coalesced, microtask-deferred event that fires + * once per burst of `propchange` events, carrying the net first→last delta + * per prop in a `Map`. Round-trips (value returns to the + * burst-start) drop out. + * + * Subclasses that define an `updated(event)` method are auto-wired to + * `propschange`, mirroring Lit's `updated(changedProperties)`. + */ + +import propsPlugin from "./index.js"; + +const dependencies = [propsPlugin]; + +/** + * Event fired after a burst of `propchange` events settles within a microtask. + * `e.changed` is a `Map` carrying the net first→last delta; + * `oldValue` is the stored internal value (parsed + converted) before the + * first change in the burst, mirroring `propchange`'s `e.oldValue`. Current + * values are read via `this[name]` inside the handler. + */ +export class PropsChangeEvent extends Event { + /** + * @param {string} type + * @param {EventInit & {changed: Map}} options + */ + constructor (type, { changed, ...options } = {}) { + super(type, options); + this.changed = changed; + } +} + +const hooks = { + constructor () { + // Per-prop burst tracking: first oldValue (sticky) + latest value + spec. + let entries = new Map(); + let scheduled = false; + + // Attaching in the `constructor` hook means the listener catches + // mount-time propchanges (which fire before `connectedCallback`). + this.addEventListener("propchange", event => { + let entry = entries.get(event.name); + if (entry) { + entry.value = event.value; + return; + } + + if (!scheduled) { + scheduled = true; + queueMicrotask(() => { + scheduled = false; + let changed = new Map(); + for (let [name, { firstOldValue, value, spec }] of entries) { + if (!spec.equals(firstOldValue, value)) { + changed.set(name, firstOldValue); + } + } + entries.clear(); + + if (changed.size > 0) { + this.dispatchEvent(new PropsChangeEvent("propschange", { changed })); + } + }); + } + + entries.set(event.name, { + firstOldValue: event.oldValue, + value: event.value, + spec: event.prop.spec, + }); + }); + + if (this.updated) { + this.addEventListener("propschange", this.updated); + } + }, +}; + +export default { dependencies, hooks }; diff --git a/src/plugins/props/util/ElementProps.js b/src/plugins/props/util/ElementProps.js index a0901ac7..c4713c7d 100644 --- a/src/plugins/props/util/ElementProps.js +++ b/src/plugins/props/util/ElementProps.js @@ -1,7 +1,6 @@ import { symbols } from "xtensible"; import ElementProp from "./ElementProp.js"; import PropChangeEvent from "./PropChangeEvent.js"; -import PropsChangeEvent from "./PropsChangeEvent.js"; const { props } = symbols.known; @@ -45,15 +44,6 @@ export default class ElementProps extends Map { */ #eventQueue = []; - /** - * Per-prop burst tracking for `propschange`. Each entry holds the prop's - * burst-start `oldValue` (sticky), latest dispatched `value`, and spec. - * Populated by {@link #firePropChangeEvent} on every dispatch; consumed - * and cleared by {@link #drain} when the microtask fires `propschange`. - * @type {Map} - */ - #propschangeEntries = new Map(); - /** * Construct the per-element collection and run the mount-time init pass. * Pre-mount user writes (`element.foo = …`) and pre-mount attribute writes @@ -169,36 +159,19 @@ export default class ElementProps extends Map { } /** - * Dispatch a single `propchange` event and update `propschange` tracking. - * Skips no-op events (oldValue === value), which only arise as the result - * of coalescing a paused round-trip; live writes are already filtered - * upstream by {@link ElementProp#set}. + * Dispatch a single `propchange` event. Skips no-op events + * (oldValue === value), which only arise as the result of coalescing + * a paused round-trip; live writes are already filtered upstream by + * {@link ElementProp#set}. * * @param {PropChangeEvent} event */ #firePropChangeEvent (event) { - let { spec } = event.prop; - if (spec.equals(event.oldValue, event.value)) { + if (event.prop.spec.equals(event.oldValue, event.value)) { return; } this.element.dispatchEvent(event); - - let entry = this.#propschangeEntries.get(event.name); - if (entry) { - entry.value = event.value; - return; - } - - // First entry of a fresh burst schedules the propschange drain. - if (this.#propschangeEntries.size === 0) { - queueMicrotask(() => this.#drain()); - } - this.#propschangeEntries.set(event.name, { - firstOldValue: event.oldValue, - value: event.value, - spec, - }); } /** @@ -234,26 +207,4 @@ export default class ElementProps extends Map { this.#firePropChangeEvent(event); } } - - /** - * Fire `propschange` with the net first→last delta per prop. No-op while - * paused or when no propchange dispatched during the burst. - */ - #drain () { - if (this.#paused || this.#propschangeEntries.size === 0) { - return; - } - - let changed = new Map(); - for (let [name, { firstOldValue, value, spec }] of this.#propschangeEntries) { - if (!spec.equals(firstOldValue, value)) { - changed.set(name, firstOldValue); - } - } - this.#propschangeEntries.clear(); - - if (changed.size > 0) { - this.element.dispatchEvent(new PropsChangeEvent("propschange", { changed })); - } - } } diff --git a/src/plugins/props/util/PropsChangeEvent.js b/src/plugins/props/util/PropsChangeEvent.js deleted file mode 100644 index 1cd0b2fe..00000000 --- a/src/plugins/props/util/PropsChangeEvent.js +++ /dev/null @@ -1,29 +0,0 @@ -/** - * @typedef {object} PropsChangeEventProps - * @property {Map} changed Net change map: prop name → the value - * before the first change in the burst. Round-trips (current value equals - * the recorded old value) are filtered out. - * - * @typedef {EventInit & PropsChangeEventProps} PropsChangeEventInit - */ - -/** - * Event fired after a burst of {@link PropChangeEvent}s settles within a microtask. - * Carries the net first→last delta as a `changed` Map of `name → oldValue`. - * Current values are read via `this[name]` inside the handler. - * - * `oldValue` is the stored internal value (parsed + converted) before the - * first change in the burst, mirroring `propchange`'s `e.oldValue`. - * - * @implements {PropsChangeEventProps} - */ -export default class PropsChangeEvent extends Event { - /** - * @param {string} type - * @param {PropsChangeEventInit} options - */ - constructor (type, { changed, ...options } = {}) { - super(type, options); - this.changed = changed; - } -} diff --git a/test/plugins/events/propchange.js b/test/plugins/events/propchange.js index 0b116a6f..42de194a 100644 --- a/test/plugins/events/propchange.js +++ b/test/plugins/events/propchange.js @@ -1,5 +1,6 @@ import { defineElement } from "../../util/dom.js"; import { default as propsPlugin } from "../../../src/plugins/props/index.js"; +import { default as propschangePlugin } from "../../../src/plugins/props/propschange.js"; import { default as eventsPlugin } from "../../../src/plugins/events/index.js"; const flush = () => Promise.resolve(); @@ -34,7 +35,7 @@ export default { description: "Burst dispatches one propchange (old=1, value=3); the alias mirrors it, propschange sees only [v, 1].", async run () { let tag = defineElement({ - plugins: [propsPlugin, eventsPlugin], + plugins: [propsPlugin, propschangePlugin, eventsPlugin], props: { v: { type: Number, default: 0 } }, events: { valuechange: { propchange: "v" } }, }); diff --git a/test/plugins/props/index.js b/test/plugins/props/index.js index 5ad55842..9e1be3cc 100644 --- a/test/plugins/props/index.js +++ b/test/plugins/props/index.js @@ -28,27 +28,22 @@ export default { element.addEventListener("propchange", e => events.push([e.name, e.target[e.name]])); - // Snapshot as [name, oldValue] tuples so tests can compare with - // plain array equality. - let propsEvents = []; - element.addEventListener("propschange", e => - propsEvents.push([...e.changed])); - for (let [name, value] of Object.entries(attributes ?? {})) { element.setAttribute(name, value); } document.body.append(element); - Object.assign(this.data, { element, events, propsEvents }); + Object.assign(this.data, { element, events }); }, afterEach () { this.data.element.remove(); }, - tests: [reflection, defaults, computed, propchange, propschange, lifecycle], + tests: [reflection, defaults, computed, propchange, lifecycle], }, + propschange, inheritance, install, observedAttributes, diff --git a/test/plugins/props/propschange.js b/test/plugins/props/propschange.js index d4e83527..f5d3797a 100644 --- a/test/plugins/props/propschange.js +++ b/test/plugins/props/propschange.js @@ -3,10 +3,38 @@ * events settles. Tests await `Promise.resolve()` (or a microtask) to let the * drain run before asserting. */ +import { default as propsPlugin } from "../../../src/plugins/props/index.js"; +import { default as propschangePlugin } from "../../../src/plugins/props/propschange.js"; +import { defineElement } from "../../util/dom.js"; + const flush = () => Promise.resolve(); export default { - name: "propschange events", + name: "propschange plugin", + + beforeEach () { + let { props, attributes, mixin } = this.arg; + let tag = defineElement({ plugins: [propsPlugin, propschangePlugin], props, mixin }); + let element = document.createElement(tag); + + let events = []; + element.addEventListener("propchange", e => events.push([e.name, e.target[e.name]])); + + let propsEvents = []; + element.addEventListener("propschange", e => propsEvents.push([...e.changed])); + + for (let [name, value] of Object.entries(attributes ?? {})) { + element.setAttribute(name, value); + } + + document.body.append(element); + + Object.assign(this.data, { element, events, propsEvents }); + }, + + afterEach () { + this.data.element.remove(); + }, tests: [ { @@ -175,7 +203,12 @@ export default { ["b", 0, 20], ["a", 3, 7], ], - propschange: [[["a", 0], ["b", 0]]], + propschange: [ + [ + ["a", 0], + ["b", 0], + ], + ], }, }, { @@ -226,10 +259,7 @@ export default { }; }, }, - expect: [ - [["v", undefined]], - [["v", 0]], - ], + expect: [[["v", undefined]], [["v", 0]]], }, ], }; From c49d1261bd9d69ed086a751beca95749d712ae44 Mon Sep 17 00:00:00 2001 From: Lea Verou Date: Wed, 27 May 2026 16:28:10 -0400 Subject: [PATCH 08/18] Restructure props/ to match events/ meta-plugin pattern MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the props-core plugin definition from `props/index.js` to `props/base.js` (mirroring `events/base.js`). The new `props/index.js` is a meta-plugin that bundles base + propschange: import props from "./base.js"; import propschange from "./propschange.js"; export { props, propschange }; export default { dependencies: [props, propschange] }; Internal imports updated to use `./base.js` where they want props core (propschange, events/onprops, events/propchange). The top-level `plugins/index.js` drops its separate `propschange` import — the props meta-plugin now carries it. Tests use `props/base.js` where they exercise props core alone (the `Behavior` suite, `install`, `observed-attributes`, `inheritance`, and the `events/propchange` test that loads `eventsPlugin` separately). The propschange test continues to explicitly import both `base` and `propschange` to document what it exercises. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/plugins/events/onprops.js | 2 +- src/plugins/events/propchange.js | 2 +- src/plugins/index.js | 5 +- src/plugins/props/base.js | 98 +++++++++++++++++++++ src/plugins/props/index.js | 102 ++-------------------- src/plugins/props/propschange.js | 2 +- test/plugins/events/propchange.js | 2 +- test/plugins/props/index.js | 2 +- test/plugins/props/inheritance.js | 2 +- test/plugins/props/install.js | 2 +- test/plugins/props/observed-attributes.js | 2 +- test/plugins/props/propschange.js | 2 +- 12 files changed, 117 insertions(+), 106 deletions(-) create mode 100644 src/plugins/props/base.js diff --git a/src/plugins/events/onprops.js b/src/plugins/events/onprops.js index 76df81b8..63a7a808 100644 --- a/src/plugins/events/onprops.js +++ b/src/plugins/events/onprops.js @@ -1,7 +1,7 @@ /** * Add on* props for UI events, just like native UI events */ -import propsPlugin from "../props/index.js"; +import propsPlugin from "../props/base.js"; import base from "./base.js"; import { symbols } from "xtensible"; import { defineOwnProperty } from "xtensible/util"; diff --git a/src/plugins/events/propchange.js b/src/plugins/events/propchange.js index 34cd49fb..f8cb1179 100644 --- a/src/plugins/events/propchange.js +++ b/src/plugins/events/propchange.js @@ -5,7 +5,7 @@ import { symbols } from "xtensible"; import base, { events } from "./base.js"; -import { props } from "../props/index.js"; +import { props } from "../props/base.js"; import PropChangeEvent from "../props/util/PropChangeEvent.js"; const { propchange } = symbols.new; diff --git a/src/plugins/index.js b/src/plugins/index.js index 5eb6ba53..05f4708a 100644 --- a/src/plugins/index.js +++ b/src/plugins/index.js @@ -4,15 +4,14 @@ import elements from "./elements/index.js"; import props from "./props/index.js"; -import propschange from "./props/propschange.js"; import events from "./events/index.js"; import formBehavior from "./form-behavior/index.js"; import slots from "./slots/index.js"; import states from "./states/index.js"; import styles from "./styles/index.js"; -export { elements, slots, states, props, propschange, events, formBehavior, styles }; +export { elements, slots, states, props, events, formBehavior, styles }; export default { - dependencies: [elements, slots, states, props, propschange, events, formBehavior, styles], + dependencies: [elements, slots, states, props, events, formBehavior, styles], }; diff --git a/src/plugins/props/base.js b/src/plugins/props/base.js new file mode 100644 index 00000000..5a0ef176 --- /dev/null +++ b/src/plugins/props/base.js @@ -0,0 +1,98 @@ +import Props from "./util/Props.js"; +import Prop from "./util/Prop.js"; +import ElementProps from "./util/ElementProps.js"; +import ElementProp from "./util/ElementProp.js"; +import { symbols } from "xtensible"; +import { defineOwnProperty, getSuperMethod } from "xtensible/util"; +import { defineLazyProperty } from "../../util/lazy.js"; +import PropType from "./util/PropType.js"; +import "./types/index.js"; + +export const { props } = symbols.known; + +export { PropType, Props, Prop, ElementProps, ElementProp }; + +const hooks = { + setup () { + if (Object.hasOwn(this, "props")) { + this.defineProps(); + } + }, + + constructor () { + if (!this.constructor[props]) { + return; + } + + if (this.propChangedCallback) { + this.addEventListener("propchange", this.propChangedCallback); + } + }, + + connected () { + this.props.paused = false; + }, + + disconnected () { + this.props.paused = true; + }, + + "attribute-changed" ({ name, oldValue }) { + this.props.attributeChanged(name, oldValue); + }, +}; + +const provides = { + props: undefined, // see below + + // Must be on the prototype before customElements.define runs — spec reads observedAttributes only if ACB is non-null. + attributeChangedCallback (name, oldValue, value) { + // super.attributeChangedCallback() + getSuperMethod(this, provides.attributeChangedCallback)?.call(this, name, oldValue, value); + + this.$hook("attribute-changed", { name, oldValue, value }); + }, + + constructor: { + defineProps (def = this.props) { + if (def instanceof Props && def.Class === this) { + // Already defined + return null; + } + + let env = { props: def }; + this.$hook("define-props", env); + + this[props].add(env.props); + }, + + get observedAttributes () { + // FIXME how to combine with existing observedAttributes? + let attributes = [...this[props].allValues()] + .map(prop => prop.reflect.from) + .filter(Boolean); + return [...new Set(attributes)]; + }, + }, +}; + +/** + * Per-element collection of {@link ElementProp} wrappers, materialized on + * first access. The {@link ElementProps} constructor self-installs as the + * element's own `props` data property, shadowing this accessor from then on. + */ +defineLazyProperty(provides, "props", { + get () { + if (this.constructor.props) { + return new ElementProps(this); + } + }, + configurable: true, + writable: true, +}); + +defineOwnProperty(provides.constructor, props, function () { + return new Props(this, this.props); +}); + +export default { hooks, provides }; diff --git a/src/plugins/props/index.js b/src/plugins/props/index.js index 5a0ef176..a7c8ca16 100644 --- a/src/plugins/props/index.js +++ b/src/plugins/props/index.js @@ -1,98 +1,12 @@ -import Props from "./util/Props.js"; -import Prop from "./util/Prop.js"; -import ElementProps from "./util/ElementProps.js"; -import ElementProp from "./util/ElementProp.js"; -import { symbols } from "xtensible"; -import { defineOwnProperty, getSuperMethod } from "xtensible/util"; -import { defineLazyProperty } from "../../util/lazy.js"; -import PropType from "./util/PropType.js"; -import "./types/index.js"; - -export const { props } = symbols.known; - -export { PropType, Props, Prop, ElementProps, ElementProp }; - -const hooks = { - setup () { - if (Object.hasOwn(this, "props")) { - this.defineProps(); - } - }, - - constructor () { - if (!this.constructor[props]) { - return; - } - - if (this.propChangedCallback) { - this.addEventListener("propchange", this.propChangedCallback); - } - }, - - connected () { - this.props.paused = false; - }, - - disconnected () { - this.props.paused = true; - }, - - "attribute-changed" ({ name, oldValue }) { - this.props.attributeChanged(name, oldValue); - }, -}; - -const provides = { - props: undefined, // see below - - // Must be on the prototype before customElements.define runs — spec reads observedAttributes only if ACB is non-null. - attributeChangedCallback (name, oldValue, value) { - // super.attributeChangedCallback() - getSuperMethod(this, provides.attributeChangedCallback)?.call(this, name, oldValue, value); - - this.$hook("attribute-changed", { name, oldValue, value }); - }, - - constructor: { - defineProps (def = this.props) { - if (def instanceof Props && def.Class === this) { - // Already defined - return null; - } - - let env = { props: def }; - this.$hook("define-props", env); - - this[props].add(env.props); - }, - - get observedAttributes () { - // FIXME how to combine with existing observedAttributes? - let attributes = [...this[props].allValues()] - .map(prop => prop.reflect.from) - .filter(Boolean); - return [...new Set(attributes)]; - }, - }, -}; - /** - * Per-element collection of {@link ElementProp} wrappers, materialized on - * first access. The {@link ElementProps} constructor self-installs as the - * element's own `props` data property, shadowing this accessor from then on. + * Common props plugins, exported as a plugin, as well as separate exports. */ -defineLazyProperty(provides, "props", { - get () { - if (this.constructor.props) { - return new ElementProps(this); - } - }, - configurable: true, - writable: true, -}); -defineOwnProperty(provides.constructor, props, function () { - return new Props(this, this.props); -}); +import props from "./base.js"; +import propschange from "./propschange.js"; -export default { hooks, provides }; +export { props, propschange }; + +export default { + dependencies: [props, propschange], +}; diff --git a/src/plugins/props/propschange.js b/src/plugins/props/propschange.js index 5a399b2b..1a244d3d 100644 --- a/src/plugins/props/propschange.js +++ b/src/plugins/props/propschange.js @@ -8,7 +8,7 @@ * `propschange`, mirroring Lit's `updated(changedProperties)`. */ -import propsPlugin from "./index.js"; +import propsPlugin from "./base.js"; const dependencies = [propsPlugin]; diff --git a/test/plugins/events/propchange.js b/test/plugins/events/propchange.js index 42de194a..209bebb9 100644 --- a/test/plugins/events/propchange.js +++ b/test/plugins/events/propchange.js @@ -1,5 +1,5 @@ import { defineElement } from "../../util/dom.js"; -import { default as propsPlugin } from "../../../src/plugins/props/index.js"; +import { default as propsPlugin } from "../../../src/plugins/props/base.js"; import { default as propschangePlugin } from "../../../src/plugins/props/propschange.js"; import { default as eventsPlugin } from "../../../src/plugins/events/index.js"; diff --git a/test/plugins/props/index.js b/test/plugins/props/index.js index 9e1be3cc..92cede48 100644 --- a/test/plugins/props/index.js +++ b/test/plugins/props/index.js @@ -1,4 +1,4 @@ -import { default as propsPlugin } from "../../../src/plugins/props/index.js"; +import { default as propsPlugin } from "../../../src/plugins/props/base.js"; import { defineElement } from "../../util/dom.js"; import reflection from "./reflection.js"; import defaults from "./defaults.js"; diff --git a/test/plugins/props/inheritance.js b/test/plugins/props/inheritance.js index 12519d58..e4984929 100644 --- a/test/plugins/props/inheritance.js +++ b/test/plugins/props/inheritance.js @@ -1,5 +1,5 @@ import ElementFactory from "../../../src/element/factory.js"; -import { default as propsPlugin, props } from "../../../src/plugins/props/index.js"; +import { default as propsPlugin, props } from "../../../src/plugins/props/base.js"; let i = 0; diff --git a/test/plugins/props/install.js b/test/plugins/props/install.js index 0df473d7..d343e29d 100644 --- a/test/plugins/props/install.js +++ b/test/plugins/props/install.js @@ -1,5 +1,5 @@ import { defineElement } from "../../util/dom.js"; -import { default as propsPlugin } from "../../../src/plugins/props/index.js"; +import { default as propsPlugin } from "../../../src/plugins/props/base.js"; export default { name: "Installation", diff --git a/test/plugins/props/observed-attributes.js b/test/plugins/props/observed-attributes.js index dc80a1c9..9b728f11 100644 --- a/test/plugins/props/observed-attributes.js +++ b/test/plugins/props/observed-attributes.js @@ -1,4 +1,4 @@ -import { default as propsPlugin } from "../../../src/plugins/props/index.js"; +import { default as propsPlugin } from "../../../src/plugins/props/base.js"; import { defineElement } from "../../util/dom.js"; export default { diff --git a/test/plugins/props/propschange.js b/test/plugins/props/propschange.js index f5d3797a..eb2c76d9 100644 --- a/test/plugins/props/propschange.js +++ b/test/plugins/props/propschange.js @@ -3,7 +3,7 @@ * events settles. Tests await `Promise.resolve()` (or a microtask) to let the * drain run before asserting. */ -import { default as propsPlugin } from "../../../src/plugins/props/index.js"; +import { default as propsPlugin } from "../../../src/plugins/props/base.js"; import { default as propschangePlugin } from "../../../src/plugins/props/propschange.js"; import { defineElement } from "../../util/dom.js"; From b12c1f7c9219cd45f68c50420c6cfa16fc02ba59 Mon Sep 17 00:00:00 2001 From: Lea Verou Date: Thu, 28 May 2026 11:55:13 -0400 Subject: [PATCH 09/18] Apply suggestions from code review Co-authored-by: Dmitry Sharabin --- src/plugins/props/util/ElementProp.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/props/util/ElementProp.js b/src/plugins/props/util/ElementProp.js index 4abdc213..e7370b6c 100644 --- a/src/plugins/props/util/ElementProp.js +++ b/src/plugins/props/util/ElementProp.js @@ -251,7 +251,7 @@ export default class ElementProp { update (dependency) { let { spec } = this; - if (dependency && dependency.spec === spec.defaultProp) { + if (dependency && dependency.spec === spec.defaultProp && !spec.get) { let oldValue = this.value; this.value = dependency.value === undefined ? undefined From 0becf80561d2b62f4980b667eca1c8317650a939 Mon Sep 17 00:00:00 2001 From: Lea Verou Date: Thu, 28 May 2026 11:54:10 -0400 Subject: [PATCH 10/18] Fix stale attribute fields leaking across coalesced propchange events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a paused burst coalesced an attribute-source event followed by a property-source event on the same prop, Object.assign(prev, event) only overlaid event's own properties — leaving prev's attributeName, attributeValue, and oldAttributeValue intact even though the surviving dispatch was now source: "property". Keep the latest event of each run instead and rebase its oldValue to the burst-start value. Refs #129 Co-Authored-By: Claude Opus 4.7 (1M context) --- src/plugins/props/util/ElementProps.js | 10 +++---- test/plugins/props/propschange.js | 40 ++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/src/plugins/props/util/ElementProps.js b/src/plugins/props/util/ElementProps.js index c4713c7d..7c0ec7c2 100644 --- a/src/plugins/props/util/ElementProps.js +++ b/src/plugins/props/util/ElementProps.js @@ -181,16 +181,16 @@ export default class ElementProps extends Map { * `paused` setter on resume. */ #flushPending () { - // In-place coalesce consecutive same-prop entries: collapse later - // events into the run's first one, keeping its burst-start oldValue. + // In-place coalesce consecutive same-prop entries: keep the last + // event of each run (its fields describe the latest write), but + // rewrite its oldValue to the run's burst-start value. let writeIdx = 0; let lastName = null; for (let event of this.#eventQueue) { if (event.name === lastName) { let prev = this.#eventQueue[writeIdx - 1]; - let { oldValue } = prev; - Object.assign(prev, event); - prev.oldValue = oldValue; + event.oldValue = prev.oldValue; + this.#eventQueue[writeIdx - 1] = event; } else { this.#eventQueue[writeIdx++] = event; diff --git a/test/plugins/props/propschange.js b/test/plugins/props/propschange.js index eb2c76d9..e5a6ac2d 100644 --- a/test/plugins/props/propschange.js +++ b/test/plugins/props/propschange.js @@ -240,6 +240,46 @@ export default { propschange: [[["v", 1]]], }, }, + { + name: "Paused burst: coalescing attribute→property does not leak attribute fields", + description: + "When an attribute write is followed by a property write on the same prop during a pause, the resumed propchange must reflect the latest write — no stale attributeName/attributeValue/oldAttributeValue from the prior attribute event.", + async run () { + let { element } = this.data; + await flush(); + let propchangeLog = []; + element.addEventListener("propchange", e => + propchangeLog.push({ + source: e.source, + value: e.value, + attributeName: e.attributeName, + attributeValue: e.attributeValue, + oldAttributeValue: e.oldAttributeValue, + })); + + element.props.paused = true; + element.setAttribute("foo", "1"); + element.foo = 2; + element.props.paused = false; + + await flush(); + return propchangeLog; + }, + arg: { + props: { + foo: { type: Number, reflect: { from: true, to: false } }, + }, + }, + expect: [ + { + source: "property", + value: 2, + attributeName: undefined, + attributeValue: undefined, + oldAttributeValue: undefined, + }, + ], + }, { name: "updated() method auto-wires to propschange", async run () { From 8f5c234d56d89ac415f6678dc25605e381aab2c9 Mon Sep 17 00:00:00 2001 From: Lea Verou Date: Thu, 28 May 2026 11:55:50 -0400 Subject: [PATCH 11/18] Prettier --- src/plugins/props/util/ElementProp.js | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/plugins/props/util/ElementProp.js b/src/plugins/props/util/ElementProp.js index e7370b6c..bb0d4a4a 100644 --- a/src/plugins/props/util/ElementProp.js +++ b/src/plugins/props/util/ElementProp.js @@ -253,9 +253,10 @@ export default class ElementProp { if (dependency && dependency.spec === spec.defaultProp && !spec.get) { let oldValue = this.value; - this.value = dependency.value === undefined - ? undefined - : this.convert(spec.parse(dependency.value)); + this.value = + dependency.value === undefined + ? undefined + : this.convert(spec.parse(dependency.value)); this.source = "default"; this.changed({ source: "default", value: this.value, oldValue }); return; @@ -288,9 +289,6 @@ export default class ElementProp { let { spec, source } = this; let userOwned = source === "property" || source === "attribute"; - return ( - spec.dependencies.has(dep.name) || - (spec.defaultProp === dep.spec && !userOwned) - ); + return spec.dependencies.has(dep.name) || (spec.defaultProp === dep.spec && !userOwned); } } From b719f4bdc852beb24c80030092e992b739b7fc36 Mon Sep 17 00:00:00 2001 From: Lea Verou Date: Thu, 28 May 2026 11:56:06 -0400 Subject: [PATCH 12/18] Use `this.source` Co-Authored-By: Dmitry Sharabin --- src/plugins/props/util/ElementProp.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/props/util/ElementProp.js b/src/plugins/props/util/ElementProp.js index bb0d4a4a..152fd033 100644 --- a/src/plugins/props/util/ElementProp.js +++ b/src/plugins/props/util/ElementProp.js @@ -57,7 +57,7 @@ export default class ElementProp { // see a real value, not undefined. this.value = this.get(); this.source = "default"; - this.changed({ source: "default", value: this.value, oldValue: undefined }); + this.changed({ source: this.source, value: this.value, oldValue: undefined }); } } From 58e7d30a44ad509148f648840a4e32b3b80c359f Mon Sep 17 00:00:00 2001 From: Lea Verou Date: Thu, 28 May 2026 15:20:37 -0400 Subject: [PATCH 13/18] Split prop input from derived value in ElementProp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The wrapper used to store only the post-convert value, so a dep cascade on a convert prop re-ran convert on the already-converted result — silently wrong for non-idempotent converts, plus the same path double-converted via set() re-entry. Add an internalValue slot for the parsed-but-not-converted user input; value is now derived from it via a single #derive() that also unifies the defaultProp and default fallbacks. Falls out from the new model: - source taxonomy collapses to "property" | "attribute" | undefined; the removed "default" / "get" / "convert" labels described derivation steps, not input origins, and nothing in the codebase branched on them. - The user-ownership guard becomes structurally impossible to violate, so it's gone. - dependsOn switches from a source-string check to internalValue !== undefined. - update() loses its unused dep parameter. Issue #14's pre-mount bootstrap is preserved by honoring internalValue in #derive for get props and clearing it on the first cascade recompute. Adds a regression test (convert reading a sibling prop, sibling changes post-write) that fails on the old code and passes here. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/plugins/props/README.md | 2 +- src/plugins/props/util/ElementProp.js | 212 +++++++++++++--------- src/plugins/props/util/ElementProps.js | 2 +- src/plugins/props/util/PropChangeEvent.js | 17 +- test/plugins/events/propchange.js | 2 +- test/plugins/props/propchange.js | 24 +++ 6 files changed, 170 insertions(+), 89 deletions(-) diff --git a/src/plugins/props/README.md b/src/plugins/props/README.md index 49e2a4ee..541efc2a 100644 --- a/src/plugins/props/README.md +++ b/src/plugins/props/README.md @@ -242,7 +242,7 @@ element.addEventListener("propchange", e => { e.name; // prop name e.value; // new stored value e.oldValue; // previous stored value - e.source; // "property" | "attribute" | "get" | "default" | … + e.source; // "property" | "attribute" | undefined }); ``` diff --git a/src/plugins/props/util/ElementProp.js b/src/plugins/props/util/ElementProp.js index 152fd033..24ac0587 100644 --- a/src/plugins/props/util/ElementProp.js +++ b/src/plugins/props/util/ElementProp.js @@ -2,22 +2,49 @@ import { resolveValue } from "../util.js"; /** * Per-element wrapper around a {@link Prop} spec. - * Holds the current and previous value for one prop on one element, and the - * per-element behavior (get, set, update, cascade) for that prop. + * Holds the current input and derived value for one prop on one element, and + * the per-element behavior (get, set, update, cascade) for that prop. + * + * The two-slot model: + * - {@link internalValue} stores the user-supplied input (parsed, not yet + * converted). `undefined` means "no user input; fall through to default." + * - {@link value} stores the derived/visible value: `convert(input)`, where + * `input` is `internalValue` if set, otherwise the resolved default. It is + * eagerly cached on every write and on every dep-cascade recompute. + * + * Separating the slots means a dep cascade re-derives `value` from the + * untouched `internalValue` rather than re-running `convert` on an + * already-converted value, which is incorrect for non-idempotent converts. */ export default class ElementProp { - /** Current stored value (cached resolved default, computed result, or explicit user write). */ - value; + /** + * Parsed-but-not-converted input from a user write. `undefined` when no + * user input has landed (the prop is showing its default), including after + * an attribute removal collapses the input back to undefined. + */ + internalValue; + + /** + * Current derived value: `convert(internalValue ?? resolvedDefault)`. + * Not declared as a class field so {@link Object.hasOwn} can distinguish + * "never computed" from "computed to undefined" in the lazy {@link get} + * path — `undefined` is a legitimately cacheable result. + * @type {*} + */ /** Value prior to the most recent change. */ oldValue; /** - * Source of the current {@link value}: one of `"default"`, `"property"`, - * `"attribute"`, `"get"`, `"convert"`, or `undefined` before any value lands. - * `"property"` and `"attribute"` mark a user-owned value, which gates the - * defaultProp cascade (see {@link dependsOn}) and the reflect-on-first- - * explicit-write path in {@link set}. + * Origin label for the most recent user write: `"property"`, `"attribute"`, + * or `undefined` if no write has ever landed (the mount-time default fires + * with `undefined`). Persists across dep-cascade recomputes and across + * attribute removal — describes the origin of the input shape, not whether + * input is currently present. Use `internalValue !== undefined` to check + * "is the prop user-owned right now?" (see {@link dependsOn}). + * + * Plugins may introduce additional source values via {@link set}; the + * built-in code only emits `"property"`, `"attribute"`, or `undefined`. */ source; @@ -34,7 +61,7 @@ export default class ElementProp { // find this wrapper in the Map. props.set(spec.name, this); - let { name, reflect, defaultProp, get: computed } = spec; + let { name, reflect } = spec; let { element } = props; if (Object.hasOwn(element, name)) { @@ -51,13 +78,13 @@ export default class ElementProp { name: reflect.from, }); } - else if (!defaultProp && !computed) { - // Plain prop with a value-or-function default. Cache the resolved - // default into this.value so dependents reading it via the cascade - // see a real value, not undefined. - this.value = this.get(); - this.source = "default"; - this.changed({ source: this.source, value: this.value, oldValue: undefined }); + else if (!spec.defaultProp && !spec.get) { + // Plain prop. Derive eagerly so dependents reading via the cascade + // see a real value, and fire a mount event for listeners. + // (defaultProp and get props get their mount event via cascade from + // their dependency, so the constructor doesn't fire here.) + this.value = this.#derive(); + this.changed({ source: undefined, value: this.value, oldValue: undefined }); } } @@ -70,53 +97,77 @@ export default class ElementProp { } /** - * Read this prop's current value, falling back to its default if unset. + * Read this prop's current value, lazily deriving if the cache is empty. + * `Object.hasOwn` is the sentinel because a legitimately-cached `undefined` + * (e.g. a prop with no input and no default) must not trigger re-derivation + * on every read. */ get () { - if (this.value === undefined) { - this.update(); + if (!Object.hasOwn(this, "value")) { + this.value = this.#derive(); + } + return this.value; + } + + /** + * Compute the visible value from current state. Single source of truth for + * derivation; called on writes (after updating {@link internalValue}), on + * dep cascades, and on lazy reads. + */ + #derive () { + let { spec, element } = this; + + if (spec.get) { + // Computed prop. A pre-mount property/attribute write seeds + // internalValue (issue #14 bootstrap); honor it until update() + // clears it on the first dep-cascade recompute. + // + // Edge case: a computed prop with no declared deps never receives + // a cascade, so a bootstrap seed stays as the visible value forever + // — spec.get is effectively masked. Pre-mount writes to depless + // computeds are pathological config; the alternative (silently + // dropping the seed) seems worse. + let raw = + this.internalValue !== undefined + ? this.internalValue + : spec.parse(spec.get.call(element)); + return this.convert(raw); } - if (this.value !== undefined) { - return this.value; + if (this.internalValue !== undefined) { + return this.convert(this.internalValue); } - let { spec, element } = this; + // Fall back to default. `defaultProp` and `default` are two variants + // of the same concept; a function default with reactive deps is + // normalized to a synthetic defaultProp upstream (see Props#createProp), + // so most reactive defaults flow through the first branch. Reparse + // the resolved raw value to bridge it into this spec's shape. let raw; - if (spec.defaultProp) { raw = this.props.get(spec.defaultProp.name).get(); } else if (spec.default !== undefined) { raw = resolveValue(spec.default, [element, element]); } - else { - return undefined; - } - let value; - try { - value = spec.parse(raw); - } - catch (e) { - console.warn( - "Failed to parse default value", - raw, - `for prop ${this.name}. Original error was: `, - e, - ); - return null; - } - - return this.convert(value); + return raw === undefined ? undefined : this.convert(spec.parse(raw)); } /** - * Write a value: parse, convert, store, and reflect to attribute when applicable. + * Write a user-supplied value: parse, store as input, re-derive, reflect to + * attribute when applicable. * @param {*} value Raw value (string from an attribute, any from a property write). * @param {{source?: string, name?: string, oldAttributeValue?: string | null}} [options] */ set (value, { source, name, oldAttributeValue } = {}) { + if (source !== "property" && source !== "attribute") { + // Non-user sources don't write through internalValue; they're + // derivations. Route to the cascade entry point. + this.update(); + return; + } + let { spec } = this; let parsed; @@ -139,48 +190,42 @@ export default class ElementProp { parsed = undefined; } - parsed = this.convert(parsed); - - let isUserWrite = source === "property" || source === "attribute"; + this.internalValue = parsed; + let newValue = this.#derive(); - let wasUserOwned = this.source === "property" || this.source === "attribute"; + let wasUserOwned = this.source !== undefined; - if (spec.equals(parsed, this.value)) { - // Same value. First explicit write of the cached default still needs - // to reflect to the attribute (issue #105) and take ownership so the - // defaultProp cascade stops tracking us, but no propchange fires — - // nothing actually changed. - if (isUserWrite && !wasUserOwned) { + if (spec.equals(newValue, this.value)) { + // Same derived value. First explicit write of the cached default + // still needs to reflect to the attribute (issue #105) and shift + // ownership so the defaultProp cascade stops tracking us, but no + // propchange fires — nothing actually changed. + if (!wasUserOwned) { this.source = source; if (source === "property") { - this.#reflectToAttribute(parsed); + this.#reflectToAttribute(newValue); } } return; } - // Don't let a non-user source (e.g. a "get" recompute) silently strip - // user ownership established by a prior property/attribute write. - if (isUserWrite || !wasUserOwned) { - this.source = source; - } - this.oldValue = this.value; - this.value = parsed; + this.value = newValue; + this.source = source; let change = { source, - value: parsed, + value: newValue, oldValue: this.oldValue, }; if (source === "property") { - let reflected = this.#reflectToAttribute(parsed); + let reflected = this.#reflectToAttribute(newValue); if (reflected) { Object.assign(change, reflected); } } - else if (source === "attribute") { + else { Object.assign(change, { attributeName: name, attributeValue: value, @@ -245,36 +290,33 @@ export default class ElementProp { } /** - * Recalculate this prop's value (for computed props or `convert`) and store it. - * @param {ElementProp} [dependency] The dependency whose change triggered this update. + * Recalculate the derived value from the existing input (does not touch + * {@link internalValue}). Invoked when a dependency changes. */ - update (dependency) { + update () { let { spec } = this; - if (dependency && dependency.spec === spec.defaultProp && !spec.get) { - let oldValue = this.value; - this.value = - dependency.value === undefined - ? undefined - : this.convert(spec.parse(dependency.value)); - this.source = "default"; - this.changed({ source: "default", value: this.value, oldValue }); - return; + // First recompute on a computed prop discards any bootstrap value: + // from here on, spec.get is the source of truth. + if (spec.get && this.internalValue !== undefined) { + this.internalValue = undefined; + this.source = undefined; } - if (spec.get) { - let value = spec.get.call(this.element); - this.set(value, { source: "get" }); - } + let newValue = this.#derive(); - if (spec.convert && this.value !== undefined) { - this.set(this.convert(this.value), { source: "convert" }); + if (spec.equals(newValue, this.value)) { + return; } + + this.oldValue = this.value; + this.value = newValue; + this.changed({ source: this.source, value: newValue, oldValue: this.oldValue }); } /** * Whether this prop currently depends on `dep`'s value. - * Includes the default-prop link only while this prop is not user-owned. + * Includes the default-prop link only while this prop has no user input. * @param {ElementProp} dep * @returns {boolean} */ @@ -287,8 +329,8 @@ export default class ElementProp { return true; } - let { spec, source } = this; - let userOwned = source === "property" || source === "attribute"; + let { spec } = this; + let userOwned = this.internalValue !== undefined; return spec.dependencies.has(dep.name) || (spec.defaultProp === dep.spec && !userOwned); } } diff --git a/src/plugins/props/util/ElementProps.js b/src/plugins/props/util/ElementProps.js index 7c0ec7c2..54f18f55 100644 --- a/src/plugins/props/util/ElementProps.js +++ b/src/plugins/props/util/ElementProps.js @@ -153,7 +153,7 @@ export default class ElementProps extends Map { for (let depSpec of dependentSpecs) { let dep = this.get(depSpec.name); if (dep.dependsOn(ep)) { - dep.update(ep); + dep.update(); } } } diff --git a/src/plugins/props/util/PropChangeEvent.js b/src/plugins/props/util/PropChangeEvent.js index 66ca8e2b..db7c6737 100644 --- a/src/plugins/props/util/PropChangeEvent.js +++ b/src/plugins/props/util/PropChangeEvent.js @@ -5,7 +5,16 @@ * @property {*} value Current stored value (parsed + converted). * @property {*} oldValue Stored value before this change, or — for a coalesced * resume dispatch — the burst-start value the consumer was last told. - * @property {"default" | "property" | "attribute" | "get" | "convert"} source + * @property {"property" | "attribute" | undefined} source Origin label for + * the most recent user write — `"property"`, `"attribute"`, or `undefined` + * if no write has ever landed. Persists across dep-cascade recomputes + * *and* across attribute removal: source describes the origin shape of the + * prop's input, not whether input is currently present. A `source: + * "property"` event can therefore mean either "user just wrote" or "user + * wrote previously, and a cascade just recomputed the visible value" — + * diff `value` vs `oldValue` if you need to tell them apart. Plugins may + * introduce additional source values; the built-in code only emits the + * three above. * @property {string} [attributeName] Set when `source === "attribute"` or a * property write reflects to an attribute. * @property {string | null} [attributeValue] @@ -44,6 +53,12 @@ export default class PropChangeEvent extends Event { /** * Replay this change on a different element. Used to mirror a prop's * value onto a sub-element from a `propchange` listener. + * + * Only events whose `source` is `"property"` or `"attribute"` (i.e. a user + * write) are mirrored — cascade-driven changes carry `source: undefined` + * and are intentionally not replicated, since the target should be reached + * by its own cascade if it shares the same dep graph. + * * @param {Element} target Element to apply the change to. */ applyTo (target) { diff --git a/test/plugins/events/propchange.js b/test/plugins/events/propchange.js index 209bebb9..5b0b062c 100644 --- a/test/plugins/events/propchange.js +++ b/test/plugins/events/propchange.js @@ -28,7 +28,7 @@ export default { return element._log ?? []; }, - expect: [{ value: "x", source: "default" }], + expect: [{ value: "x", source: undefined }], }, { name: "Shortcut events ride along on coalesced propchange and don't feed propschange", diff --git a/test/plugins/props/propchange.js b/test/plugins/props/propchange.js index 28953d6e..95cc946f 100644 --- a/test/plugins/props/propchange.js +++ b/test/plugins/props/propchange.js @@ -160,6 +160,30 @@ export default { ["v", "baz"], ], }, + { + name: "Convert re-runs on the original input when a non-default dep changes", + description: + "A prop with a convert that reads another reactive prop must re-derive from the user's input, not from the previously converted value. Regression for the two-slot refactor: the old single-slot model re-ran convert on the already-converted value, which compounded for non-idempotent converts.", + run () { + let { element } = this.data; + element.multiplier = 2; + element.v = 5; // 5 * 2 = 10 + element.multiplier = 3; // should be 5 * 3 = 15, not 10 * 3 (or worse) + return element.v; + }, + arg: { + props: { + multiplier: { type: Number, default: 1 }, + v: { + type: Number, + convert (n) { + return n * this.multiplier; + }, + }, + }, + }, + expect: 15, + }, { name: "Writes that collapse to the same value after convert do not fire propchange", description: From 444cb3211531176936a2d7398fbcfc793575f0b7 Mon Sep 17 00:00:00 2001 From: Lea Verou Date: Thu, 28 May 2026 16:51:23 -0400 Subject: [PATCH 14/18] Support custom sources better --- src/plugins/props/util/ElementProp.js | 23 ++++++++++++++--------- src/plugins/props/util/PropChangeEvent.js | 2 +- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/plugins/props/util/ElementProp.js b/src/plugins/props/util/ElementProp.js index 24ac0587..7302b78c 100644 --- a/src/plugins/props/util/ElementProp.js +++ b/src/plugins/props/util/ElementProp.js @@ -154,6 +154,11 @@ export default class ElementProp { return raw === undefined ? undefined : this.convert(spec.parse(raw)); } + /** + * For validation and to distinguish user vs. cascade writes + */ + static sources = ["property", "attribute"]; + /** * Write a user-supplied value: parse, store as input, re-derive, reflect to * attribute when applicable. @@ -161,7 +166,7 @@ export default class ElementProp { * @param {{source?: string, name?: string, oldAttributeValue?: string | null}} [options] */ set (value, { source, name, oldAttributeValue } = {}) { - if (source !== "property" && source !== "attribute") { + if (!this.constructor.sources.includes(source)) { // Non-user sources don't write through internalValue; they're // derivations. Route to the cascade entry point. this.update(); @@ -202,7 +207,7 @@ export default class ElementProp { // propchange fires — nothing actually changed. if (!wasUserOwned) { this.source = source; - if (source === "property") { + if (source !== "attribute") { this.#reflectToAttribute(newValue); } } @@ -219,19 +224,19 @@ export default class ElementProp { oldValue: this.oldValue, }; - if (source === "property") { - let reflected = this.#reflectToAttribute(newValue); - if (reflected) { - Object.assign(change, reflected); - } - } - else { + if (source === "attribute") { Object.assign(change, { attributeName: name, attributeValue: value, oldAttributeValue, }); } + else { + let reflected = this.#reflectToAttribute(newValue); + if (reflected) { + Object.assign(change, reflected); + } + } this.changed(change); } diff --git a/src/plugins/props/util/PropChangeEvent.js b/src/plugins/props/util/PropChangeEvent.js index db7c6737..316c005b 100644 --- a/src/plugins/props/util/PropChangeEvent.js +++ b/src/plugins/props/util/PropChangeEvent.js @@ -70,7 +70,7 @@ export default class PropChangeEvent extends Event { target.setAttribute(this.attributeName, this.attributeValue); } } - else if (this.source === "property") { + else { target[this.name] = this.value; } } From 92f3e0f66f9da8ea0021a66adb920f2bc7ae7fe5 Mon Sep 17 00:00:00 2001 From: Lea Verou Date: Thu, 28 May 2026 17:31:57 -0400 Subject: [PATCH 15/18] Update src/plugins/props/util/PropChangeEvent.js Co-authored-by: Dmitry Sharabin --- src/plugins/props/util/PropChangeEvent.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/props/util/PropChangeEvent.js b/src/plugins/props/util/PropChangeEvent.js index 316c005b..42bce459 100644 --- a/src/plugins/props/util/PropChangeEvent.js +++ b/src/plugins/props/util/PropChangeEvent.js @@ -70,7 +70,7 @@ export default class PropChangeEvent extends Event { target.setAttribute(this.attributeName, this.attributeValue); } } - else { + else if (this.source) { target[this.name] = this.value; } } From ca4813649677c1106ce443a4447bf8ce88d0d96e Mon Sep 17 00:00:00 2001 From: Lea Verou Date: Thu, 28 May 2026 19:20:44 -0400 Subject: [PATCH 16/18] Fire mount event for every prop, not just plain-default ones MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Computed and defaultProp props used to rely on a cascade from an upstream prop to deliver their mount event. That breaks when the upstream isn't a declared prop — e.g. `default() { return this.textContent }` gets promoted to a synthetic computed depending on `textContent`, but nothing reactive sits on the other end of that edge, so no cascade ever fires. The pre-refactor code masked this because `get()` had side effects: lazy reads of an orphaned defaultProp chain would fire the mount event as a byproduct of resolving the value. The two-slot refactor made `get()` side-effect-free, which exposed the latent gap. Drop the constructor's plain-prop gate. Every prop now eagerly derives and fires; a later dep-cascade `update()` short-circuits on the equality check, so single-firing is preserved. Adds a regression test (props with a `default()` that reads a non-prop receiver member). Co-Authored-By: Claude Opus 4.7 (1M context) --- src/plugins/props/util/ElementProp.js | 9 ++++----- test/plugins/props/propchange.js | 29 +++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/src/plugins/props/util/ElementProp.js b/src/plugins/props/util/ElementProp.js index 7302b78c..404aa057 100644 --- a/src/plugins/props/util/ElementProp.js +++ b/src/plugins/props/util/ElementProp.js @@ -78,11 +78,10 @@ export default class ElementProp { name: reflect.from, }); } - else if (!spec.defaultProp && !spec.get) { - // Plain prop. Derive eagerly so dependents reading via the cascade - // see a real value, and fire a mount event for listeners. - // (defaultProp and get props get their mount event via cascade from - // their dependency, so the constructor doesn't fire here.) + else { + // Every prop fires a mount event. Deps govern *re*-computation, not + // whether the initial derivation happens; a later cascade short-circuits + // on the equality check in update(). this.value = this.#derive(); this.changed({ source: undefined, value: this.value, oldValue: undefined }); } diff --git a/test/plugins/props/propchange.js b/test/plugins/props/propchange.js index 95cc946f..19a35604 100644 --- a/test/plugins/props/propchange.js +++ b/test/plugins/props/propchange.js @@ -65,6 +65,35 @@ export default { ["derived", 8], ], }, + { + name: "default() that reads a non-prop receiver member still fires on mount", + description: + "`inferDependencies` promotes a `default() { return this.X }` to a synthetic " + + "computed depending on `X` regardless of whether `X` is a declared prop. " + + "When `X` isn't a prop (e.g. `textContent`, a mixin getter, an instance field), " + + "no upstream cascade can reach the synthetic, so the mount event has to come " + + "from the prop's own constructor.", + arg: { + mixin (Class) { + Object.defineProperty(Class.prototype, "content", { + get () { + return "hi"; + }, + }); + }, + props: { + value: { + default () { + return this.content; + }, + }, + }, + }, + expect: [ + ["defaultValue", "hi"], + ["value", "hi"], + ], + }, ], }, { From 457750e496b59f4075322b13c7e5d2145a2b3aad Mon Sep 17 00:00:00 2001 From: Dmitry Sharabin Date: Fri, 29 May 2026 09:44:36 +0200 Subject: [PATCH 17/18] Add propschange regression tests for round-trip and re-entrant writes Two cases from PR review feedback: - a round-trip on one prop drops out while a genuinely changed prop stays - a write from inside a propchange handler joins the same propschange burst Co-Authored-By: Claude Opus 4.8 (1M context) --- test/plugins/props/propschange.js | 47 +++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/test/plugins/props/propschange.js b/test/plugins/props/propschange.js index e5a6ac2d..4dafdf03 100644 --- a/test/plugins/props/propschange.js +++ b/test/plugins/props/propschange.js @@ -98,6 +98,28 @@ export default { arg: { props: { v: { type: Number, default: 0 } } }, expect: { propchangeAfter: 2, propschangeAfter: 0 }, }, + { + name: "Round-trip on one prop drops it; genuinely changed prop stays", + async run () { + let { element } = this.data; + await flush(); // drain mount-time propschange + this.data.propsEvents.length = 0; + + element.a = 5; + element.a = 0; // back to stored default + element.b = "new"; + + await flush(); + return this.data.propsEvents; + }, + arg: { + props: { + a: { type: Number, default: 0 }, + b: { type: String, default: "old" }, + }, + }, + expect: [[["b", "old"]]], + }, { name: "Cascade: dependents land in the same propschange", description: @@ -129,6 +151,31 @@ export default { ], ], }, + { + name: "Re-entrant write from a propchange handler joins the same propschange", + async run () { + let { element } = this.data; + await flush(); // drain mount-time propschange + this.data.propsEvents.length = 0; + + element.addEventListener("propchange", e => { + if (e.name === "a") { + element.b = "from_listener"; + } + }); + + element.a = "new"; + await flush(); + return this.data.propsEvents; + }, + arg: { props: { a: { default: "foo" }, b: { default: "bar" } } }, + expect: [ + [ + ["a", "foo"], + ["b", "bar"], + ], + ], + }, { name: "Disconnect/reconnect: revert propchange dispatched, no propschange (dispatched round-trip)", description: From caf6f2c4d1ee30c352036c349c14b3e16466ec2b Mon Sep 17 00:00:00 2001 From: Dmitry Sharabin Date: Mon, 1 Jun 2026 17:27:03 +0200 Subject: [PATCH 18/18] Guard spec.parse(), event cloning, and remove applyTo() (#132) Co-authored-by: Claude Opus 4.6 (1M context) --- src/plugins/props/util/ElementProp.js | 36 +++++++++++++++++++---- src/plugins/props/util/PropChangeEvent.js | 33 ++++----------------- 2 files changed, 36 insertions(+), 33 deletions(-) diff --git a/src/plugins/props/util/ElementProp.js b/src/plugins/props/util/ElementProp.js index 404aa057..6f508148 100644 --- a/src/plugins/props/util/ElementProp.js +++ b/src/plugins/props/util/ElementProp.js @@ -126,10 +126,22 @@ export default class ElementProp { // — spec.get is effectively masked. Pre-mount writes to depless // computeds are pathological config; the alternative (silently // dropping the seed) seems worse. - let raw = - this.internalValue !== undefined - ? this.internalValue - : spec.parse(spec.get.call(element)); + let raw; + if (this.internalValue !== undefined) { + raw = this.internalValue; + } + else { + try { + raw = spec.parse(spec.get.call(element)); + } + catch (e) { + console.warn( + `Failed to parse computed value for prop ${this.name}. Original error was:`, + e, + ); + return; + } + } return this.convert(raw); } @@ -150,7 +162,21 @@ export default class ElementProp { raw = resolveValue(spec.default, [element, element]); } - return raw === undefined ? undefined : this.convert(spec.parse(raw)); + if (raw === undefined) { + return; + } + + try { + raw = spec.parse(raw); + } + catch (e) { + console.warn( + `Failed to parse default value for prop ${this.name}. Original error was:`, + e, + ); + return; + } + return this.convert(raw); } /** diff --git a/src/plugins/props/util/PropChangeEvent.js b/src/plugins/props/util/PropChangeEvent.js index 42bce459..1b9603ee 100644 --- a/src/plugins/props/util/PropChangeEvent.js +++ b/src/plugins/props/util/PropChangeEvent.js @@ -41,37 +41,14 @@ export default class PropChangeEvent extends Event { // and silently ignores the rest. super(type, options); - // Assign the rest as own properties, skipping anything Event already - // owns (init dict, state like `target`/`defaultPrevented`, methods). + // Assign the rest as own properties, skipping anything Event + // already owns (init dict, state, unforgeable own properties). + // NB: do NOT use class fields — they create own properties that + // would shadow options and break this check. for (let [key, value] of Object.entries(options)) { - if (!(key in Event.prototype)) { + if (!(key in this)) { this[key] = value; } } } - - /** - * Replay this change on a different element. Used to mirror a prop's - * value onto a sub-element from a `propchange` listener. - * - * Only events whose `source` is `"property"` or `"attribute"` (i.e. a user - * write) are mirrored — cascade-driven changes carry `source: undefined` - * and are intentionally not replicated, since the target should be reached - * by its own cascade if it shares the same dep graph. - * - * @param {Element} target Element to apply the change to. - */ - applyTo (target) { - if (this.source === "attribute") { - if (this.attributeValue === null) { - target.removeAttribute(this.attributeName); - } - else { - target.setAttribute(this.attributeName, this.attributeValue); - } - } - else if (this.source) { - target[this.name] = this.value; - } - } }