Race: external read of a Computed-backed prop before `initializeFor` pollutes the attribute and clobbers programmatic writes

[DmitrySharabin]

Summary

A Computed-backed prop's lazy first compute fires its subscriber, which reflects the resolved value to the attribute. If this first compute is triggered by an external read (e.g. a parent component reading a child's prop during the parent's own mount) before the child's Props.initializeFor runs, then:

1. The resolved default is reflected to the child's attribute.
2. A programmatic write to the child between that read and the child's initializeFor writes to rawSignal but the synthesized attribute remains.
3. The child's Props.initializeFor Pass 1 reads the attribute (now holding the synthesized default), treats it as user input, and overwrites the programmatic write.

Net result: the programmatic write is silently lost.

Repro (drop-in test)

Add this to the "Runtime behavior" group in test/Prop.js, alongside the "Attribute reflection" sub-group:

{
    name: "Initialization timing",
    tests: [
        {
            name: "Programmatic write between an external read and initializeFor wins over default reflection",
            async run () {
                let Class = FakeElement.with({
                    v: { type: Number, default: 7, reflect: true },
                });
                let el = new Class();

                void el.v;   // External read: forces lazy first compute
                el.v = 42;   // Programmatic write before mount

                el.mount();  // Child's initializeFor runs after the parent's logic
                await flush(2);

                return [el.v, el.getAttribute("v")];
            },
            expect: [42, "42"],
        },
    ],
},

Currently returns [7, "7"] on main and on props-batched-drain — the synthesized default attribute is re-imported by Pass 1, overwriting rawSignal := 42 back to 7.

Real-world surface

Found while integration-testing nude-element against color-elements. The <color-picker> element contains a <space-picker> in its shadow DOM. During color-picker's own mount its spaceId.changed propchange handler reads this._el.space_picker.value (color-picker.js:149-152), forcing space-picker's Computed to first-compute and reflect the default "a98rgb". Color-picker then pushes sp.value = "lab". Space-picker's Props.initializeFor runs next, sees the synthesized value="a98rgb" attribute, and overwrites rawSignal back to "a98rgb" — clobbering the parent's intent.

Detailed trace

1. Color-picker reads sp.value → lazy getSignal creates a Computed with its subscriber attached at construction time. signal.value triggers first compute → falls through to default "a98rgb".
2. Subscriber #onComputedChange fires synchronously: setAttribute("value", "a98rgb") on the inner space-picker (wrapped in ignoredAttributes.add/delete — but that only protects the synchronous attributeChangedCallback window).
3. Color-picker writes sp.value = "lab" → prop.set writes rawSignal := "lab". The Computed marks itself dirty; its subscriber hasn't fired yet (microtask flush).
4. Space-picker's Props.initializeFor runs (next microtask). Pass 1 iterates observedAttributes and finds value="a98rgb" (synthesized in step 2) → prop.set(el, "a98rgb", { source: "attribute" }) → rawSignal := "a98rgb". Programmatic write lost.

The ignoredAttributes mechanism doesn't catch this because Pass 1 reads attributes via getAttribute() directly (not via attributeChangedCallback), and by then the ignored set is empty.

Why this is hard to fix cleanly

There's a contract conflict to resolve before picking an implementation. test/Prop.js codifies the following behaviors:

• "Default + reflect reflects on mount (plain)" — { default: 7, reflect: true } → el.getAttribute("plain") === "7" after mount.
• "Default + convert + reflect reflects on mount" — same, with convert applied.
• "Reflection writes go to reflect.to alias, not the prop name" — expects [["data-v", "7"], ["data-v", "42"]] (default reflects on mount, then update).

These tests assert that defaults DO materialize as attributes on mount. That's the explicit, opt-in contract. The race surfaces because the same machinery materializes defaults at the wrong time as well.

Candidate fixes

Option A — change the contract ("defaults don't reflect")

• Implementation: gate reflection in #onComputedChange on whether rawSignal has a user-set value:

  let fromDefault = rawSignal.value === undefined;
  this.#onComputedChange(element, source, newValue, oldValue, fromDefault);

