From fa139e7fa90e45d00592cf7b428007576f4675d8 Mon Sep 17 00:00:00 2001 From: Dmitry Sharabin Date: Fri, 15 May 2026 13:21:50 +0200 Subject: [PATCH 1/3] Populate detail on `first_connected` shortcut re-fires (#106) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The re-fire previously dispatched a PropChangeEvent with no detail, so a listener that read e.detail.value would crash on the catch-up dispatch while succeeding on the regular dispatch. Populate a minimal detail with source: "initial" (marker for synthetic catch-up) and value (the field whose absence caused the crash). Found while integration-testing nude-element against color-elements ( exercises this path). --- src/plugins/events/propchange.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/plugins/events/propchange.js b/src/plugins/events/propchange.js index 36dbf509..ee37f60a 100644 --- a/src/plugins/events/propchange.js +++ b/src/plugins/events/propchange.js @@ -6,6 +6,7 @@ import { symbols } from "xtensible"; import base, { events } from "./base.js"; import { props } from "../props/index.js"; +import PropChangeEvent from "../props/util/PropChangeEvent.js"; const { propchange } = symbols.new; @@ -45,12 +46,13 @@ const hooks = { let propName = this.constructor[propchange][eventName]; let value = this[propName]; - if (value !== undefined) { - this.constructor[props].firePropChangeEvent(this, eventName, { - name: propName, - prop: this.constructor[props].get(propName), - }); + if (value === undefined) { + continue; } + + let prop = this.constructor[props].get(propName); + let detail = { source: "initial", value }; + this.dispatchEvent(new PropChangeEvent(eventName, { name: propName, prop, detail })); } }, }; From b509235b98e1315a10819fc8356c265984ce9037 Mon Sep 17 00:00:00 2001 From: Dmitry Sharabin Date: Fri, 15 May 2026 13:21:58 +0200 Subject: [PATCH 2/3] Preserve explicit `null` in reflected attribute writes `change.attributeValue ?? change.element.getAttribute(...)` treated an explicit null (callers signaling "clear this attribute") the same as "no override given, look it up on the element." When a reflected prop was set to null or undefined, the fallback re-read the existing attribute, so the clear never happened. Switch to `!== undefined` so `null` propagates to the subsequent `attributeValue === null` branch and removes the attribute. Fixes the "Clearing a reflected prop removes the attribute" regression tests for both `value: undefined` and `value: null`. --- src/plugins/props/util/Prop.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/plugins/props/util/Prop.js b/src/plugins/props/util/Prop.js index cdd0d7c2..ca3785be 100644 --- a/src/plugins/props/util/Prop.js +++ b/src/plugins/props/util/Prop.js @@ -254,7 +254,9 @@ let Self = class Prop { if (element.setAttribute) { let attributeName = change.attributeName ?? this.toAttribute; let attributeValue = - change.attributeValue ?? change.element.getAttribute(attributeName); + change.attributeValue !== undefined + ? change.attributeValue + : change.element.getAttribute(attributeName); if (attributeValue === null) { element.removeAttribute(attributeName); From e82d409f8ddf043a4e229aa78a18d32ac4c799da Mon Sep 17 00:00:00 2001 From: Dmitry Sharabin Date: Fri, 15 May 2026 13:48:33 +0200 Subject: [PATCH 3/3] Polish propchange event detail names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rename the `propchange` event detail fields to match the post-signals naming on `gitbutler/workspace` so consumers see a single, clearly named value pair: - `event.detail.parsedValue` → `event.detail.value` (now the parsed / converted value; the raw user-provided input is no longer in the detail) - `event.detail.oldInternalValue` → `event.detail.oldValue` - `event.detail.attributeName` is only set when an attribute is actually involved (was previously always present, sometimes undefined) Internal supporting changes: - `Prop#set` destructured param `oldValue` → `oldAttributeValue` so the new local `oldValue` (previous parsed value) doesn't shadow it - `Props#attributeChanged` caller updated to pass `oldAttributeValue: oldValue` - `src/plugins/events/onprops.js` consumer reads `change.value` / `change.oldValue` - One `test/plugins/props/propchange.js` consumer updated Baseline preserved: 91 pass / 7 fail / 4 skip — the 7 pre-existing regression pins are unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/plugins/events/onprops.js | 8 ++++---- src/plugins/props/util/Prop.js | 17 +++++++---------- src/plugins/props/util/Props.js | 6 +++++- test/plugins/props/propchange.js | 3 +-- 4 files changed, 17 insertions(+), 17 deletions(-) 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/props/util/Prop.js b/src/plugins/props/util/Prop.js index ca3785be..a2a17565 100644 --- a/src/plugins/props/util/Prop.js +++ b/src/plugins/props/util/Prop.js @@ -183,10 +183,9 @@ let Self = class Prop { return value; } - set (element, value, { source, name, oldValue } = {}) { - let oldInternalValue = element.props[this.name]; + set (element, value, { source, name, oldAttributeValue } = {}) { + let oldValue = element.props[this.name]; - let attributeName = name; let parsedValue; try { @@ -205,7 +204,7 @@ let Self = class Prop { parsedValue = this.spec.convert.call(element, parsedValue); } - if (this.equals(parsedValue, oldInternalValue)) { + if (this.equals(parsedValue, oldValue)) { return; } @@ -214,10 +213,8 @@ let Self = class Prop { let change = { element, source, - value, - parsedValue, - oldInternalValue, - attributeName: name, + value: parsedValue, + oldValue, }; if (source === "property") { @@ -240,9 +237,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 62f7d884..b58db1c0 100644 --- a/src/plugins/props/util/Props.js +++ b/src/plugins/props/util/Props.js @@ -133,7 +133,11 @@ 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, + }); } } diff --git a/test/plugins/props/propchange.js b/test/plugins/props/propchange.js index 810ace34..bdcb7da7 100644 --- a/test/plugins/props/propchange.js +++ b/test/plugins/props/propchange.js @@ -145,8 +145,7 @@ export default { run () { let { element } = this.data; let log = []; - element.addEventListener("propchange", e => - log.push([e.name, e.detail.parsedValue])); + element.addEventListener("propchange", e => log.push([e.name, e.detail.value])); element.v = "foo"; element.v = "bar";