diff --git a/.changeset/steady-select-items.md b/.changeset/steady-select-items.md new file mode 100644 index 000000000..e364d0c08 --- /dev/null +++ b/.changeset/steady-select-items.md @@ -0,0 +1,5 @@ +--- +"@cloudflare/kumo": patch +--- + +Prevent equivalent Select object-map items from causing redundant Base UI store updates. diff --git a/packages/kumo/src/components/select/select.test.tsx b/packages/kumo/src/components/select/select.test.tsx index 5932db31b..7ebb8e79f 100644 --- a/packages/kumo/src/components/select/select.test.tsx +++ b/packages/kumo/src/components/select/select.test.tsx @@ -1,9 +1,155 @@ import { act, fireEvent, render, screen } from "@testing-library/react"; import { describe, expect, it, vi } from "vite-plus/test"; -import { useState } from "react"; +import { Profiler, useState } from "react"; import { Select } from "./select"; describe("Select", () => { + describe("object-map items", () => { + it("does not notify Base UI item subscribers for equivalent maps", () => { + const onRender = vi.fn(); + const renderSelect = () => ( + + + + ); + const { rerender } = render(renderSelect()); + onRender.mockClear(); + + rerender(renderSelect()); + + expect(onRender).toHaveBeenCalledTimes(1); + }); + + it("propagates key, label, and selected value changes", () => { + const { rerender } = render( + , + ); + expect(trigger.textContent).toContain("Green Apple"); + + rerender( + , + ); + + await act(async () => { + fireEvent.click(screen.getByRole("combobox")); + }); + expect(screen.getByRole("option").getAttribute("aria-disabled")).not.toBe( + "true", + ); + + rerender( + , + ); + + await act(async () => { + fireEvent.click(screen.getByRole("combobox")); + }); + expect( + screen.getAllByRole("option").map((option) => option.textContent), + ).toEqual(["Apple", "Banana"]); + + rerender( + + + ); + const { rerender } = render(renderSelect()); + onRender.mockClear(); + + rerender(renderSelect()); + expect(onRender).toHaveBeenCalledTimes(1); + + await act(async () => { + fireEvent.click(screen.getByRole("combobox")); + }); + expect(screen.getByRole("option").getAttribute("aria-disabled")).not.toBe( + "true", + ); + }); + }); + describe("size", () => { it("applies size classes to the trigger", () => { const { container } = render( diff --git a/packages/kumo/src/components/select/select.tsx b/packages/kumo/src/components/select/select.tsx index 096aacc16..d383e7d6d 100644 --- a/packages/kumo/src/components/select/select.tsx +++ b/packages/kumo/src/components/select/select.tsx @@ -1,6 +1,6 @@ import { Select as SelectBase } from "@base-ui/react/select"; import { CaretUpDownIcon, CheckIcon } from "@phosphor-icons/react"; -import { forwardRef, useId } from "react"; +import { forwardRef, useEffect, useId, useLayoutEffect, useRef } from "react"; import type { ReactNode } from "react"; import { cn } from "../../utils/cn"; import { buttonVariants } from "../button"; @@ -22,6 +22,11 @@ export const KUMO_SELECT_DEFAULT_VARIANTS = { size: "base", } as const; +// useLayoutEffect warns when rendered with react-dom/server (React 18); +// fall back to useEffect on the server where neither runs anyway. +const useIsomorphicLayoutEffect = + typeof window !== "undefined" ? useLayoutEffect : useEffect; + /** * Select component styling metadata for Figma plugin code generation * Extracted from select.tsx implementation (source of truth) @@ -141,6 +146,12 @@ function isItemDescriptor( return "label" in candidate && candidate.label !== undefined; } +interface NormalizedSelectItem { + label: ReactNode; + value: T; + disabled?: boolean; +} + /** * Normalizes items to array format for Base UI. * Object maps are converted to array format so Base UI can properly @@ -150,15 +161,62 @@ function normalizeItems( items: | Record | ReadonlyArray<{ label: ReactNode; value: T }>, -): ReadonlyArray<{ label: ReactNode; value: T }> { +): ReadonlyArray> { if (Array.isArray(items)) { return items; } // Convert object map to array format - return Object.entries(items).map(([key, entry]) => ({ - value: key as T, - label: isItemDescriptor(entry) ? entry.label : entry, - })); + return Object.entries(items).map(([key, entry]) => { + const descriptor = isItemDescriptor(entry); + return { + value: key as T, + label: descriptor ? entry.label : entry, + disabled: descriptor ? entry.disabled : undefined, + }; + }); +} + +function useNormalizedItems( + items: + | Record + | ReadonlyArray<{ label: ReactNode; value: T }> + | undefined, +): ReadonlyArray> | undefined { + const objectMapCache = useRef< + ReadonlyArray> | undefined + >(undefined); + let normalizedItems: ReadonlyArray> | undefined; + + if (items === undefined) { + normalizedItems = undefined; + } else if (Array.isArray(items)) { + normalizedItems = items; + } else { + const nextItems = normalizeItems(items); + const cached = objectMapCache.current; + const itemsAreEqual = + cached?.length === nextItems.length && + cached.every((item, index) => { + const nextItem = nextItems[index]; + return ( + Object.is(item.value, nextItem.value) && + Object.is(item.label, nextItem.label) && + item.disabled === nextItem.disabled + ); + }); + + normalizedItems = itemsAreEqual ? cached : nextItems; + } + + const isObjectMap = items !== undefined && !Array.isArray(items); + + useIsomorphicLayoutEffect(() => { + if (isObjectMap) { + objectMapCache.current = normalizedItems; + } + }, [isObjectMap, normalizedItems]); + + return normalizedItems; } /** @@ -167,40 +225,21 @@ function normalizeItems( * Filters out null values (typically used for placeholders). */ function renderOptionsFromItems( - items: - | Record - | ReadonlyArray<{ label: ReactNode; value: T }>, + normalizedItems: ReadonlyArray>, + isObjectMap: boolean, ): ReactNode { - const normalizedItems = normalizeItems(items); - - // Build a lookup for disabled metadata from object-map items. - // Object-map keys are always strings (Record), so the lookup - // uses string keys. The array form ({ label, value }[]) does not support - // descriptors — consumers should use the children API for that case. - const disabledLookup = new Map(); - if (!Array.isArray(items)) { - for (const [key, entry] of Object.entries(items)) { - if (isItemDescriptor(entry)) { - disabledLookup.set(key, { disabled: entry.disabled }); - } - } - } - // Filter out null values and render options return normalizedItems .filter((item) => item.value !== null) .map((item, index) => { const key = typeof item.value === "string" ? item.value : `option-${index}`; - // When items is an object-map, value is always a string key from - // Object.entries. When items is an array, disabledLookup is empty. - const meta = - typeof item.value === "string" - ? disabledLookup.get(item.value) - : undefined; - return ( - ); @@ -424,15 +463,16 @@ export function Select({ const triggerAriaLabel = ariaLabel ?? (!triggerLabelledBy ? fallbackLabel : undefined); - // Normalize items to array format for Base UI compatibility - // This fixes placeholder not showing with object map items - const normalizedItems = props.items ? normalizeItems(props.items) : undefined; + // Keep equivalent object maps stable for Base UI's external store. + const normalizedItems = useNormalizedItems(props.items); + const hasObjectMapItems = + props.items !== undefined && !Array.isArray(props.items); // Auto-render children from items if no explicit children provided const renderedChildren = children ? children - : props.items - ? renderOptionsFromItems(props.items) + : normalizedItems + ? renderOptionsFromItems(normalizedItems, hasObjectMapItems) : null; // Wrap renderValue to handle null values properly: