fix(Option): infer textValue from mixed children and keep value stable

packages/codemods/src/migrations.generated.ts

@@ -6,6 +6,16 @@ import type { MigrationEntry } from "./catalog/types.js";
 
 /** Every migration, newest first. Bodies live in `src/migrations`. */
 export const migrations: Omit<MigrationEntry, "body">[] = [
+  {
+    id: "option-value-inferred-from-mixed-children",
+    since: "1.1.12",
+    title: "Option: value is inferred from mixed children",
+    kind: "migration",
+    action: "none",
+    remotePackage: true,
+    apply:
+      "No code change required for the option itself. An `Option` whose children are text plus an element — text and a `Badge`, text and an icon — now carries that text as its key, where it previously fell back to react-aria's generated `react-aria-N`. Check anywhere such a key was read back: a stored or server-side selection, a `defaultValue`/`value` matched against it, or a test asserting on it. Pass an explicit `value` to pin the key to something other than the display text.",
+  },
   {
     id: "use-design-tokens-build-metadata-removed",
     since: "1.0.16",

packages/codemods/src/migrations/option-value-inferred-from-mixed-children/entry.md

@@ -0,0 +1,60 @@
+---
+since: 1.1.12
+title: "Option: value is inferred from mixed children"
+kind: migration
+action: none
+remotePackage: true
+apply:
+  "No code change required for the option itself. An `Option` whose children are
+  text plus an element — text and a `Badge`, text and an icon — now carries that
+  text as its key, where it previously fell back to react-aria's generated
+  `react-aria-N`. Check anywhere such a key was read back: a stored or
+  server-side selection, a `defaultValue`/`value` matched against it, or a test
+  asserting on it. Pass an explicit `value` to pin the key to something other
+  than the display text."
+---
+
+An `Option` used to infer its `textValue` — and, through it, its `value` — only
+when its children were exactly one text node. Text next to any element left both
+undefined, so react-aria assigned the option a key off a render-order counter:
+
+```tsx
+<Select>
+  <Option>
+    Millennium Falcon <Badge>Latest</Badge>
+  </Option>
+  <Option>X-Wing</Option>
+</Select>
+```
+
+| Option                        | key before     | key now             |
+| ----------------------------- | -------------- | ------------------- |
+| `Millennium Falcon` + `Badge` | `react-aria-1` | `Millennium Falcon` |
+| `X-Wing`                      | `X-Wing`       | `X-Wing`            |
+
+The key is what the field reports as its selected value and what `defaultValue`
+has to match, so the old key made the field submit a meaningless string, could
+not be preselected, and shifted when unrelated markup around it changed.
+
+Only text that is a child itself contributes — text inside an element child does
+not, so the option above is `"Millennium Falcon"` and not
+`"Millennium Falcon Latest"`.
+
+An `Option` with no text among its children at all still has no key to infer,
+and now says so on the console instead of failing silently:
+
+```tsx
+// logs: An <Option> has no 'value' and none could be inferred from its children…
+<Option>
+  <Text>Millennium Falcon</Text>
+</Option>
+```
+
+Give those options a `value`:
+
+```diff
+- <Option>
++ <Option value="millennium-falcon">
+    <Text>Millennium Falcon</Text>
+  </Option>
+```

packages/codemods/src/tests/guide.test.ts

@@ -24,7 +24,7 @@ describe("the generated migration guide", () => {
     const order = [...committed.matchAll(/<a id="([a-z0-9-]+)"><\/a>/g)].map(
       (match) => match[1],
     );
-    expect(order[0]).toBe("use-design-tokens-build-metadata-removed");
+    expect(order[0]).toBe("option-value-inferred-from-mixed-children");
     expect(order.at(-1)).toBe("renamed-css-export");
   });
 });

packages/codemods/src/tests/remoteScope.test.ts

