Skip to content

Commit 386ebaa

Browse files
fix(webui-framework): @attr property reflection (#304)
* Fix @attr property reflection The bug was that `@attr` looked reactive from the component template, but it was not fully the same reactive primitive as `@observable`. Property writes updated the backing field and could refresh template bindings, but they did not reflect back to the host DOM attribute. `@attr` also was not registered in the observable-name registry, so targeted updates, SSR state seeding, and `setState()` treated it differently from `@observable`. ```ts host.label = "bob"; ``` Before this fix, that assignment left the host markup stale: ```html <test-attr label="Status"></test-attr> ``` This broke attribute selectors, external DOM reads, serialization, and parent code that expects `@attr` to be observable state plus DOM attribute synchronization. The fix makes `@attr` reuse the observable registration path and adds guarded property-to-attribute reflection. The reflection guard prevents a DOM write from re-entering `attributeChangedCallback` and converting non-string backing values into strings. ```ts host.count = 5; // stays number-backed while reflecting count="5" ``` It also syncs pre-connect `@attr` property values after mount so client-created elements preserve values assigned before `appendChild()`. Tests now cover default and custom attr names, boolean attr reflection, DOM-to-property updates, `setState()`, type-preserving reflection, and pre-connect assignment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix inherited decorator metadata The review found that subclassed WebUI elements did not inherit decorator metadata from their base classes. That meant inherited `@attr` properties could appear in `observedAttributes` while `attributeChangedCallback`, `setState()`, SSR state seeding, targeted updates, and mount-time attribute sync only saw metadata registered directly on the subclass. ```ts class BaseCard extends WebUIElement { @attr baseLabel = ""; } class ProductCard extends BaseCard { @attr label = ""; } ``` Before this fix, `base-label` changes on `<product-card>` were not routed to `baseLabel`, and inherited observable names were missing from the subclass registry. The fix resolves observable and attr metadata through the constructor chain and copies inherited maps when a subclass registers its own decorators. That keeps lookup cost bounded to subclass setup or rare inheritance fallback, while direct classes still use the same WeakMap-backed hot path. Tests now cover inherited `@observable` names and inherited `@attr` metadata for DOM-to-property and property-to-attribute synchronization. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * base class --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 7854469 commit 386ebaa

6 files changed

Lines changed: 440 additions & 65 deletions

File tree

packages/webui-framework/src/decorators.test.ts

Lines changed: 135 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,46 @@
44
import { strict as assert } from 'node:assert';
55
import { describe, test } from 'node:test';
66

7-
import { toKebabCase } from './decorators.js';
7+
import {
8+
attr,
9+
getObservableNames,
10+
isAttributeProperty,
11+
observable,
12+
toKebabCase,
13+
} from './decorators.js';
14+
15+
class FakeElement {
16+
$ready = true;
17+
isConnected = false;
18+
private readonly attrs = new Map<string, string>();
19+
20+
getAttribute(name: string): string | null {
21+
return this.attrs.get(name) ?? null;
22+
}
23+
24+
hasAttribute(name: string): boolean {
25+
return this.attrs.has(name);
26+
}
27+
28+
setAttribute(name: string, value: string): void {
29+
const oldValue = this.getAttribute(name);
30+
const newValue = String(value);
31+
this.attrs.set(name, newValue);
32+
this.attributeChangedCallback?.(name, oldValue, newValue);
33+
}
34+
35+
removeAttribute(name: string): void {
36+
const oldValue = this.getAttribute(name);
37+
this.attrs.delete(name);
38+
this.attributeChangedCallback?.(name, oldValue, null);
39+
}
40+
41+
attributeChangedCallback?(
42+
name: string,
43+
oldValue: string | null,
44+
newValue: string | null,
45+
): void;
46+
}
847

948
describe('toKebabCase', () => {
1049
test('converts multi-word ARIA properties to correct attribute names', () => {
@@ -79,3 +118,98 @@ describe('toKebabCase', () => {
79118
assert.equal(toKebabCase('dataTitle'), 'data-title');
80119
});
81120
});
121+
122+
describe('observable decorators', () => {
123+
test('@observable registers reactive property names', () => {
124+
class TestElement {}
125+
observable(TestElement.prototype, 'count');
126+
127+
assert.equal(getObservableNames(TestElement).has('count'), true);
128+
});
129+
130+
test('@observable names inherit through subclasses', () => {
131+
class BaseElement {}
132+
observable(BaseElement.prototype, 'baseCount');
133+
class TestElement extends BaseElement {}
134+
observable(TestElement.prototype, 'count');
135+
136+
const names = getObservableNames(TestElement);
137+
assert.equal(names.has('baseCount'), true);
138+
assert.equal(names.has('count'), true);
139+
});
140+
141+
test('@attr registers reactive property names', () => {
142+
class TestElement {}
143+
attr(TestElement.prototype, 'label');
144+
145+
assert.equal(getObservableNames(TestElement).has('label'), true);
146+
});
147+
148+
test('@attr reflects property values without stringifying backing state', () => {
149+
class TestElement extends FakeElement {}
150+
attr(TestElement.prototype, 'count');
151+
152+
const element = new TestElement() as TestElement & { count: number };
153+
element.count = 5;
154+
155+
assert.equal(element.getAttribute('count'), '5');
156+
assert.equal(element.count, 5);
157+
});
158+
159+
test('@attr reacts to string attribute changes', () => {
160+
class TestElement extends FakeElement {}
161+
attr({ attribute: 'display-value' })(TestElement.prototype, 'displayValue');
162+
163+
const element = new TestElement() as TestElement & { displayValue: string | null };
164+
element.setAttribute('display-value', 'Ready');
165+
166+
assert.equal(element.displayValue, 'Ready');
167+
});
168+
169+
test('@attr boolean mode reflects presence and removal', () => {
170+
class TestElement extends FakeElement {}
171+
attr({ mode: 'boolean', attribute: 'is-active' })(TestElement.prototype, 'isActive');
172+
173+
const element = new TestElement() as TestElement & { isActive: boolean };
174+
element.isActive = true;
175+
assert.equal(element.hasAttribute('is-active'), true);
176+
177+
element.isActive = false;
178+
assert.equal(element.hasAttribute('is-active'), false);
179+
180+
element.setAttribute('is-active', '');
181+
assert.equal(element.isActive, true);
182+
183+
element.removeAttribute('is-active');
184+
assert.equal(element.isActive, false);
185+
});
186+
187+
test('@attr metadata inherits through subclasses', () => {
188+
class BaseElement extends FakeElement {}
189+
attr(BaseElement.prototype, 'baseLabel');
190+
class TestElement extends BaseElement {}
191+
attr(TestElement.prototype, 'label');
192+
193+
const names = getObservableNames(TestElement);
194+
assert.equal(names.has('baseLabel'), true);
195+
assert.equal(names.has('label'), true);
196+
assert.equal(isAttributeProperty(TestElement, 'baseLabel'), true);
197+
assert.equal(isAttributeProperty(TestElement, 'label'), true);
198+
assert.equal(isAttributeProperty(TestElement, 'missing'), false);
199+
200+
const element = new TestElement() as TestElement & {
201+
baseLabel: string;
202+
label: string;
203+
};
204+
element.setAttribute('base-label', 'Base');
205+
element.setAttribute('label', 'Child');
206+
207+
assert.equal(element.baseLabel, 'Base');
208+
assert.equal(element.label, 'Child');
209+
210+
element.baseLabel = 'Updated Base';
211+
element.label = 'Updated Child';
212+
assert.equal(element.getAttribute('base-label'), 'Updated Base');
213+
assert.equal(element.getAttribute('label'), 'Updated Child');
214+
});
215+
});

0 commit comments

Comments
 (0)