From 0fa983aea8c70b7d5275f4feac8ffba271f5bd63 Mon Sep 17 00:00:00 2001
From: Alexander van Elsas <58037137+avanelsas@users.noreply.github.com>
Date: Thu, 4 Jun 2026 12:45:30 +0200
Subject: [PATCH] fix(x-select): make .value reflect the current selection
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

x-select's `.value` returned the value ATTRIBUTE, which a user selection never updates
(only the inner <select> + the dispatched select-change event changed). So consumers that
read `el.value` — x-form's collect-values, the demo's status filter — saw a stale value:
the filter looked dead, and forms silently submitted the pre-selection value.

Every other BareDOM form control makes `.value`/`.checked` reflect the current value (the
contract x-form relies on); x-select was the lone exception. Override its `value` getter to
read the live inner <select>, with one disambiguation: a non-empty value set BEFORE its
<option> exists (async-loaded options) is surfaced as pending via the attribute until the
option arrives. This preserves both existing tests (value-attr-not-auto-set-on-change,
string-property-value) and adds two regression tests (selection reflection; set-before-options).

clj-kondo clean; full karma 5190.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 src/baredom/components/x_select/x_select.cljs | 44 ++++++++++++++++++-
 .../components/x_select/x_select_test.cljs    | 41 +++++++++++++++++
 2 files changed, 84 insertions(+), 1 deletion(-)

diff --git a/src/baredom/components/x_select/x_select.cljs b/src/baredom/components/x_select/x_select.cljs
index 92eee2c6..ee4ac893 100644
--- a/src/baredom/components/x_select/x_select.cljs
+++ b/src/baredom/components/x_select/x_select.cljs
@@ -398,8 +398,50 @@
 ;; Element class and registration
 ;; ---------------------------------------------------------------------------
 
+(defn- option-value-present?
+  "True when the inner <select> currently has an <option> with this exact value."
+  [^js select-el value]
+  (let [opts (.-options select-el)
+        n    (.-length opts)]
+    (loop [i 0]
+      (cond
+        (>= i n)                          false
+        (= value (.-value (aget opts i))) true
+        :else                             (recur (inc i))))))
+
+;; `value` must reflect the CURRENT selection — that is the native <select> contract and
+;; what every other BareDOM form control honours (x-form's collect-values and direct
+;; `el.value` readers depend on it). The reflecting accessor install-properties! would give
+;; reads the `value` ATTRIBUTE, which a user selection never updates, so override `value`
+;; with a getter that reads the live inner <select>. The one wrinkle: a non-empty value set
+;; BEFORE its <option> exists (async-loaded options) can't be held by the inner <select> —
+;; surface that as pending via the attribute until the option arrives and apply-model!
+;; selects it. (`value-attr-not-auto-set-on-change-test` still holds: a user change updates
+;; only the inner <select>, never the attribute.)
+(defn- define-value-prop! [^js proto]
+  (.defineProperty
+   js/Object proto "value"
+   #js {:configurable true
+        :enumerable   true
+        :get (fn xs-get-value []
+               (this-as ^js this
+                 (if-let [refs (du/getv this k-refs)]
+                   (let [^js select-el (gobj/get refs part-select)
+                         attr          (or (du/get-attr this model/attr-value) "")]
+                     (if (and (not= attr "") (not (option-value-present? select-el attr)))
+                       attr                       ; pending: option not loaded yet
+                       (.-value select-el)))      ; live selection (incl. a deliberate empty pick)
+                   (or (du/get-attr this model/attr-value) ""))))
+        :set (fn xs-set-value [v]
+               ;; Set the attribute (the controlled/pending value); apply-model! selects the
+               ;; matching <option> on the inner select. Same as the reflecting setter it replaces.
+               (this-as ^js this
+                 (du/set-attr! this model/attr-value
+                               (if (and (some? v) (not= v js/undefined)) (str v) ""))))}))
+
 (defn- install-property-accessors! [^js proto]
-  (du/install-properties! proto model/property-api))
+  (du/install-properties! proto model/property-api)   ; disabled / required / name + (reflecting) value
+  (define-value-prop! proto))                          ; override `value`: getter reads the live inner <select>
 
 (defn init! []
   (component/register! model/tag-name
diff --git a/test/baredom/components/x_select/x_select_test.cljs b/test/baredom/components/x_select/x_select_test.cljs
index 9c0cfa3d..54817fbe 100644
--- a/test/baredom/components/x_select/x_select_test.cljs
+++ b/test/baredom/components/x_select/x_select_test.cljs
@@ -324,6 +324,47 @@
           0))
        0))))
 
+;; Conforming behaviour: `el.value` reflects the user's CURRENT selection (native <select>
+;; contract) — what x-form's collect-values and the demo's status filter read. Previously
+;; el.value returned the stale `value` attribute and never tracked a user selection.
+(deftest value-reflects-user-selection-test
+  (async done
+    (let [el    (append! (make-el))
+          opt-a (.createElement js/document "option")
+          opt-b (.createElement js/document "option")]
+      (.setAttribute opt-a "value" "alpha") (set! (.-textContent opt-a) "Alpha")
+      (.setAttribute opt-b "value" "beta")  (set! (.-textContent opt-b) "Beta")
+      (.appendChild el opt-a)
+      (.appendChild el opt-b)
+      (js/setTimeout
+       (fn []
+         (let [^js sel (shadow-part el "[part=select]")]
+           (set! (.-value sel) "beta")
+           (.dispatchEvent sel (js/Event. "change" #js {:bubbles true})))
+         (js/setTimeout
+          (fn []
+            (is (= "beta" (.-value el))
+                "el.value reflects the user's selection (not the stale attribute)")
+            (done))
+          0))
+       0))))
+
+;; A value set BEFORE its <option> exists (async-loaded options) reads back as pending,
+;; then resolves to the live selection once the option arrives.
+(deftest value-set-before-options-is-pending-then-resolves-test
+  (async done
+    (let [el (append! (make-el))]
+      (set! (.-value el) "beta")
+      (is (= "beta" (.-value el)) "pending value reads back before its option exists")
+      (let [opt-b (.createElement js/document "option")]
+        (.setAttribute opt-b "value" "beta") (set! (.-textContent opt-b) "Beta")
+        (.appendChild el opt-b))
+      (js/setTimeout
+       (fn []
+         (is (= "beta" (.-value el)) "resolves to the now-present option's value")
+         (done))
+       0))))
+
 (deftest select-change-event-bubbles-and-composed-test
   (async done
     (let [el    (append! (make-el))