fix(web): assign static select values through DOM property (#3167)

Author: nickshiro

.changeset/fix-static-select-value.md

@@ -0,0 +1,6 @@
+---
+"@solidjs/babel-plugin": patch
+"@solidjs/compiler": patch
+---
+
+Assign static `<select value>` values through the live DOM property so the matching option is selected consistently with reactive values.

packages/babel-plugin/src/dom/element.ts

@@ -1221,7 +1221,10 @@ function transformAttributes(
         }
 
         // properties
-        if (staticValue && ChildProperties.has(key)) {
+        if (
+          staticValue &&
+          (ChildProperties.has(key) || (tagName === "select" && key === "value"))
+        ) {
           results.exprs.push(
             t.expressionStatement(
               setAttr(attribute, elem, key, staticValue as babelTypes.Expression, { tagName })

packages/babel-plugin/test/staticSelectValue.spec.js

@@ -0,0 +1,28 @@
+const babel = require("@babel/core");
+const plugin = require("../index");
+
+function compile(code) {
+  return babel.transformSync(code, {
+    babelrc: false,
+    configFile: false,
+    filename: "input.jsx",
+    plugins: [[plugin, { moduleName: "r-dom", generate: "dom" }]]
+  }).code;
+}
+
+test("assigns static select values through the DOM property", () => {
+  const code = compile(`
+    const stringAttribute = <select value="2"><option value="2">Two</option></select>;
+    const stringExpression = <select value={"2"}><option value="2">Two</option></select>;
+    const numberExpression = <select value={2}><option value="2">Two</option></select>;
+    const multipleString = <select multiple value="2"><option value="2">Two</option></select>;
+    const multipleArray = <select multiple value={["1", "2"]}><option value="2">Two</option></select>;
+    const dynamicChildren = <select value="2">{options()}</select>;
+  `);
+
+  expect(code).not.toMatch(/_\$template\(`<select value=/);
+  expect(code.match(/queueMicrotask/g)).toHaveLength(6);
+  expect(code.match(/\.value = (?:"2"|2)/g)).toHaveLength(10);
+  expect(code.match(/\.value = \["1", "2"\]/g)).toHaveLength(2);
+  expect(code.indexOf("queueMicrotask")).toBeLessThan(code.indexOf("_$insert("));
+});

packages/compiler/__tests__/transform.test.js

@@ -182,6 +182,29 @@ describe("@solidjs/compiler transform", () => {
     expect(result.code).not.toContain("</input>");
   });
 
+  it("assigns static select values through the DOM property", () => {
+    const result = transform(
+      `
+        const stringAttribute = <select value="2"><option value="2">Two</option></select>;
+        const stringExpression = <select value={"2"}><option value="2">Two</option></select>;
+        const numberExpression = <select value={2}><option value="2">Two</option></select>;
+        const multipleString = <select multiple value="2"><option value="2">Two</option></select>;
+        const multipleArray = <select multiple value={["1", "2"]}><option value="2">Two</option></select>;
+        const dynamicChildren = <select value="2">{options()}</select>;
+      `,
+      {
+        filename: "static-select-value.jsx",
+        moduleName: "r-dom"
+      }
+    );
+
+    expect(result.code).not.toMatch(/_\$template\(`<select value=/);
+    expect(result.code.match(/queueMicrotask/g)).toHaveLength(6);
+    expect(result.code.match(/\.value = (?:"2"|2)/g)).toHaveLength(10);
+    expect(result.code.match(/\.value = \["1", "2"\]/g)).toHaveLength(2);
+    expect(result.code.indexOf("queueMicrotask")).toBeLessThan(result.code.indexOf("_$insert("));
+  });
+
   it("compiles the supported simpleElements fixture subset from Babel sources", () => {
     const source = readFixture("simpleElements");
     const subset = source.slice(

packages/compiler/src/dom/attrs.rs

@@ -165,7 +165,7 @@ impl<'a> AstDomTransform<'a, '_> {
             if has_children && plan.key == "textContent" {
                 continue;
             }
-            match self.classify_plan(&plan) {
+            match self.classify_plan(&plan, tag_name) {
                 PlanDisposition::Skip => {}
                 PlanDisposition::Inline(value) => match value {
                     None => append_bare_attribute(template, &plan.key, self.omit_attribute_spacing),
@@ -200,7 +200,7 @@ impl<'a> AstDomTransform<'a, '_> {
 
     /// Pure classification of one planned attribute, mirroring Babel's
     /// static-vs-expression branch in the attribute loop.
-    fn classify_plan(&self, plan: &AttrPlan<'a>) -> PlanDisposition {
+    fn classify_plan(&self, plan: &AttrPlan<'a>, tag_name: &str) -> PlanDisposition {
         if self.hydratable && plan.key == "$ServerOnly" {
             return PlanDisposition::Skip;
         }
@@ -211,6 +211,7 @@ impl<'a> AstDomTransform<'a, '_> {
             return PlanDisposition::Skip;
         }
         let reserved = plan.style_property || plan.class_property || plan.key.starts_with("prop:");
+        let select_value = tag_name == "select" && plan.key == "value";
         match &plan.value {
             PlanValue::None => {
                 if reserved {
@@ -222,7 +223,7 @@ impl<'a> AstDomTransform<'a, '_> {
                 }
             }
             PlanValue::Literal(value) => {
-                if reserved || child_properties(&plan.key) {
+                if reserved || child_properties(&plan.key) || select_value {
                     PlanDisposition::Runtime
                 } else {
                     PlanDisposition::Inline(Some(value.clone()))
@@ -243,14 +244,14 @@ impl<'a> AstDomTransform<'a, '_> {
                         }
                     }
                     Expression::StringLiteral(literal) => {
-                        if child_properties(&plan.key) {
+                        if child_properties(&plan.key) || select_value {
                             PlanDisposition::Runtime
                         } else {
                             PlanDisposition::Inline(Some(literal.value.to_string()))
                         }
                     }
                     Expression::NumericLiteral(literal) => {
-                        if child_properties(&plan.key) {
+                        if child_properties(&plan.key) || select_value {
                             PlanDisposition::Runtime
                         } else {
                             PlanDisposition::Inline(Some(format_number(literal.value)))
@@ -586,7 +587,7 @@ impl<'a> AstDomTransform<'a, '_> {
         } = self.plan_attributes(attributes, tag_name)?;
         let mut pending: std::vec::Vec<(String, Option<String>)> = std::vec::Vec::new();
         for plan in &plans {
-            match self.classify_plan(plan) {
+            match self.classify_plan(plan, tag_name) {
                 PlanDisposition::Skip => {}
                 PlanDisposition::Inline(value) => pending.push((plan.key.clone(), value)),
                 PlanDisposition::Runtime => return Ok(None),

packages/web/test/element.spec.tsx

@@ -37,6 +37,41 @@ describe("Basic element attributes", () => {
     expect(d.innerHTML).toBe("<p>Hi</p>");
   });
 
+  test("static select value selects the matching option (#3167)", () => {
+    const select = (
+      <select value="2">
+        <option value="">None</option>
+        <option value="1">One</option>
+        <option value="2">Two</option>
+      </select>
+    ) as unknown as HTMLSelectElement;
+
+    expect(select.value).toBe("2");
+    expect(select.selectedIndex).toBe(2);
+  });
+
+  test("static select value is reapplied after dynamic children (#3167)", async () => {
+    const options = () => [<option value="1">One</option>, <option value="2">Two</option>];
+    const select = (<select value="2">{options()}</select>) as unknown as HTMLSelectElement;
+
+    await Promise.resolve();
+
+    expect(select.value).toBe("2");
+    expect(select.selectedIndex).toBe(1);
+  });
+
+  test("static select value selects one option in a multiple select (#3167)", () => {
+    const select = (
+      <select multiple value="2">
+        <option value="1">One</option>
+        <option value="2">Two</option>
+      </select>
+    ) as unknown as HTMLSelectElement;
+
+    expect(select.multiple).toBe(true);
+    expect(Array.from(select.options, option => option.selected)).toEqual([false, true]);
+  });
+
   test("class", () => {
     const classes = { first: true, second: false, "third fourth": true },
       d = (<div class={classes} />) as unknown as HTMLDivElement;