diff --git a/src/plugins/events/onprops.js b/src/plugins/events/onprops.js index 9a184247..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"; @@ -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,40 +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 - let change = event.detail; - - if (change.oldValue) { - this.removeEventListener(eventName, change.oldValue); - } - - if (change.value) { - this.addEventListener(eventName, change.value); - } - } - }); - }, }; const providesStatic = {}; diff --git a/src/plugins/events/propchange.js b/src/plugins/events/propchange.js index a8159efa..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; @@ -18,42 +18,55 @@ 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 () { - // 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]; - let value = this[propName]; + constructor () { + let aliases = this.constructor[propchange]; + if (!aliases) { + return; + } - if (value === undefined) { - continue; + // 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. 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) { + return; } - let prop = this.props.get(propName); - let detail = { source: "initial", value }; - this.dispatchEvent(new PropChangeEvent(eventName, { name: propName, prop, detail })); - } + 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, + })); + } + }); }, }; diff --git a/src/plugins/props/README.md b/src/plugins/props/README.md index 00c5f90d..541efc2a 100644 --- a/src/plugins/props/README.md +++ b/src/plugins/props/README.md @@ -228,3 +228,59 @@ 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" | undefined +}); +``` + +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/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 4b7a1541..a7c8ca16 100644 --- a/src/plugins/props/index.js +++ b/src/plugins/props/index.js @@ -1,94 +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.propChangedCallback && this.constructor[props]) { - 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 new file mode 100644 index 00000000..1a244d3d --- /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 "./base.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/ElementProp.js b/src/plugins/props/util/ElementProp.js index ef69b671..6f508148 100644 --- a/src/plugins/props/util/ElementProp.js +++ b/src/plugins/props/util/ElementProp.js @@ -2,22 +2,52 @@ 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. `undefined` means no explicit value has been set, - * which triggers default resolution on read. + * 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. */ - value; + internalValue; /** - * Last value that was different from the current one (i.e. the value - * before the most recent change that passed the equality check). + * 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; + /** + * 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; + /** * @param {ElementProps} props The owning per-element collection. * @param {Prop} spec The class-level prop spec. @@ -31,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)) { @@ -48,11 +78,12 @@ export default class ElementProp { name: reflect.from, }); } - 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" }); + 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 }); } } @@ -65,54 +96,109 @@ 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; + } - if (this.value !== undefined) { - 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; + 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); } - let { spec, element } = this; - let raw; + if (this.internalValue !== undefined) { + return this.convert(this.internalValue); + } + // 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; + + if (raw === undefined) { + return; } - let value; try { - value = spec.parse(raw); + raw = spec.parse(raw); } catch (e) { console.warn( - "Failed to parse default value", - raw, - `for prop ${this.name}. Original error was: `, + `Failed to parse default value for prop ${this.name}. Original error was:`, e, ); - return null; + return; } - - return this.convert(value); + return this.convert(raw); } /** - * Write a value: parse, convert, store, and reflect to attribute when applicable. + * 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. * @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 } = {}) { - let { spec, element } = this; + 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(); + return; + } + + let { spec } = this; let parsed; try { @@ -134,52 +220,87 @@ export default class ElementProp { parsed = undefined; } - parsed = this.convert(parsed); + this.internalValue = parsed; + let newValue = this.#derive(); + + let wasUserOwned = this.source !== undefined; - if (spec.equals(parsed, this.value)) { + 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 !== "attribute") { + this.#reflectToAttribute(newValue); + } + } return; } 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" && 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); - } - } - else if (source === "attribute") { + 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); } + /** + * 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 @@ -199,32 +320,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) { - // 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" }); - 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 has no explicit value. + * Includes the default-prop link only while this prop has no user input. * @param {ElementProp} dep * @returns {boolean} */ @@ -238,9 +360,7 @@ export default class ElementProp { } let { spec } = this; - return ( - spec.dependencies.has(dep.name) || - (spec.defaultProp === dep.spec && this.value === undefined) - ); + 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 adb49ed5..54f18f55 100644 --- a/src/plugins/props/util/ElementProps.js +++ b/src/plugins/props/util/ElementProps.js @@ -6,15 +6,15 @@ 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. + * 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,23 +29,17 @@ 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); - } + this.#flushPending(); } } - /** - * 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. + * Paused-pending `propchange` events in write order. Empty during + * unpaused operation — live writes go straight through + * {@link #firePropChangeEvent}. * @type {PropChangeEvent[]} */ #eventQueue = []; @@ -136,19 +130,20 @@ export default class ElementProps extends Map { } /** - * Fire propchange events for `ep` and cascade updates to any dependents. + * 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 eventNames = ["propchange", ...(ep.spec.eventNames ?? [])]; - for (let eventName of eventNames) { - this.#firePropChangeEvent(eventName, { - name: ep.name, - prop: ep, - detail: change, - }); + let event = new PropChangeEvent("propchange", { name: ep.name, prop: ep, ...change }); + + if (this.#paused) { + this.#eventQueue.push(event); + } + else { + this.#firePropChangeEvent(event); } // Update all props that depend on this one. {@link get} materializes a @@ -158,24 +153,58 @@ 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(); } } } /** - * Dispatch (or queue, if paused) a propchange-style event. - * @param {string} eventName - * @param {{name: string, prop: ElementProp, detail: Object}} eventProps + * 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 (eventName, eventProps) { - let event = new PropChangeEvent(eventName, eventProps); + #firePropChangeEvent (event) { + if (event.prop.spec.equals(event.oldValue, event.value)) { + return; + } - if (this.#paused) { - this.#eventQueue.push(event); + this.element.dispatchEvent(event); + } + + /** + * 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: 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]; + event.oldValue = prev.oldValue; + this.#eventQueue[writeIdx - 1] = event; + } + else { + this.#eventQueue[writeIdx++] = event; + lastName = event.name; + } } - else { - this.element.dispatchEvent(event); + 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); } } } 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/src/plugins/props/util/PropChangeEvent.js b/src/plugins/props/util/PropChangeEvent.js index 9a6314f0..1b9603ee 100644 --- a/src/plugins/props/util/PropChangeEvent.js +++ b/src/plugins/props/util/PropChangeEvent.js @@ -1,29 +1,54 @@ -export default class PropChangeEvent extends CustomEvent { - constructor (type, { name, prop, ...options } = {}) { - super(type, options); - - this.name = name; - this.prop = prop; - } +/** + * @typedef {object} PropChangeEventProps + * @property {string} name Prop name. + * @property {import("./ElementProp.js").default} prop + * @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 {"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] + * @property {string | null} [oldAttributeValue] + * + * @typedef {EventInit & PropChangeEventProps} PropChangeEventInit + */ +/** + * Per-prop change event. Fires synchronously inside a property/attribute + * 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} + */ +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. - * @param {Element} target Element to apply the change to. + * @param {string} type + * @param {PropChangeEventInit} options */ - applyTo (target) { - let { source, attributeName, attributeValue, value } = this.detail; + constructor (type, options = {}) { + // Event picks out its init-dict keys (bubbles, cancelable, composed) + // and silently ignores the rest. + super(type, options); - if (source === "attribute") { - if (attributeValue === null) { - target.removeAttribute(attributeName); - } - else { - target.setAttribute(attributeName, attributeValue); + // 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 this)) { + this[key] = value; } } - else if (source === "property") { - target[this.name] = value; - } } } diff --git a/test/plugins/events/propchange.js b/test/plugins/events/propchange.js index 50316b67..5b0b062c 100644 --- a/test/plugins/events/propchange.js +++ b/test/plugins/events/propchange.js @@ -1,14 +1,17 @@ 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"; +const flush = () => Promise.resolve(); + export default { name: "Shortcut events", tests: [ { - name: "first_connected shortcut re-fire carries a detail", - 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], @@ -16,18 +19,55 @@ 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(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: [{ value: "x", source: undefined }], + }, + { + 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, propschangePlugin, 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]]], }, - expect: [true], }, ], }; diff --git a/test/plugins/props/index.js b/test/plugins/props/index.js index ff8afcb1..92cede48 100644 --- a/test/plugins/props/index.js +++ b/test/plugins/props/index.js @@ -1,9 +1,10 @@ -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"; 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 = []; @@ -42,6 +43,7 @@ export default { tests: [reflection, defaults, computed, propchange, lifecycle], }, + propschange, inheritance, install, observedAttributes, 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/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/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/propchange.js b/test/plugins/props/propchange.js index bdcb7da7..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"], + ], + }, ], }, { @@ -145,7 +174,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"; @@ -160,8 +189,34 @@ 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: + "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 +235,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..4dafdf03 --- /dev/null +++ b/test/plugins/props/propschange.js @@ -0,0 +1,352 @@ +/** + * `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. + */ +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"; + +const flush = () => Promise.resolve(); + +export default { + 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: [ + { + 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: "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: + "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: "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: + "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: 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 () { + 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: "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 () { + 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]]], + }, + ], +}; 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; }