• Pros: smallest diff (~10 lines).
• Cons:
  • Breaks the 3 tests above and the contract they encode.
  • Leaves stale attributes when a user writes undefined to re-engage the default after a prior write (subscriber fires with rawSignal.value === undefined → skips reflection → previously-set attribute remains). The "matches native HTML" justification doesn't fully hold even within the new code.
  • Requires a README note documenting the breaking behavior change.

Option B1 — per-element initialization flag (minimal, contract-preserving)

• Props tracks #initialized = new WeakSet(). initializeFor(element) adds the element at the start (before Pass 1).
• #onComputedChange gates the reflection block on this.props.hasInitialized(element).
• Pros: ~6 lines net. Preserves all 3 contract tests. Correctly handles re-engaging default after a write (no stale attribute).
• Cons: Still has a "propchange blip" — early read fires propchange("a98rgb"), later write fires propchange("lab"). Two events for what's logically one transition. Not a regression vs. Option A but architecturally ugly.

Option B2 — subscribe-after-init (architectural, preferred)

The deepest root cause is that the Computed subscriber owns work (element.props update, reflection, propchange) that should belong to initializeFor. A first compute is initialization, not a value change.

• Implementation:
  • Remove signal.subscribe(...) from both Computed branches of getSignal.
  • In prop.initializeFor, after the existing logic: force-read signal.value, manually call #onComputedChange once (if element.props[name] is undefined and value is defined), then subscribe for subsequent changes.
  • Move the closured source string to a per-Prop instance field so prop.initializeFor can pass it to #onComputedChange.
  • Add #subscribed = new WeakSet() and #computedSource private fields on Prop.
• Pros:
  • Eliminates the race entirely — subscribers physically can't fire before init.
  • Eliminates the propchange blip (only one propchange fires in the bug scenario, with the settled value).
  • Preserves the default link in all cases (including write-undefined-to-re-engage).
  • Clean separation: initializeFor does init, the subscriber does reactivity.
• Cons: ~15-line refactor; touches the lazy-subscribe pattern. Worth its own review pass.

Option C — workaround in color-elements

• Restructure color-picker's spaceId.changed to not read sp.value synchronously during the parent's drainFor. E.g. defer to a microtask, or compare against a last-known-pushed value tracked in color-picker itself.
• Pros: zero nude-element changes.
• Cons: papers over a real nude-element bug; other consumers can hit the same race in different shapes; doesn't surface the architectural issue.

Concerns to track regardless of which fix lands

• propchange "blip": two events for one logical transition. Only Option B2 eliminates it.
• Default-link severance: should an external read before initializeFor sever the default link?
  • B2: no (rawSignal stays undefined throughout the early read).
  • B1: same as B2 (subscriber fires but doesn't reflect; rawSignal still undefined).
  • A: depends on timing of subsequent attribute import.
• Object.hasOwn instance-value path (Prop.js:201-209): pre-existing quirk where the resulting propchange source reports the closured "default"/"convert" rather than "property". All three options inherit this; not a regression.
• spec.get Computeds with reflect:true: this.reflect = spec.reflect ?? !this.spec.get — reflection defaults off for get-props. Any fix should preserve that default while allowing opt-in.
• Re-mount after disconnect: WeakSet entries (B1, B2) persist across disconnect/reconnect. That's almost certainly correct (the prop layer doesn't "uninitialize"), but worth noting.

Recommended next steps

1. Decide between A / B1 / B2 (preference: B2).
2. Land the failing test (drop-in code above) on the working branch.
3. Implement the chosen fix.
4. Add a prose note in src/plugins/props/README.md clarifying when defaults reflect (whatever the final rule is).
5. Consider documenting the Object.hasOwn source-reporting quirk separately.

Background

A drive-by patch implementing Option A was tested while debugging the color-elements integration. It fixed the symptom but broke the 3 contract tests above without test/doc updates, so it was rejected.

The race is a pre-existing bug from #91 (signals-props) — not introduced by #102 (props-batched-drain). Filed separately so #102 can ship focused on its actual scope.

[LeaVerou]

Defaults should not reflect. The fact that they do is an independent DX bug. Lit also had this issue (and perhaps still does) and it was very annoying. Attributes should reflect when they are programmatically written, not just for their default values. CSS needs to be able to distinguish between default and explicitly set value. So yeah, A.