diff --git a/.changeset/rsc-lazy-icon.md b/.changeset/rsc-lazy-icon.md new file mode 100644 index 0000000000..ff56577520 --- /dev/null +++ b/.changeset/rsc-lazy-icon.md @@ -0,0 +1,5 @@ +--- +"@cloudflare/kumo": patch +--- + +`Button`, `LinkButton`, `Badge`, `DropdownMenu` items, `Sidebar.MenuButton` and `InputGroup.Button` no longer crash with React error #306 when `icon` is an element passed from a React Server Component. React can deliver such an element as a lazy reference, which Kumo now unwraps before deciding whether to render `icon` as an element or a component (#891). diff --git a/packages/kumo/ai/USAGE.md b/packages/kumo/ai/USAGE.md index c5f32da522..b36557ea24 100644 --- a/packages/kumo/ai/USAGE.md +++ b/packages/kumo/ai/USAGE.md @@ -136,6 +136,12 @@ import { PlusIcon, TrashIcon, GearIcon } from "@phosphor-icons/react"; Delete ``` +`icon` props also accept a rendered element, including one passed from a React Server Component: + +```tsx + +``` + ## Controlled State Reference | Component | Value Prop | Change Callback | Default Prop | diff --git a/packages/kumo/src/components/badge/badge.test.tsx b/packages/kumo/src/components/badge/badge.test.tsx index d4597d00fd..ca763134fa 100644 --- a/packages/kumo/src/components/badge/badge.test.tsx +++ b/packages/kumo/src/components/badge/badge.test.tsx @@ -1,3 +1,4 @@ +import { version, type ReactElement, type ReactNode } from "react"; import { describe, expect, it, vi } from "vite-plus/test"; import { render, screen } from "@testing-library/react"; import { ArrowRightIcon } from "@phosphor-icons/react"; @@ -38,6 +39,22 @@ describe("Badge", () => { expect(screen.getByText("Next").querySelector("svg")).toBeTruthy(); }); + // React Flight (RSC) can deliver an element prop as a lazy reference whose + // resolved value is an element, not a component (#891). + it.skipIf(version.startsWith("18."))( + "renders an icon element delivered as an RSC lazy reference", + () => { + const lazyElement = { + $$typeof: Symbol.for("react.lazy"), + _payload: , + _init: (payload: ReactElement) => payload, + } as unknown as ReactNode; + + render(Next); + expect(screen.getByText("Next").querySelector("svg")).toBeTruthy(); + }, + ); + describe("filled appearance (default)", () => { it("applies variant classes for filled badges", () => { render(Error); diff --git a/packages/kumo/src/components/badge/badge.tsx b/packages/kumo/src/components/badge/badge.tsx index 188cd56d71..6146f2a6a8 100644 --- a/packages/kumo/src/components/badge/badge.tsx +++ b/packages/kumo/src/components/badge/badge.tsx @@ -2,6 +2,7 @@ import React, { type ReactNode } from "react"; import type { Icon } from "@phosphor-icons/react"; import { cn } from "../../utils/cn"; import { resolveVariant } from "../../utils/resolve-variant"; +import { unwrapLazyElement } from "../../utils/unwrap-lazy-element"; /** Base styles applied to all badge variants. */ export const KUMO_BADGE_BASE_STYLES = @@ -163,7 +164,8 @@ export function badgeVariants({ // Legacy type alias for backwards compatibility export type BadgeVariant = KumoBadgeVariant; -const renderIconNode = (IconComponent?: Icon | ReactNode) => { +const renderIconNode = (iconProp?: Icon | ReactNode) => { + const IconComponent = unwrapLazyElement(iconProp); if (!IconComponent) return null; const Component = IconComponent as React.ComponentType< Record diff --git a/packages/kumo/src/components/button/button.test.tsx b/packages/kumo/src/components/button/button.test.tsx index 583bdd0f1d..80bd270ffb 100644 --- a/packages/kumo/src/components/button/button.test.tsx +++ b/packages/kumo/src/components/button/button.test.tsx @@ -1,7 +1,7 @@ import React from "react"; import { describe, expect, it } from "vite-plus/test"; import { render, screen } from "@testing-library/react"; -import { Plus } from "@phosphor-icons/react"; +import { Plus, type Icon } from "@phosphor-icons/react"; import { Button, RefreshButton, LinkButton, buttonVariants } from "./button"; describe("Button", () => { @@ -150,6 +150,35 @@ describe("Button", () => { const svg = button.querySelector("svg"); expect(svg).toBeTruthy(); }); + + // React Flight (RSC) can deliver an element prop as a lazy reference whose + // resolved value is an element, not a component (#891). + it.skipIf(React.version.startsWith("18."))( + "accepts an element delivered as an RSC lazy reference", + () => { + const lazyElement = { + $$typeof: Symbol.for("react.lazy"), + _payload: , + _init: (payload: React.ReactElement) => payload, + } as unknown as React.ReactNode; + + render(); + const button = screen.getByRole("button", { name: "Add" }); + expect(button.querySelector("svg")).toBeTruthy(); + }, + ); + + it("accepts a React.lazy icon component", async () => { + const LazyPlus = React.lazy(() => Promise.resolve({ default: Plus })); + + render( + + + , + ); + const button = await screen.findByRole("button", { name: "Add" }); + expect(button.querySelector("svg")).toBeTruthy(); + }); }); it("title prop wraps in Tooltip and removes native title attribute", () => { diff --git a/packages/kumo/src/components/button/button.tsx b/packages/kumo/src/components/button/button.tsx index 9025f77712..8395f74ee2 100644 --- a/packages/kumo/src/components/button/button.tsx +++ b/packages/kumo/src/components/button/button.tsx @@ -5,6 +5,7 @@ import { Tooltip } from "../tooltip/tooltip"; import { cn } from "../../utils/cn"; import { resolveVariant } from "../../utils/resolve-variant"; import { useLinkComponent } from "../../utils/link-provider"; +import { unwrapLazyElement } from "../../utils/unwrap-lazy-element"; /** Button variant definitions mapping shape, size, and variant names to their Tailwind classes. */ export const KUMO_BUTTON_VARIANTS = { @@ -202,7 +203,8 @@ const getTextClasses = ( }; // Normalize icon prop to support both React elements and component types -const renderIconNode = (IconComponent?: Icon | React.ReactNode) => { +const renderIconNode = (iconProp?: Icon | React.ReactNode) => { + const IconComponent = unwrapLazyElement(iconProp); if (!IconComponent) return null; if (React.isValidElement(IconComponent)) return IconComponent; const Comp = IconComponent as React.ComponentType>; diff --git a/packages/kumo/src/components/dropdown/dropdown.test.tsx b/packages/kumo/src/components/dropdown/dropdown.test.tsx index 50756f52f4..c073f97a3b 100644 --- a/packages/kumo/src/components/dropdown/dropdown.test.tsx +++ b/packages/kumo/src/components/dropdown/dropdown.test.tsx @@ -1,3 +1,4 @@ +import { version } from "react"; import { render, screen } from "@testing-library/react"; import { describe, expect, it } from "vite-plus/test"; import { CopyIcon, GearIcon } from "@phosphor-icons/react"; @@ -108,6 +109,26 @@ describe("DropdownMenu", () => { ); expect(screen.getByTestId("custom-icon").className).toContain("h-4 w-4"); }); + + // React Flight (RSC) can deliver an element prop as a lazy reference whose + // resolved value is an element, not a component (#891). + it.skipIf(version.startsWith("18."))( + "accepts an icon element delivered as an RSC lazy reference", + () => { + const lazyElement = { + $$typeof: Symbol.for("react.lazy"), + _payload: , + _init: (payload: React.ReactElement) => payload, + } as unknown as React.ReactNode; + + renderMenu( + Item, + ); + expect(screen.getByTestId("custom-icon").className).toContain( + "h-4 w-4", + ); + }, + ); }); describe("icon spacing", () => { diff --git a/packages/kumo/src/components/dropdown/dropdown.tsx b/packages/kumo/src/components/dropdown/dropdown.tsx index d7f6c6a796..f504a90547 100644 --- a/packages/kumo/src/components/dropdown/dropdown.tsx +++ b/packages/kumo/src/components/dropdown/dropdown.tsx @@ -3,6 +3,7 @@ import * as React from "react"; import { cn } from "../../utils/cn"; import { resolveVariant } from "../../utils/resolve-variant"; import { useLinkComponent } from "../../utils/link-provider"; +import { unwrapLazyElement } from "../../utils/unwrap-lazy-element"; import { usePortalContainer, type PortalContainer, @@ -64,7 +65,8 @@ export function dropdownVariants({ * identically. The slot also lets icon-less rows detect that a sibling has an * icon and inset themselves to keep the text aligned. */ -const renderIconNode = (IconComponent?: Icon | React.ReactNode) => { +const renderIconNode = (iconProp?: Icon | React.ReactNode) => { + const IconComponent = unwrapLazyElement(iconProp); if (!IconComponent) return null; let icon: React.ReactNode; if (React.isValidElement<{ className?: string }>(IconComponent)) { diff --git a/packages/kumo/src/components/input-group/input-group-button.tsx b/packages/kumo/src/components/input-group/input-group-button.tsx index e0c0bfea07..e2e8b6d275 100644 --- a/packages/kumo/src/components/input-group/input-group-button.tsx +++ b/packages/kumo/src/components/input-group/input-group-button.tsx @@ -5,6 +5,7 @@ import React, { type ReactNode, } from "react"; import { cn } from "../../utils/cn"; +import { unwrapLazyElement } from "../../utils/unwrap-lazy-element"; import { type ButtonProps, Button as ButtonExternal } from "../button/button"; import { Tooltip, type KumoTooltipSide } from "../tooltip/tooltip"; import type { KumoInputSize } from "../input/input"; @@ -71,7 +72,7 @@ export const Button = forwardRef< disabled, tooltip, tooltipSide = "bottom", - icon, + icon: iconProp, ...props }: PropsWithChildren, ref: React.ForwardedRef, @@ -117,6 +118,7 @@ export const Button = forwardRef< ? INPUT_GROUP_SIZE[context.size ?? "base"].iconSize : undefined; + const icon = unwrapLazyElement(iconProp); const sizedIcon = icon && contextIconSize && diff --git a/packages/kumo/src/components/input-group/input-group.test.tsx b/packages/kumo/src/components/input-group/input-group.test.tsx index fdb174480c..b02b789b20 100644 --- a/packages/kumo/src/components/input-group/input-group.test.tsx +++ b/packages/kumo/src/components/input-group/input-group.test.tsx @@ -616,6 +616,31 @@ describe("InputGroup", () => { expect(icon.getAttribute("data-size")).toBe("42"); }); + // React Flight (RSC) can deliver an element prop as a lazy reference whose + // resolved value is an element, not a component (#891). + it.skipIf(React.version.startsWith("18."))( + "passes an element delivered as an RSC lazy reference through unchanged", + () => { + const lazyElement = { + $$typeof: Symbol.for("react.lazy"), + _payload: , + _init: (payload: React.ReactElement) => payload, + } as unknown as React.ReactNode; + + render( + + + + + + , + ); + + const icon = screen.getByTestId("element-icon"); + expect(icon.getAttribute("data-size")).toBe("42"); + }, + ); + it("does not inject icon size outside InputGroup context", () => { const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); diff --git a/packages/kumo/src/components/sidebar/sidebar.test.tsx b/packages/kumo/src/components/sidebar/sidebar.test.tsx index 61a3c878c3..22922f6499 100644 --- a/packages/kumo/src/components/sidebar/sidebar.test.tsx +++ b/packages/kumo/src/components/sidebar/sidebar.test.tsx @@ -12,6 +12,7 @@ import { within, } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; +import { version } from "react"; import { renderToString } from "react-dom/server"; import { hydrateRoot } from "react-dom/client"; import { describe, it, expect, vi, beforeEach } from "vite-plus/test"; @@ -1047,6 +1048,31 @@ describe("Sidebar.MenuButton", () => { expect(link!.getAttribute("data-active")).toBe("true"); expect(link!.getAttribute("data-kumo-part")).toBe("menu-button-link"); }); + + // React Flight (RSC) can deliver an element prop as a lazy reference whose + // resolved value is an element, not a component (#891). + it.skipIf(version.startsWith("18."))( + "should render an icon element delivered as an RSC lazy reference", + () => { + const lazyElement = { + $$typeof: Symbol.for("react.lazy"), + _payload: , + _init: (payload: React.ReactElement) => payload, + } as unknown as React.ReactNode; + + render( + + + + Home + + + , + ); + const button = screen.getByRole("button", { name: "Home" }); + expect(within(button).getByTestId("lazy-icon")).toBeTruthy(); + }, + ); }); describe("Sidebar.MenuSub", () => { diff --git a/packages/kumo/src/components/sidebar/sidebar.tsx b/packages/kumo/src/components/sidebar/sidebar.tsx index 93e301877b..00cf74d3d2 100644 --- a/packages/kumo/src/components/sidebar/sidebar.tsx +++ b/packages/kumo/src/components/sidebar/sidebar.tsx @@ -18,6 +18,7 @@ import { ScrollArea as ScrollAreaBase } from "@base-ui/react/scroll-area"; import { CaretRightIcon, XIcon } from "@phosphor-icons/react"; import { cn } from "../../utils/cn"; import { useLinkComponent } from "../../utils/link-provider"; +import { unwrapLazyElement } from "../../utils/unwrap-lazy-element"; import { SkeletonLine } from "../loader/skeleton-line"; import { Tooltip, TooltipProvider } from "../tooltip"; import { Button } from "../button"; @@ -1500,9 +1501,10 @@ const SidebarMenuButton = forwardRef( // Render icon — supports both component types and React elements const iconNode = (() => { - if (!IconProp) return null; - if (React.isValidElement(IconProp)) return IconProp; - const Comp = IconProp as React.ComponentType<{ className?: string }>; + const icon = unwrapLazyElement(IconProp); + if (!icon) return null; + if (React.isValidElement(icon)) return icon; + const Comp = icon as React.ComponentType<{ className?: string }>; return ( value, + }; +} + +async function unwrapAfterLoad(value: unknown): Promise { + try { + return unwrapLazyElement(value); + } catch (pending) { + await pending; + return unwrapLazyElement(value); + } +} + +describe("unwrapLazyElement", () => { + it.skipIf(isReact18)( + "unwraps a lazy reference that resolves to an element", + () => { + const result = unwrapLazyElement(flightLazy()); + expect(React.isValidElement(result)).toBe(true); + expect((result as React.ReactElement).type).toBe(Plus); + }, + ); + + it.skipIf(isReact18)("unwraps nested lazy references", () => { + const result = unwrapLazyElement(flightLazy(flightLazy())); + expect((result as React.ReactElement).type).toBe(Plus); + }); + + it.each([ + ["forwardRef", Plus], + ["function", () => ], + ["memo", React.memo(() => )], + ])("returns a React.lazy %s component unchanged", async (_, component) => { + const LazyIcon = React.lazy(() => + Promise.resolve({ default: component as React.ComponentType }), + ); + expect(await unwrapAfterLoad(LazyIcon)).toBe(LazyIcon); + }); + + it("returns non-lazy values unchanged", () => { + const element = ; + expect(unwrapLazyElement(element)).toBe(element); + expect(unwrapLazyElement(Plus)).toBe(Plus); + expect(unwrapLazyElement("text")).toBe("text"); + expect(unwrapLazyElement(null)).toBe(null); + expect(unwrapLazyElement(undefined)).toBe(undefined); + }); + + it("keeps the wrapper when it does not resolve to an element", () => { + const lazy = flightLazy("text"); + expect(unwrapLazyElement(lazy)).toBe(lazy); + }); +}); diff --git a/packages/kumo/src/utils/unwrap-lazy-element.ts b/packages/kumo/src/utils/unwrap-lazy-element.ts new file mode 100644 index 0000000000..75d6c9f78e --- /dev/null +++ b/packages/kumo/src/utils/unwrap-lazy-element.ts @@ -0,0 +1,53 @@ +import React from "react"; + +const REACT_LAZY_TYPE = Symbol.for("react.lazy"); + +function isLazyReference(value: unknown): boolean { + return ( + typeof value === "object" && + value !== null && + (value as { $$typeof?: unknown }).$$typeof === REACT_LAZY_TYPE + ); +} + +function isThenable(value: unknown): boolean { + return ( + typeof value === "object" && + value !== null && + typeof (value as { then?: unknown }).then === "function" + ); +} + +/** + * Unwraps a lazy element reference produced by React Flight. + * + * When an element prop (e.g. `icon={}`) is passed from a Server + * Component, the Flight client can deliver it as a lazy wrapper instead of an + * element (https://github.com/react/react/issues/32392, closed as not + * planned). The wrapper fails `React.isValidElement`, and rendering it as a + * component throws React error #306 because it resolves to an element rather + * than a component. + * + * `React.Children.toArray` resolves the wrapper to its element, the same + * approach Base UI (`unwrapLazyRenderProp`) and Ark UI use. A pending wrapper + * suspends. + * + * Anything that does not resolve to an element is returned unchanged, so the + * caller renders it as a component as before. That includes a `React.lazy` + * component: `toArray` returns nothing for one resolving to a function, and + * throws for one resolving to a `forwardRef` or `memo` object (such as a + * Phosphor icon). It also includes every lazy value on React 18, whose + * `toArray` does not resolve lazy values; Server Components require React 19. + * A genuine error resurfaces when React renders the returned wrapper. + */ +export function unwrapLazyElement(value: T): T { + if (!isLazyReference(value)) return value; + let unwrapped: unknown; + try { + unwrapped = React.Children.toArray(value as React.ReactNode)[0]; + } catch (error) { + if (isThenable(error)) throw error; + return value; + } + return React.isValidElement(unwrapped) ? (unwrapped as T) : value; +}