Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/plugins/props/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -163,3 +163,5 @@ 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 (via `el.prop = undefined` or `removeAttribute`) clears any previously-reflected attribute.
77 changes: 57 additions & 20 deletions src/plugins/props/util/Prop.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,21 +104,9 @@ let Self = class Prop {
#onComputedChange (element, source, newValue, oldValue) {
element.props[this.name] = newValue;

// Reflect to attribute if this prop opts in
if (this.toAttribute) {
let attributeValue = this.stringify(newValue);
let oldAttributeValue = element.getAttribute(this.toAttribute);
if (oldAttributeValue !== attributeValue) {
element.ignoredAttributes.add(this.toAttribute);
if (attributeValue === null) {
element.removeAttribute(this.toAttribute);
}
else {
element.setAttribute(this.toAttribute, attributeValue);
}
element.ignoredAttributes.delete(this.toAttribute);
}
}
// Defaults don't synthesize attributes (would clobber pre-mount writes; see #105);
// passing `null` clears any stale attribute left from a prior user write.
this.#reflect(element, source === "default" ? null : newValue);

this.changed(element, {
element,
Expand All @@ -128,6 +116,30 @@ let Self = class Prop {
});
}

/**
* The single reflection site, called from both user writes (`prop.set`) and
* dep-driven recomputes (`#onComputedChange`). Mirrors pre-signals' shape
* where `set()` was the one place attribute reflection happened.
*
* @param {HTMLElement} element
* @param {*} value - Post-convert value to reflect; `null` removes the attribute.
*/
#reflect (element, value) {
if (!this.toAttribute) {
return;
}

let attributeName = this.toAttribute;
let attributeValue = this.stringify(value);
if (element.getAttribute(attributeName) === attributeValue) {
return;
}

element.ignoredAttributes.add(attributeName);
this.applyChange(element, { source: "attribute", attributeName, attributeValue });
element.ignoredAttributes.delete(attributeName);
}

/**
* Get or lazily create the Signal for this prop on a given element.
* - Computed props (spec.get): Computed that auto-tracks dependencies
Expand Down Expand Up @@ -157,7 +169,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) {
Expand All @@ -181,7 +193,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 {
Expand Down Expand Up @@ -271,15 +284,37 @@ let Self = class Prop {
return;
}

// 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, oldInternalValue)) {
return;
}

if (rawSignal) {
// Computed-backed: write to the raw signal. The Computed recomputes,
// and its subscriber (#onComputedChange) handles element.props,
// reflection, and events.
// and its subscriber (#onComputedChange) handles element.props and events.
rawSignal.value = parsedValue;

// Why reflect here too, when the subscriber already calls `#reflect`?
// - The Computed dedupes recomputes whose new value equals its
// cached value. `el.v = 5` when the default resolves to 5 produces
// no subscriber fire, so reflection from `#onComputedChange` is
// skipped (#105).
// - We can't read `signal.value` to get the post-convert form: that
// would force a sync recompute, fire subscribers immediately, and
// split multi-prop dep cascades across two propchange drains.
// So apply `spec.convert` manually and call `#reflect` directly.
if (source === "property" && parsedValue !== undefined) {
this.#reflect(
element,
this.spec.convert ? this.spec.convert.call(element, parsedValue) : parsedValue,
);
}
}
else {
// For plain props: update signal, element.props, reflect, and fire events
Expand Down Expand Up @@ -330,7 +365,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);
Expand Down
14 changes: 2 additions & 12 deletions src/signals.js
Original file line number Diff line number Diff line change
Expand Up @@ -187,17 +187,7 @@ 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);
}
// Bypass Computed's read-only setter; Signal#set handles the equals dedupe.
Object.getOwnPropertyDescriptor(Signal.prototype, "value").set.call(this, value);
}
}
59 changes: 52 additions & 7 deletions test/Prop.js
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ export default {
expect: [
"src/default",
"mirror/default",
"src/default",
"src/property",
"mirror/default",
],
},
Expand Down Expand Up @@ -372,8 +372,8 @@ export default {
"computed/get",
"fnDefault/default",

// Update — Computed-backed: source stays construction-time
"plain/default",
// Update
"plain/property",
"computed/get",
"fnDefault/default",
],
Expand Down Expand Up @@ -561,6 +561,18 @@ export default {
},
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,
},
],
},
{
Expand All @@ -580,15 +592,15 @@ export default {
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: {
Expand All @@ -602,7 +614,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,
},
],
},
Expand Down
4 changes: 4 additions & 0 deletions test/util/FakeElement.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,15 @@ export default class FakeElement extends EventTarget {
}

setAttribute (name, value) {
let oldValue = this.#attrs.get(name) ?? null;
this.#attrs.set(name, String(value));
this.constructor.props?.attributeChanged(this, name, oldValue);
}

removeAttribute (name) {
let oldValue = this.#attrs.get(name) ?? null;
this.#attrs.delete(name);
this.constructor.props?.attributeChanged(this, name, oldValue);
}

mount () {
Expand Down