@@ -53,6 +53,7 @@ const targets: Record<string, string[]> = {
   "form-resets-after-modal-close": ["Form"],
   "overlay-controller-add-on-close-return-type": ["OverlayController"],
   "cartesian-chart-empty-view": ["CartesianChart"],
+  "option-value-inferred-from-mixed-children": ["Option"],
   "action-prop-to-on-action": ["Action", "ActionProps"],
   "button-props-interfaces": [
     "ResetButtonProps",

packages/components/MIGRATION.md

@@ -28,6 +28,68 @@ The CLI's own output does detect it and prints the right form.
 
 ---
 
+<a id="option-value-inferred-from-mixed-children"></a>
+
+## Option: value is inferred from mixed children
+
+**Since `1.1.12`** · migration · no code change needed · also applies to
+`@mittwald/flow-remote-react-components`
+
+An `Option` used to infer its `textValue` — and, through it, its `value` — only
+when its children were exactly one text node. Text next to any element left both
+undefined, so react-aria assigned the option a key off a render-order counter:
+
+```tsx
+<Select>
+  <Option>
+    Millennium Falcon <Badge>Latest</Badge>
+  </Option>
+  <Option>X-Wing</Option>
+</Select>
+```
+
+| Option                        | key before     | key now             |
+| ----------------------------- | -------------- | ------------------- |
+| `Millennium Falcon` + `Badge` | `react-aria-1` | `Millennium Falcon` |
+| `X-Wing`                      | `X-Wing`       | `X-Wing`            |
+
+The key is what the field reports as its selected value and what `defaultValue`
+has to match, so the old key made the field submit a meaningless string, could
+not be preselected, and shifted when unrelated markup around it changed.
+
+Only text that is a child itself contributes — text inside an element child does
+not, so the option above is `"Millennium Falcon"` and not
+`"Millennium Falcon Latest"`.
+
+An `Option` with no text among its children at all still has no key to infer,
+and now says so on the console instead of failing silently:
+
+```tsx
+// logs: An <Option> has no 'value' and none could be inferred from its children…
+<Option>
+  <Text>Millennium Falcon</Text>
+</Option>
+```
+
+Give those options a `value`:
+
+```diff
+- <Option>
++ <Option value="millennium-falcon">
+    <Text>Millennium Falcon</Text>
+  </Option>
+```
+
+**Apply:** No code change required for the option itself. An `Option` whose
+children are text plus an element — text and a `Badge`, text and an icon — now
+carries that text as its key, where it previously fell back to react-aria's
+generated `react-aria-N`. Check anywhere such a key was read back: a stored or
+server-side selection, a `defaultValue`/`value` matched against it, or a test
+asserting on it. Pass an explicit `value` to pin the key to something other than
+the display text.
+
+---
+
 <a id="use-design-tokens-build-metadata-removed"></a>
 
 ## useDesignTokens(): no longer returns style-dictionary build metadata

packages/components/src/components/Option/Option.browser.test.tsx

@@ -0,0 +1,123 @@
+import { Badge } from "@/components/Badge";
+import { Label } from "@/components/Label";
+import { Option } from "@/components/Option";
+import { Select } from "@/components/Select";
+import { Text } from "@/components/Text";
+import type { ReactNode } from "react";
+import { expect, test, vi } from "vitest";
+import { page } from "vitest/browser";
+import { render } from "vitest-browser-react";
+
+/*
+ * The shape of the Select Default story: one option whose children are text
+ * plus a Badge, next to plain-text options. Its key used to fall back to
+ * react-aria's render-order counter (`react-aria-1`), because neither
+ * `textValue` nor `value` could be inferred from more than one child (#3028).
+ */
+const renderSelect = (props?: {
+  defaultValue?: string;
+  onChange?: (value: unknown) => void;
+}) =>
+  render(
+    <Select defaultValue={props?.defaultValue} onChange={props?.onChange}>
+      <Label>Starship</Label>
+      <Option>
+        Millennium Falcon <Badge>Latest</Badge>
+      </Option>
+      <Option>X-Wing</Option>
+      <Option>TIE Fighter</Option>
+    </Select>,
+  );
+
+const toggle = page.getByRole("button", { name: "Starship" });
+
+const openOptions = async () => {
+  await toggle.click();
+  await expect.element(page.getByRole("listbox")).toBeVisible();
+};
+
+const optionKeys = () =>
+  Array.from(document.querySelectorAll("[role='option']")).map((option) =>
+    option.getAttribute("data-key"),
+  );
+
+test("every option gets its text as its key, mixed children included", async () => {
+  renderSelect();
+  await openOptions();
+
+  expect(optionKeys()).toEqual(["Millennium Falcon", "X-Wing", "TIE Fighter"]);
+});
+
+test("selecting an option with mixed children reports its text as the value", async () => {
+  const onChange = vi.fn();
+  renderSelect({ onChange });
+  await openOptions();
+
+  await page.getByRole("option", { name: /Millennium Falcon/ }).click();
+
+  expect(onChange).toHaveBeenCalledWith("Millennium Falcon");
+});
+
+test("an option with mixed children can be targeted by defaultValue", async () => {
+  renderSelect({ defaultValue: "Millennium Falcon" });
+
+  await expect.element(toggle).toHaveTextContent("Millennium Falcon");
+});
+
+test("the inferred textValue leaves out the text of element children", async () => {
+  renderSelect({ defaultValue: "Millennium Falcon" });
+
+  await expect.element(toggle).not.toHaveTextContent("Latest");
+});
+
+/*
+ * With no text among its children there is nothing to infer, so `value` stays
+ * undefined and react-aria generates the key. That used to be silent — the only
+ * console output was react-aria's warning about `textValue`, which says nothing
+ * about the key becoming the option's form value.
+ */
+test("an option whose children carry no text of their own says so", async () => {
+  const error = vi.spyOn(console, "error").mockImplementation(() => undefined);
+
+  render(
+    <Select>
+      <Label>Starship</Label>
+      <Option>
+        <Text>Millennium Falcon</Text>
+      </Option>
+    </Select>,
+  );
+
+  await expect
+    .poll(() => error.mock.calls.flat().join("\n"))
+    .toMatch(/Option.*no 'value'/s);
+
+  error.mockRestore();
+});
+
+test("an explicit value silences the warning and wins over the children", async () => {
+  const error = vi.spyOn(console, "error").mockImplementation(() => undefined);
+  const onChange = vi.fn();
+
+  const options: ReactNode = (
+    <Option value="mf">
+      Millennium Falcon <Badge>Latest</Badge>
+    </Option>
+  );
+
+  render(
+    <Select onChange={onChange}>
+      <Label>Starship</Label>
+      {options}
+    </Select>,
+  );
+
+  await openOptions();
+  expect(optionKeys()).toEqual(["mf"]);
+
+  await page.getByRole("option", { name: /Millennium Falcon/ }).click();
+  expect(onChange).toHaveBeenCalledWith("mf");
+  expect(error.mock.calls.flat().join("\n")).not.toMatch(/Option.*no 'value'/s);
+
+  error.mockRestore();
+});

packages/components/src/components/Option/Option.tsx

@@ -1,18 +1,28 @@
 import { type PropsWithChildren } from "react";
-import { Children } from "react";
+import { Children, useEffect, useRef } from "react";
 import * as Aria from "react-aria-components";
 import clsx from "clsx";
 import styles from "./Option.module.scss";
 import type { FlowComponentProps } from "@/lib/componentFactory/flowComponent";
 import { flowComponent } from "@/lib/componentFactory/flowComponent";
-import { extractTextFromFirstChild } from "@/lib/react/remote";
+import { extractTextFromChildren } from "@/lib/react/remote";
 import { IconCheck } from "@/components/Icon/components/icons";
 
 export interface OptionProps
   extends
     Omit<Aria.ListBoxItemProps, "children" | "value" | "id">,
     PropsWithChildren,
     FlowComponentProps {
+  /**
+   * The value this option contributes to the field's selection, and the key
+   * `defaultValue` / `value` of the surrounding field target.
+   *
+   * Defaults to `textValue`, which itself defaults to the text among the
+   * option's children — element children such as a `Badge` do not contribute,
+   * so `<Option>Millennium Falcon <Badge>Latest</Badge></Option>` is
+   * `"Millennium Falcon"`. Pass it explicitly when the value must not follow
+   * the display text, or when the children carry no text at all.
+   */
   value?: string | number;
 }
 
@@ -21,12 +31,14 @@ export const Option = flowComponent("Option", (props) => {
   const {
     className,
     children,
-    textValue = extractTextFromFirstChild(children),
+    textValue = extractTextFromChildren(children),
     value = textValue,
     ref,
     ...rest
   } = props;
 
+  useWarnMissingValue(value === undefined);
+
   const rootClassName = clsx(styles.option, className);
   const hasChildren = Children.count(children) >= 1;
 
@@ -46,4 +58,31 @@ export const Option = flowComponent("Option", (props) => {
   );
 });
 
+/*
+ * Without an `id`, react-aria gives the item a key off a render-order counter
+ * (`react-aria-1`). That key is what the field reports as its selected value
+ * and what `defaultValue` has to match, so an option without a `value` silently
+ * submits a meaningless string and cannot be preselected — and the key shifts
+ * when unrelated markup around it changes. react-aria warns about the missing
+ * `textValue` in this situation but says nothing about the key, which is the
+ * expensive half (#3028).
+ *
+ * Warn in an effect rather than during render, so a double render in StrictMode
+ * does not log twice; the ref keeps it to once per option.
+ */
+const useWarnMissingValue = (isMissing: boolean): void => {
+  const hasWarned = useRef(false);
+
+  useEffect(() => {
+    if (!isMissing || hasWarned.current) {
+      return;
+    }
+
+    hasWarned.current = true;
+    console.error(
+      "An <Option> has no 'value' and none could be inferred from its children, so it falls back to a generated, render-order-dependent key. Pass a 'value' to give the option a stable key.",
+    );
+  }, [isMissing]);
+};
+
 export default Option;