Add batched propschange event#129
Conversation
✅ Deploy Preview for nude-element ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
propschange event + ElementProp source state
propschange event + ElementProp source statepropschange event
d96044d to
7fbee69
Compare
Fires once per microtask after a burst of `propchange` events settles,
carrying the net first→last delta as a `Map<name, oldValue>` (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.
Replaces per-field `@type` declarations and the inline object type on `ElementProps#firePropChangeEvent` with shared JSDoc typedefs.
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
The burst queue used to be a Map<propName, PropChangeEvent> 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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
02ce28b to
d31e0d9
Compare
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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) <noreply@anthropic.com>
I wonder if we should mention it in the docs just to draw the user's attention to the fact that we don't use |
DmitrySharabin
left a comment
There was a problem hiding this comment.
We have a couple of issues to fix. See comments.
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) <noreply@anthropic.com>
Co-Authored-By: Dmitry Sharabin <dmitrysharabin@gmail.com>
|
I'd also suggest adding these two regression tests: {
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: "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"]]],
} |
DmitrySharabin
left a comment
There was a problem hiding this comment.
I'd suggest adding these tests (I believe they might help catch regressions). Other than that, LGTM! Excited to see what comes next.
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) <noreply@anthropic.com>
|
While testing the changes with the color elements, I found a regression. The default value of the color-inline component is calculated based on the component value: {
type: String,
default () {
return this.textContent.trim();
},
}The updated architecture where {
name: "default() that reads a non-prop (e.g. this.textContent) fires on mount",
description:
"A function default fires a mount event when it reads another prop, but not when it reads a non-prop receiver member:" +
" default () { return this.base } // this.base is a prop → fires ✓" +
" default () { return this.content } // this.content isn't → fires ✗ (BUG)" +
"inferDependencies scrapes the name either way, promoting default() to a synthetic computed prop that depends on `content`. Since no such prop exists, nothing cascades to it and a computed prop doesn't self-fire on mount, so the chain stays silent. The value still derives correctly on read, but the mount propchange never fires.",
arg: {
mixin (Class) {
Object.defineProperty(Class.prototype, "content", {
get () {
return "hi";
},
});
},
props: {
value: {
default () {
return this.content;
},
},
},
},
expect: [
["defaultValue", "hi"],
["value", "hi"],
],
} |
|
You mean it doesn't run at all? Or it doesn't re-run? |
Co-authored-by: Dmitry Sharabin <dmitrysharabin@gmail.com>
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) <noreply@anthropic.com>
|
Pushed a fix for the regression, feel free to add any tests you feel are useful |
|
@DmitrySharabin pushed Root cause was as you intuited — Fix: every prop's constructor now eagerly derives and fires its own mount event. Single-firing is preserved because a subsequent cascade-driven |
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) <noreply@anthropic.com>
There was a problem hiding this comment.
We should fix this regression first: #129 (comment).
I also added these tests, so please pull.
| 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, | ||
| })); | ||
| } |
There was a problem hiding this comment.
Should mount-drain propchanges be re-dispatched as semantic alias events?
This re-broadcasts every propchange — including the mount-time default fire — as its alias (spacechange, valuechange, …). So a consumer listening for the alias gets a "change" for a value that never changed (the initial default).
Real case that bit me: a parent listens to a nested child's spacechange and writes the child's value back into its own reflected prop:
// child: events = { spacechange: { propchange: "selectedSpace" } } // selectedSpace is a computed (get) prop
spacePicker.addEventListener("spacechange", () => {
this.value = spacePicker.value + "." + channelSelect.value; // ← also runs at mount
});At mount the child's computed selectedSpace fires with its default (a98rgb), this listener re-fires it as spacechange, and the parent's authored value="oklab.a" gets overwritten (and reflected back to the attribute, destroying the markup). Because it races the parent's own value-sync, it's intermittent.
The catch is that for a computed aliased prop you can't filter it by source: both the mount fire and a genuine post-mount cascade arrive with source: undefined (ElementProp.update() clears it) — only oldValue === undefined separates them. And PropChangeEvent.applyTo keys off source, so it would also refuse to mirror real computed-prop changes.
So: should aliases fire during the mount drain for initial/default values at all — and if they do, what's the intended signal to tell "initial" from "changed"? (Pre-#106 these carried source: "initial", which at least gave consumers a marker to filter on.)
There was a problem hiding this comment.
I think the whole idea of applyTo() is fundamentally broken TBH. And for most callsites in color-elements replacing it would be a one line fix, I think. So I wonder if we should just get rid of it…
There was a problem hiding this comment.
With the adoption of updated({change}), none of the color elements use it. So, yes, we can get rid of it.
|
Submitted a PR with possible fixes if it helps: #132. |
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Summary
Adds a new
propschangeevent: a coalesced, microtask-deferred batch event that fires once after a burst ofpropchangeevents settles, carrying the net first→last delta per prop asMap<name, oldValue>. Round-trips (value returns to its burst-start) drop out. Lives in a newpropschangesub-plugin underprops/and is auto-wired to subclasses that define anupdated(event)method, mirroring Lit'supdated(changedProperties).Mount fires one
propschangewith every prop inchanged(oldValue: undefined), so initial values arrive as a single settled snapshot.Compared to
mainNew:
propschangeevent + plugin (src/plugins/props/propschange.js)Listens for
propchangeon each instance, maintains per-prop burst tracking (firstOldValue+ latestvalue+ spec), and fires onePropsChangeEvent("propschange", { changed })per microtask. Auto-wiresupdated(event). Theprops/plugin folder now mirrors theevents/pattern:base.jsis the core props plugin,index.jsis a meta-plugin bundlingbase + propschange, so the defaultNudeElementships with both. Anyone who wants props core in isolation can importprops/base.js.New:
ElementProp.sourceTracks where the current value came from:
"default" | "property" | "attribute" | "get" | "convert". Drives the "user-owned" stickiness ElementProps uses to keep a"get"recompute from silently stripping ownership that a prior property/attribute write established.Breaking:
PropChangeEventshapePropChangeEventnow extendsEvent(notCustomEvent) and exposes its fields directly —e.value,e.oldValue,e.source,e.attributeName, etc. — instead of nesting undere.detail. Each dispatch gets a fresh event object; fields don't mutate past the handler.Downstream impact (incl. color-elements):
e.detail.*→e.*onpropchangelisteners.Behavior: paused/resume sequential coalescing
paused/resume themselves already existed on main, but onpaused = falsethe queue used to flush every queued event verbatim. Now it walks the queue in write order and in-place coalesces consecutive same-prop entries (preserving the run's burst-startoldValue), dispatching one event per run. A sequence likeA A B Aproduces three dispatches, not two — non-adjacent same-prop runs stay separate, so the consumer hears a faithful narrative.Behavior:
on*=handlers in markupOn*-prop handlers now attach via the on* prop's spec
changedcallback, which runs synchronously insideElementProp.changed— beforepropChangedfires. Handlers bind during the prop's own init pass, in time for the mount propchanges that follow. Markup<x onfoo="…">receives the mount-time event with its realsource(e.g."default") rather than a synthesized"initial"re-fire.Tests
npx htest test/index.js— 185/189 pass (4 pre-existing skips)e.detail→e.*flat-shape change and sanity-check thepropschange/source-state behavior