diff --git a/src/plugins/events/onprops.js b/src/plugins/events/onprops.js index 9a5be0ac..9a184247 100644 --- a/src/plugins/events/onprops.js +++ b/src/plugins/events/onprops.js @@ -81,12 +81,12 @@ const hooks = { // Implement onEventName attributes/properties let change = event.detail; - if (change.oldInternalValue) { - this.removeEventListener(eventName, change.oldInternalValue); + if (change.oldValue) { + this.removeEventListener(eventName, change.oldValue); } - if (change.parsedValue) { - this.addEventListener(eventName, change.parsedValue); + if (change.value) { + this.addEventListener(eventName, change.value); } } }); diff --git a/src/plugins/events/propchange.js b/src/plugins/events/propchange.js index ee37f60a..6f8dff84 100644 --- a/src/plugins/events/propchange.js +++ b/src/plugins/events/propchange.js @@ -41,7 +41,10 @@ const hooks = { }, first_connected () { - // Often propchange events have already fired by the time the event handlers are added + // Don't remove: re-fires initial propchange for on*= attribute handlers, + // which onprops attaches *after* the initial dispatch. Without this, + // shortcut handlers declared in HTML never see the initial event. + // Pre-connect imperative listeners receive the event twice. for (let eventName in this.constructor[propchange]) { let propName = this.constructor[propchange][eventName]; let value = this[propName]; diff --git a/src/plugins/props/README.md b/src/plugins/props/README.md index b8a34659..da81e616 100644 --- a/src/plugins/props/README.md +++ b/src/plugins/props/README.md @@ -163,3 +163,65 @@ 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`. + +**Defaults are not reflected to attributes** — only user-set values are. + +**Restoring the default after a prior write:** both `el.prop = undefined` and `el.removeAttribute(name)` restore the default value **and** clear any previously-reflected attribute. + +## Observing changes + +There are two layers of observability for prop changes, both first-class: + +| Granularity | Event | Auto-wired callback | +| ---------------- | ------------- | ---------------------------- | +| Per prop | `propchange` | `propChangedCallback(event)` | +| Per drain (bulk) | `propschange` | `updated(event)` | + +Sync writes are coalesced into a single drain on the next microtask, so `el.x = 1; el.x = 2; el.x = 3` produces one `propchange` event with `value: 3`. `oldValue` is pinned to the pre-write value, so the event reports the full first→last delta. + +### Per-prop: `propchange` event and `propChangedCallback` + +Fires once per changed prop after the value settles. + +```js +class MyElement extends NudeElement { + propChangedCallback (event) { + console.log(event.name, event.detail.value); + } +} + +// External listeners work the same way: +el.addEventListener("propchange", e => { /* … */ }); +``` + +The event detail includes `source` (`"property"`, `"attribute"`, `"default"`, `"convert"`, `"get"`, or `"initial"` — for shortcut events re-fired on first connect to catch late-bound listeners), `value`, `oldValue`, and (when applicable) `attributeName`, `attributeValue`, `oldAttributeValue`. + +### Per-drain: `propschange` event and `updated()` callback + +Fires once at the end of every drain cycle, after every per-prop `propchange`. `event.changedProps` is `Map` — keys are the names of props that changed in this cycle, values are the previous value of each. Read the current value via `this[name]`. + +```js +class MyElement extends NudeElement { + updated (event) { + for (let [name, oldValue] of event.changedProps) { + console.log(name, oldValue, "→", this[name]); + } + } +} + +// External listeners receive the same event: +el.addEventListener("propschange", e => { + for (let [name, oldValue] of e.changedProps) { /* … */ } +}); +``` + +Use this for work you only want to run once per cycle (re-rendering a sub-tree, persisting to storage, computing derived state across multiple props). + +### Cycle ordering + +Attribute reflection is synchronous with property writes, so the DOM is up to date before the drain runs. Within a single drain: + +1. **`propchange` events** — one per changed prop. Custom shortcut events (registered via the `events` plugin's `propchange:` option) also fire here from the same payload. +2. **`propschange` event** + `updated()` callback — last, with the full Map. + +Handlers in step 2 see the fully-settled state and can read any prop's post-cascade value via `this[name]`. diff --git a/src/plugins/props/index.js b/src/plugins/props/index.js index 49b4a5c1..a79c0e5b 100644 --- a/src/plugins/props/index.js +++ b/src/plugins/props/index.js @@ -30,9 +30,19 @@ const hooks = { }, constructor () { - if (this.propChangedCallback && this.constructor[props]) { + if (!this.constructor[props]) { + return; + } + + // Per-prop callback: auto-wire to propchange events. + if (this.propChangedCallback) { this.addEventListener("propchange", this.propChangedCallback); } + + // Bulk callback: auto-wire to the propschange event. + if (this.updated) { + this.addEventListener("propschange", this.updated); + } }, first_constructor_static, @@ -40,6 +50,10 @@ const hooks = { constructed () { this.constructor[props].initializeFor(this); }, + + connected () { + this.constructor[props].connected(this); + }, }; const provides = { diff --git a/src/plugins/props/util/Prop.js b/src/plugins/props/util/Prop.js index 2e072d7a..c28f1873 100644 --- a/src/plugins/props/util/Prop.js +++ b/src/plugins/props/util/Prop.js @@ -97,16 +97,18 @@ let Self = class Prop { } /** - * Subscriber for Computed signals (spec.get, spec.convert, spec.default). - * Updates element.props cache, reflects to attributes if opted in, - * and fires propchange events. + * Side-effect handler for Computed-backed props (spec.get, spec.convert, spec.default). + * Fires on first compute, user write, and tracked dep change. Updates the + * element.props cache, reflects to the attribute if opted in, and dispatches + * the propchange event. */ #onComputedChange (element, source, newValue, oldValue) { element.props[this.name] = newValue; - // Reflect to attribute if this prop opts in + // `null` for defaults: don't synthesize an attribute (would clobber pre-mount + // writes — #105) and clear any stale attribute left over from a prior write. if (this.toAttribute) { - let attributeValue = this.stringify(newValue); + let attributeValue = source === "default" ? null : this.stringify(newValue); let oldAttributeValue = element.getAttribute(this.toAttribute); if (oldAttributeValue !== attributeValue) { element.ignoredAttributes.add(this.toAttribute); @@ -120,11 +122,16 @@ let Self = class Prop { } } + // Gate event on value change (the Computed uses force). + if (this.equals(newValue, oldValue)) { + return; + } + this.changed(element, { element, source, - parsedValue: newValue, - oldInternalValue: oldValue, + value: newValue, + oldValue, }); } @@ -143,7 +150,9 @@ let Self = class Prop { if (!signal) { // Delegate equality to the prop's type-aware equality. - let options = { equals: (a, b) => this.equals(a, b) }; + // Explicit user writes still reach the subscriber when + // the Computed dedupes against a cached default-resolved value (#105). + let options = { equals: (a, b) => this.equals(a, b), force: true }; if (this.spec.get) { signal = new Computed(() => this.spec.get.call(element), options); @@ -157,7 +166,7 @@ let Self = class Prop { let rawSignal = new Signal(undefined, options); this.#rawSignals.set(element, rawSignal); - let source = this.spec.convert ? "convert" : "default"; + let source = this.spec.convert ? "convert" : "property"; signal = new Computed(() => { let value = rawSignal.value; if (value === undefined && this.spec.default !== undefined) { @@ -181,7 +190,8 @@ let Self = class Prop { return value; }, options); signal.subscribe((newValue, oldValue) => { - this.#onComputedChange(element, source, newValue, oldValue); + let newSource = rawSignal.value === undefined ? "default" : source; + this.#onComputedChange(element, newSource, newValue, oldValue); }); } else { @@ -210,9 +220,7 @@ let Self = class Prop { // Force first compute so the subscriber emits the initial propchange signal.value; } - else { - this.changed(element, { source: "default", element }); - } + // Plain Signals start at undefined: nothing to fire about at mount. } this.#initialized = true; @@ -249,14 +257,13 @@ let Self = class Prop { return signal.value; } - set (element, value, { source, name, oldValue } = {}) { + set (element, value, { source, name, oldAttributeValue } = {}) { let signal = this.getSignal(element); let rawSignal = this.#rawSignals.get(element); // For Computed-backed props, compare against the raw user-set value - let oldInternalValue = (rawSignal ?? signal).value; + let oldValue = (rawSignal ?? signal).value; - let attributeName = name; let parsedValue; try { @@ -271,7 +278,14 @@ let Self = class Prop { return; } - if (this.equals(parsedValue, oldInternalValue)) { + // removeAttribute() arrives as null; collapse to undefined so the prop + // reverts to its natural empty state (default, if any, otherwise just + // undefined). Property writes of null remain a legitimate user value. + if (source === "attribute" && parsedValue === null) { + parsedValue = undefined; + } + + if (this.equals(parsedValue, oldValue)) { return; } @@ -289,10 +303,8 @@ let Self = class Prop { let change = { element, source, - value, - parsedValue, - oldInternalValue, - attributeName: name, + value: parsedValue, + oldValue, }; if (source === "property") { @@ -305,7 +317,12 @@ let Self = class Prop { element.ignoredAttributes.add(this.toAttribute); Object.assign(change, { attributeName, attributeValue, oldAttributeValue }); - this.applyChange(element, { ...change, source: "attribute" }); + if (attributeValue === null) { + element.removeAttribute(attributeName); + } + else { + element.setAttribute(attributeName, attributeValue); + } element.ignoredAttributes.delete(attributeName); } @@ -313,9 +330,9 @@ let Self = class Prop { } else if (source === "attribute") { Object.assign(change, { - attributeName, + attributeName: name, attributeValue: value, - oldAttributeValue: oldValue, + oldAttributeValue, }); } diff --git a/src/plugins/props/util/Props.js b/src/plugins/props/util/Props.js index 0586bd82..56dc6381 100644 --- a/src/plugins/props/util/Props.js +++ b/src/plugins/props/util/Props.js @@ -1,7 +1,12 @@ import Prop from "./Prop.js"; import PropChangeEvent from "./PropChangeEvent.js"; +import PropsChangeEvent from "./PropsChangeEvent.js"; export default class Props extends Map { + #eventDispatchQueue = new WeakMap(); + #pendingElements = new Set(); + #drainScheduled = false; + /** * * @param {HTMLElement} Class The class to define props for @@ -46,39 +51,103 @@ export default class Props extends Map { let propsFromAttribute = [...this.values()].filter(spec => spec.fromAttribute === name); for (let prop of propsFromAttribute) { - prop.set(element, element.getAttribute(name), { source: "attribute", name, oldValue }); + prop.set(element, element.getAttribute(name), { + source: "attribute", + name, + oldAttributeValue: oldValue, + }); } } - eventDispatchQueue = new WeakMap(); - /** - * Called when a prop value changes. Fires propchange events. - * Dependency propagation is handled automatically by signals. + * Called from Prop#changed when a value settles. Coalesces into the + * dispatch queue; dispatch happens in #drainFor on the next microtask. */ propChanged (element, prop, change) { - // Fire propchange event - let eventNames = ["propchange", ...(prop.eventNames ?? [])]; - for (let eventName of eventNames) { - this.firePropChangeEvent(element, eventName, { - name: prop.name, - prop, - detail: change, - }); + let map = this.#eventDispatchQueue.get(element); + if (!map) { + map = new Map(); + this.#eventDispatchQueue.set(element, map); + } + + let existing = map.get(prop.name); + if (existing) { + // Coalesce: latest value/source wins, but old values stay pinned + // to the first write so the payload spans the full first→last delta. + let { oldValue, oldAttributeValue } = existing.detail; + Object.assign(existing.detail, change, { oldValue, oldAttributeValue }); + } + else { + map.set(prop.name, { name: prop.name, prop, detail: { ...change } }); + } + + this.#pendingElements.add(element); + this.#scheduleDrain(); + } + + #scheduleDrain () { + if (this.#drainScheduled) { + return; } + + this.#drainScheduled = true; + queueMicrotask(() => { + this.#drainScheduled = false; + this.#drain(); + }); } - firePropChangeEvent (element, eventName, eventProps) { - let event = new PropChangeEvent(eventName, eventProps); + #drain () { + // Snapshot and clear: events queued by handlers (incl. on other + // elements) run on the next microtask, not in this drain. + let elements = [...this.#pendingElements]; + this.#pendingElements.clear(); - if (element.isConnected && eventProps.prop.initialized) { - element.dispatchEvent?.(event); + for (let element of elements) { + this.#drainFor(element); } - else { - let queue = this.eventDispatchQueue.get(element) ?? []; - queue.push(event); - this.eventDispatchQueue.set(element, queue); + } + + #drainFor (element) { + if (!element.isConnected) { + // Queue stays intact; `connected()` drains it on (re)connect. + return; } + + let map = this.#eventDispatchQueue.get(element); + if (!map) { + return; + } + + // Detach the queue before dispatch: re-entrant writes from event + // handlers must accumulate for the next drain, not this one. + let entries = [...map]; + this.#eventDispatchQueue.delete(element); + + let changedProps = new Map(); + for (let [, payload] of entries) { + // Plain Signals don't dedupe coalesced round-trips on their own; + // mirror Signal equality here. + let { prop, detail } = payload; + if (prop.equals(detail.value, detail.oldValue)) { + continue; + } + + changedProps.set(prop.name, detail.oldValue); + + // EventTarget isolates listener throws — siblings stay safe without try/catch. + for (let name of ["propchange", ...(prop.eventNames ?? [])]) { + element.dispatchEvent(new PropChangeEvent(name, payload)); + } + } + + if (changedProps.size > 0) { + element.dispatchEvent(new PropsChangeEvent("propschange", { changedProps })); + } + } + + connected (element) { + this.#drainFor(element); } initializeFor (element) { @@ -97,15 +166,7 @@ export default class Props extends Map { prop.initializeFor(element); } - // Dispatch any events that were queued - let queue = this.eventDispatchQueue.get(element); - - if (queue) { - for (let event of queue) { - element.dispatchEvent?.(event); - } - - this.eventDispatchQueue.delete(element); - } + // Drain synchronously so callers see initial state without waiting for the microtask. + this.#drainFor(element); } } diff --git a/src/plugins/props/util/PropsChangeEvent.js b/src/plugins/props/util/PropsChangeEvent.js new file mode 100644 index 00000000..23a94ac9 --- /dev/null +++ b/src/plugins/props/util/PropsChangeEvent.js @@ -0,0 +1,7 @@ +export default class PropsChangeEvent extends CustomEvent { + constructor (type, { changedProps, ...options } = {}) { + super(type, options); + + this.changedProps = changedProps; + } +} diff --git a/src/signals.js b/src/signals.js index 38dd9579..a0deca07 100644 --- a/src/signals.js +++ b/src/signals.js @@ -47,16 +47,21 @@ function flushDirty () { export class Signal extends EventTarget { #value; #subscribers = new Set(); + #force; /** * @param {*} value - Initial value. * @param {object} [options] * @param {(a: *, b: *) => boolean} [options.equals] - Custom equality * check. Set as an instance override of the default `===` method. + * @param {boolean} [options.force=false] - If true, subscribers are notified + * on every write, even when `equals` reports no change. The cached value + * still respects `equals` (no-op writes don't update it). */ - constructor (value, { equals } = {}) { + constructor (value, { equals, force = false } = {}) { super(); this.#value = value; + this.#force = force; if (equals) { this.equals = equals; } @@ -69,6 +74,9 @@ export class Signal extends EventTarget { set value (v) { if (this.equals(v, this.#value)) { + if (this.#force) { + this.#notify(this.#value); + } return; } @@ -187,17 +195,8 @@ export class Computed extends Signal { })); } - // Check if value actually changed, and if so, notify subscribers. - // Temporarily suspend tracking so the internal read doesn't - // register this Computed as a dependency of an outer Computed. - let prev2 = tracking; - tracking = null; - let old = super.value; - tracking = prev2; - - if (!this.equals(value, old)) { - // Use Signal.prototype.value setter directly (bypasses no-op Computed setter) - Object.getOwnPropertyDescriptor(Signal.prototype, "value").set.call(this, value); - } + // Delegate the equals/force decision to the Signal setter. + // Use Signal.prototype.value setter directly (bypasses no-op Computed setter). + Object.getOwnPropertyDescriptor(Signal.prototype, "value").set.call(this, value); } } diff --git a/test/Prop.js b/test/Prop.js index 0b70ee48..8f8da226 100644 --- a/test/Prop.js +++ b/test/Prop.js @@ -1,7 +1,7 @@ import { default as Prop } from "../src/plugins/props/util/Prop.js"; import { default as Props } from "../src/plugins/props/util/Props.js"; import { resolveValue } from "../src/util/resolve-value.js"; -import FakeElement from "./util/FakeElement.js"; +import FakeElement, { apply, flush } from "./util/FakeElement.js"; export default { name: "Prop class", @@ -285,8 +285,16 @@ export default { tests: [ { name: "propchange events", - async run ({ props, actions, only }) { - let { events } = await FakeElement.from(props, actions); + async run ({ props, actions = [], only }) { + let el = new (FakeElement.with(props))(); + + let events = []; + el.addEventListener("propchange", e => + events.push({ name: e.name, source: e.detail?.source })); + + el.mount(); + await apply(el, actions); + let stream = only ? events.filter(e => only.includes(e.name)) : events; return stream.map(({ name, source }) => `${name}/${source}`); }, @@ -298,12 +306,12 @@ export default { src: { type: String, default: "initial" }, mirror: { type: String, defaultProp: "src" }, }, - actions: [el => (el.src = "changed")], + actions: el => (el.src = "changed"), }, expect: [ "src/default", "mirror/default", - "src/default", + "src/property", "mirror/default", ], }, @@ -311,7 +319,6 @@ export default { name: "default() fires on declared name only", arg: { props: { bar: { default: () => 42 } }, - only: ["bar", "defaultBar"], }, expect: ["bar/default"], }, @@ -342,7 +349,7 @@ export default { }, }, }, - actions: [el => (el.base = 2)], + actions: el => (el.base = 2), only: ["derived"], }, expect: ["derived/default", "derived/default"], @@ -364,7 +371,7 @@ export default { }, }, }, - actions: [el => (el.plain = 5)], + actions: el => (el.plain = 5), }, expect: [ // Mount @@ -372,8 +379,8 @@ export default { "computed/get", "fnDefault/default", - // Update — Computed-backed: source stays construction-time - "plain/default", + // Update + "plain/property", "computed/get", "fnDefault/default", ], @@ -382,16 +389,333 @@ export default { name: "Assigning current default-resolved value is a no-op", arg: { props: { v: { type: Number, default: 0 } }, - actions: [el => (el.v = 0)], + actions: el => (el.v = 0), }, expect: ["v/default"], }, + { + name: "Sync writes to a plain Signal coalesce to a single event with the settled value", + arg: { + props: { v: { type: Number } }, + actions: [ + el => { + el.v = 1; + el.v = 3; + el.v = 42; + }, + ], + only: ["v"], + }, + // No mount event: plain Signal initial undefined ≡ oldValue. + expect: ["v/property"], + }, + { + name: "Round-trip back to the initial value on a plain Signal fires no event", + async run ({ props, actions }) { + let el = new (FakeElement.with(props))(); + let count = 0; + el.addEventListener("propchange", () => count++); + el.mount(); + await apply(el, actions); + return count; + }, + arg: { + props: { v: { type: Number } }, + actions: [ + el => { + el.v = 5; + el.v = undefined; + }, + ], + }, + expect: 0, + }, + ], + }, + { + name: "Handler observes post-cascade values for sibling Computeds", + async run () { + let el = new (FakeElement.with({ + a: { type: Number, default: 0 }, + b: { + type: Number, + get () { + return this.a + 1; + }, + }, + c: { + type: Number, + get () { + return this.a * 2; + }, + }, + }))(); + el.mount(); + + // Listener attached after mount, so mount events stay out. + let snapshots = []; + el.addEventListener("propchange", e => + snapshots.push({ name: e.name, b: el.b, c: el.c })); + + await apply(el, el => (el.a = 5)); + return snapshots; + }, + expect: [ + { name: "a", b: 6, c: 10 }, + { name: "b", b: 6, c: 10 }, + { name: "c", b: 6, c: 10 }, + ], + }, + { + name: "Attribute reflection — settled state", + async run ({ props, actions, attr }) { + let el = new (FakeElement.with(props))(); + el.mount(); + await apply(el, actions); + + return [el.v, el.getAttribute(attr)]; + }, + tests: [ + { + name: "External setAttribute wins over pending reflection", + arg: { + props: { v: { type: Number, reflect: true, default: 0 } }, + actions: [ + el => { + el.v = 5; + el.setAttribute("v", "99"); + }, + ], + attr: "v", + }, + expect: [99, "99"], + }, + { + name: "Property write after setAttribute drains the latest property value", + arg: { + props: { v: { type: Number, reflect: true, default: 0 } }, + actions: [ + el => { + el.setAttribute("v", "99"); + el.v = 5; + }, + ], + attr: "v", + }, + expect: [5, "5"], + }, + ], + }, + { + name: "updated() bulk semantics", + async run ({ props, actions = [] }) { + let Class = FakeElement.with(props); + + let calls = []; + Class.prototype.updated = function (event) { + calls.push( + [...event.changedProps].map(([name, old]) => ({ + name, + old, + value: this[name], + })), + ); + }; + + let el = new Class(); + el.mount(); + await apply(el, actions); + return calls; + }, + tests: [ + { + name: "Multi-prop cascade fires one call with all settled changes", + arg: { + props: { + a: { type: Number, default: 0 }, + b: { + type: Number, + get () { + return this.a + 1; + }, + }, + c: { + type: Number, + get () { + return this.a * 2; + }, + }, + }, + actions: el => (el.a = 5), + }, + // First call: mount settle. Second: el.a = 5 cascade. + expect: [ + [ + { name: "a", old: undefined, value: 0 }, + { name: "b", old: undefined, value: 1 }, + { name: "c", old: undefined, value: 0 }, + ], + [ + { name: "a", old: 0, value: 5 }, + { name: "b", old: 1, value: 6 }, + { name: "c", old: 0, value: 10 }, + ], + ], + }, + { + name: "Coalesced sync writes on a Computed-backed prop produce one call with first→last delta", + arg: { + props: { v: { type: Number, default: 0 } }, + actions: [ + el => { + el.v = 1; + el.v = 3; + el.v = 42; + }, + ], + }, + expect: [ + [{ name: "v", old: undefined, value: 0 }], + [{ name: "v", old: 0, value: 42 }], + ], + }, + { + name: "Coalesced sync writes on a plain Signal preserve the first-write old value", + // Plain-Signal path: no default, no convert, no get. + // oldValue flows through Prop.set, not the Computed subscriber. + arg: { + props: { v: { type: Number } }, + actions: [ + el => { + el.v = 1; + el.v = 50; + el.v = 99; + }, + ], + }, + // No mount call: plain Signal initial undefined ≡ post-mount value. + expect: [ + [{ name: "v", old: undefined, value: 99 }], + ], + }, + ], + }, + { + name: "Shortcut event names dispatch alongside propchange from the same payload", + async run () { + let Class = FakeElement.with({ v: { type: Number, default: 0 } }); + // Simulate a propchange shortcut (propchange.js#first_constructor_static). + Class.props.get("v").eventNames = ["change"]; + + let calls = []; + Class.prototype.updated = function (event) { + calls.push( + [...event.changedProps].map(([name, old]) => ({ + name, + old, + })), + ); + }; + + let el = new Class(); + let events = []; + for (let name of ["propchange", "change"]) { + el.addEventListener(name, e => + events.push(`${name}/${e.detail.value}`)); + } + + el.mount(); + await apply(el, el => (el.v = 42)); + + return { events, calls }; + }, + expect: { + // Mount fires both names; update fires both names. Same payload each time. + events: [ + "propchange/0", + "change/0", + "propchange/42", + "change/42", + ], + // updated(): one entry per prop per drain, regardless of how + // many shortcut event names fired. + calls: [ + [{ name: "v", old: undefined }], + [{ name: "v", old: 0 }], + ], + }, + }, + { + name: "propChangedCallback auto-wires as a per-prop propchange listener", + async run () { + let Class = FakeElement.with({ + a: { type: Number, default: 0 }, + b: { type: Number, default: 0 }, + }); + let calls = []; + Class.prototype.propChangedCallback = function (event) { + calls.push(`${event.name}/${event.detail.value}`); + }; + + let el = new Class(); + el.mount(); + await apply(el, el => { + el.a = 5; + el.b = 7; + }); + return calls; + }, + // One call per dispatched propchange event: mount × 2, update × 2. + expect: ["a/0", "b/0", "a/5", "b/7"], + }, + { + name: "propschange fires after every propchange in the same drain", + async run () { + let Class = FakeElement.with({ + a: { type: Number, default: 0 }, + b: { type: Number, default: 0 }, + }); + let order = []; + let el = new Class(); + el.addEventListener("propchange", e => order.push(`propchange/${e.name}`)); + el.addEventListener("propschange", () => order.push("propschange")); + + el.mount(); + await apply(el, el => { + el.a = 5; + el.b = 7; + }); + return order; + }, + expect: [ + // Mount drain. + "propchange/a", + "propchange/b", + "propschange", + // Update drain. + "propchange/a", + "propchange/b", + "propschange", ], }, + { + name: "propschange event exposes changedProps Map directly", + async run () { + let el = new (FakeElement.with({ v: { type: Number, default: 0 } }))(); + let seen; + el.addEventListener("propschange", e => (seen = e.changedProps)); + el.mount(); + await flush(); + return seen instanceof Map && seen.has("v"); + }, + expect: true, + }, { name: "Final value", - async run ({ props, actions, read }) { - let { el } = await FakeElement.from(props, actions); + async run ({ props, actions = [], read }) { + let el = new (FakeElement.with(props))(); + el.mount(); + await apply(el, actions); return el[read]; }, tests: [ @@ -470,7 +794,7 @@ export default { }, }, }, - actions: [el => (el.v = null)], + actions: el => (el.v = null), read: "v", }, expect: null, @@ -503,7 +827,7 @@ export default { }, }, }, - actions: [el => (el.base = 5)], + actions: el => (el.base = 5), read: "derived", }, expect: 10, @@ -556,17 +880,31 @@ export default { equals: (a, b) => Math.abs(a - b) < 0.1, }, }, - actions: [el => (el.base = 42.05)], + actions: el => (el.base = 42.05), read: "derived", }, expect: 42, }, + { + name: "removeAttribute restores default", + arg: { + props: { v: { type: Number, reflect: true, default: 5 } }, + actions: [ + el => el.setAttribute("v", "6"), + el => el.removeAttribute("v"), + ], + read: "v", + }, + expect: 5, + }, ], }, { name: "Attribute reflection", - async run ({ props, actions, attr }) { - let { el } = await FakeElement.from(props, actions); + async run ({ props, actions = [], attr }) { + let el = new (FakeElement.with(props))(); + el.mount(); + await apply(el, actions); return el.getAttribute(attr); }, tests: [ @@ -574,21 +912,21 @@ export default { name: "Prop write reflects to attribute", arg: { props: { v: { type: Number, reflect: true } }, - actions: [el => (el.v = 42)], + actions: el => (el.v = 42), attr: "v", }, expect: "42", }, { - name: "Default + reflect reflects on mount (plain)", + name: "Default does NOT reflect on mount (plain)", arg: { props: { plain: { type: Number, default: 7, reflect: true } }, attr: "plain", }, - expect: "7", + expect: null, }, { - name: "Default + convert + reflect reflects on mount", + name: "Default does NOT reflect on mount (with convert)", arg: { props: { val: { @@ -602,7 +940,40 @@ export default { }, attr: "val", }, - expect: "10", + expect: null, + }, + { + name: "removeAttribute clears the reflected attribute", + arg: { + props: { v: { type: Number, reflect: true, default: 5 } }, + actions: [ + el => el.setAttribute("v", "6"), + el => el.removeAttribute("v"), + ], + attr: "v", + }, + expect: null, + }, + { + name: "Explicit write equal to default still reflects", + arg: { + props: { v: { type: Number, reflect: true, default: 5 } }, + actions: [el => (el.v = 5)], + attr: "v", + }, + expect: "5", + }, + { + name: "Restoring the default clears the previously-reflected attribute", + arg: { + props: { v: { type: Number, reflect: true, default: 5 } }, + actions: [ + el => (el.v = 6), + el => (el.v = undefined), + ], + attr: "v", + }, + expect: null, }, ], }, diff --git a/test/Props.js b/test/Props.js index 3bb06713..00c62513 100644 --- a/test/Props.js +++ b/test/Props.js @@ -1,5 +1,5 @@ import { default as Props } from "../src/plugins/props/util/Props.js"; -import FakeElement from "./util/FakeElement.js"; +import FakeElement, { flush } from "./util/FakeElement.js"; export default { name: "Props class", @@ -94,8 +94,7 @@ export default { { name: "Pre-set attributes parse on mount", async run ({ props, value }) { - let { Class } = await FakeElement.from(props); - let el = new Class(); + let el = new (FakeElement.with(props))(); el.setAttribute("prop", value); el.mount(); return el.prop; @@ -110,13 +109,13 @@ export default { expect: 55, }, { - name: "Post-mount setAttribute updates the property (issue #98)", - skip: true, + name: "Post-mount setAttribute updates the property", async run () { - let { el } = await FakeElement.from( - { prop: { type: Number, reflect: true } }, - [el => (el.prop = 42), el => el.setAttribute("prop", "100")], - ); + let el = new (FakeElement.with({ + prop: { type: Number, reflect: true }, + }))(); + el.mount(); + el.setAttribute("prop", "100"); return el.prop; }, expect: 100, @@ -127,26 +126,53 @@ export default { name: "Disconnect / reconnect lifecycle", tests: [ { - name: "Queued propchange events drain on reconnect (case C from PR #91)", - skip: true, + name: "Queued propchange events drain on reconnect", async run () { - let { el } = await FakeElement.from({ + let el = new (FakeElement.with({ v: { type: Number, default: 0 }, - }); + }))(); + el.mount(); let names = []; el.addEventListener("propchange", e => names.push(e.name)); el.isConnected = false; el.v = 5; // Flush the Computed microtask while disconnected so - // firePropChangeEvent enters its queueing branch. - await Promise.resolve(); + // the queueing branch runs. + await flush(); el.isConnected = true; return names; }, expect: ["v"], }, + { + name: "Reconnect drains queued events without replaying mount events", + async run () { + let el = new (FakeElement.with({ + v: { type: Number, default: 0 }, + }))(); + let events = []; + el.addEventListener("propchange", e => events.push(e)); + el.mount(); + // Mount event for the default-resolved 0. + let mountCount = events.length; + + el.isConnected = false; + el.v = 5; + await flush(); + let afterDisconnect = events.length; + + el.isConnected = true; + await flush(); + let afterReconnect = events.length; + + return { mountCount, afterDisconnect, afterReconnect }; + }, + // Disconnected drain bails — payload waits in queue. Reconnect + // dispatches the post-disconnect payload, NOT the already-fired mount. + expect: { mountCount: 1, afterDisconnect: 1, afterReconnect: 2 }, + }, ], }, ], diff --git a/test/util/FakeElement.js b/test/util/FakeElement.js index a3e3f34e..fc9f40ba 100644 --- a/test/util/FakeElement.js +++ b/test/util/FakeElement.js @@ -1,12 +1,53 @@ import Props from "../../src/plugins/props/util/Props.js"; +/** Yield N microtasks so any queued work runs. */ +export async function flush (ticks = 1) { + for (let i = 0; i < ticks; i++) { + await Promise.resolve(); + } +} + +/** Apply each action to el in sequence, flushing between each. Single function is fine too. */ +export async function apply (el, actions, ticks = 1) { + actions = Array.isArray(actions) ? actions : [actions]; + + for (let action of actions) { + action(el); + await flush(ticks); + } +} + /** Minimal in-memory element for tests of Prop / Props. */ export default class FakeElement extends EventTarget { - isConnected = false; ignoredAttributes = new Set(); props = {}; #attrs = new Map(); + constructor () { + super(); + // Stand in for the plugin's `constructor` hook auto-wiring. + if (this.propChangedCallback) { + this.addEventListener("propchange", this.propChangedCallback); + } + if (this.updated) { + this.addEventListener("propschange", this.updated); + } + } + + #connected = false; + get isConnected () { + return this.#connected; + } + + set isConnected (value) { + let was = this.#connected; + this.#connected = value; + // Stand in for the real `connectedCallback`. + if (!was && value) { + this.constructor.props?.connected(this); + } + } + hasAttribute (name) { return this.#attrs.has(name); } @@ -16,11 +57,25 @@ export default class FakeElement extends EventTarget { } setAttribute (name, value) { - this.#attrs.set(name, String(value)); + let old = this.#attrs.get(name) ?? null; + let str = String(value); + if (old === str) { + return; + } + + this.#attrs.set(name, str); + // Stand in for the real `attributeChangedCallback`. + this.constructor.props?.attributeChanged(this, name, old); } removeAttribute (name) { + let old = this.#attrs.get(name) ?? null; + if (old === null) { + return; + } + this.#attrs.delete(name); + this.constructor.props?.attributeChanged(this, name, old); } mount () { @@ -40,6 +95,13 @@ export default class FakeElement extends EventTarget { return events; } + /** Build a FakeElement subclass with the given props spec. */ + static with (props) { + let Class = class extends FakeElement {}; + Class.props = new Props(Class, props); + return Class; + } + /** * Build a FakeElement subclass for `props`, instantiate it, attach the * propchange recorder, mount it, run each action thunk with a microtask @@ -48,16 +110,14 @@ export default class FakeElement extends EventTarget { * instances when a test needs pre-mount setup. */ static async from (props, actions = []) { - let Class = class extends FakeElement {}; - Class.props = new Props(Class, props); - + let Class = FakeElement.with(props); let el = new Class(); let events = el.recordEvents(); el.mount(); for (let action of actions) { action(el); - await Promise.resolve(); + await flush(1); } return { Class, el, events };