diff --git a/.changeset/unlucky-drinks-stare.md b/.changeset/unlucky-drinks-stare.md new file mode 100644 index 0000000000..fa074f0818 --- /dev/null +++ b/.changeset/unlucky-drinks-stare.md @@ -0,0 +1,5 @@ +--- +"@heroui/system-rsc": patch +--- + +Fix `extendVariants` to correctly inherit variant metadata from the base component & fix rendering of components that use 'as' prop diff --git a/packages/core/system-rsc/__tests__/extend-variants.test.tsx b/packages/core/system-rsc/__tests__/extend-variants.test.tsx index 3d5ce5f41d..ad1be44f71 100644 --- a/packages/core/system-rsc/__tests__/extend-variants.test.tsx +++ b/packages/core/system-rsc/__tests__/extend-variants.test.tsx @@ -1,122 +1,116 @@ -import type {ExtendVariantProps, ExtendVariantWithSlotsProps} from "../src/extend-variants"; - import React from "react"; import {render, screen} from "@testing-library/react"; import {extendVariants} from "../src/extend-variants"; -import {Button} from "../test-utils/extend-components"; -import {Card} from "../test-utils/slots-component"; -import {Link} from "../../react/src"; - -const createExtendNoSlotsComponent = (styles: ExtendVariantProps = {}) => - extendVariants(Button, { - variants: { - isScalable: { - true: "scale-125", - false: "", +import {Link, Button, Card, CardHeader, CardBody, CardFooter} from "../../react/src"; + +const BaseButton = extendVariants(Button, { + variants: { + isScalable: { + true: "scale-125", + false: "", + }, + size: { + xl: "size--xl", + "2xl": "size--2xl", + }, + mySize: { + lg: "px-12 py-6 text-lg", + xl: "px-12 py-6 text-xl", + }, + }, + defaultVariants: { + size: "xl", + }, + compoundVariants: [ + { + isScalable: true, + size: "2xl", + class: "scale-150", + }, + ], +}); + +const BaseCard = extendVariants(Card, { + slots: { + base: "", + header: "", + body: "", + footer: "", + }, + variants: { + variant: { + flat: "", + filled: "", + test: "", + }, + shadow: { + none: { + base: "shadow-xs", }, - size: { - xl: "size--xl", - "2xl": "size--2xl", + sm: { + base: "shadow-xs", }, - mySize: { - lg: "px-12 py-6 text-lg", - xl: "px-12 py-6 text-xl", + xl: { + base: "shadow-xl", }, - ...styles?.variants, - }, - defaultVariants: { - size: "xl", - ...styles?.defaultVariants, }, - compoundVariants: styles?.compoundVariants ?? [ - { - isScalable: true, - size: "2xl", - class: "scale-150", + radius: { + none: { + base: "rounded-xs", + header: "rounded-xs", + footer: "rounded-xs", }, - ], - }); - -const createExtendSlotsComponent = (styles: ExtendVariantWithSlotsProps = {}) => - extendVariants(Card, { - variants: { - variant: { - flat: "", - filled: "", - test: "", - }, - shadow: { - none: { - base: "shadow-xs", - }, - sm: { - base: "shadow-xs", - }, - xl: { - base: "shadow-xl", - }, + sm: { + base: "rounded-sm", + header: "rounded-t-sm", + footer: "rounded-b-sm", }, - radius: { - none: { - base: "rounded-xs", - header: "rounded-xs", - footer: "rounded-xs", - }, - sm: { - base: "rounded-sm", - header: "rounded-t-sm", - footer: "rounded-b-sm", - }, - xl: { - base: "rounded-xl", - header: "rounded-t-xl", - footer: "rounded-b-xl", - }, + xl: { + base: "rounded-xl", + header: "rounded-t-xl", + footer: "rounded-b-xl", }, }, - defaultVariants: { - shadow: "xl", - radius: "xl", + }, + defaultVariants: { + shadow: "xl", + radius: "xl", + }, + compoundVariants: [ + { + shadow: "none", + radius: "none", + class: "rounded-sm", }, - compoundVariants: styles?.compoundVariants ?? [ - { - shadow: "none", - radius: "none", - class: "rounded-sm", + { + shadow: "none", + class: { + header: "scale-75", }, - { - shadow: "none", - class: { - header: "scale-75", - }, - }, - ], - }); + }, + ], +}); describe("extendVariants function - no slots", () => { it("should render correctly", () => { - const Button2 = createExtendNoSlotsComponent(); - const wrapper = render(); + const wrapper = render(); expect(() => wrapper.unmount()).not.toThrow(); }); it("ref should be forwarded", () => { const ref = React.createRef(); - const Button2 = createExtendNoSlotsComponent(); - render(); + render(); expect(ref.current).not.toBeNull(); }); - test("as Link should work", () => { - const Button2 = createExtendNoSlotsComponent(); - + it("as Link should work", () => { const {container} = render( - + Press me - , + , ); // Link component from react package - verify it renders @@ -126,10 +120,9 @@ describe("extendVariants function - no slots", () => { expect(screen.getByText("Press me")).toBeInTheDocument(); }); - test("should render with given text", () => { - const Button2 = createExtendNoSlotsComponent(); + it("should render with given text", () => { const {container} = render( - Press me, + Press me, ); const button = container.querySelector("button"); @@ -137,10 +130,9 @@ describe("extendVariants function - no slots", () => { expect(button).toHaveTextContent("Press me"); }); - test("should override the base styles", () => { - const Button2 = createExtendNoSlotsComponent(); + it("should override the base styles", () => { const {container} = render( - Press me, + Press me, ); const button = container.querySelector("button"); @@ -148,17 +140,16 @@ describe("extendVariants function - no slots", () => { expect(button).toHaveClass("px-3 py-2 rounded-medium hover:opacity-80"); }); - test("should have the default variant styles - extended", () => { - const Button2 = createExtendNoSlotsComponent(); - const {container} = render(Press me); + it("should have the default variant styles - extended", () => { + const {container} = render(Press me); const button = container.querySelector("button"); expect(button).toHaveClass("size--xl"); }); - test("should have the default variant styles - original", () => { - const Button2 = createExtendNoSlotsComponent({ + it("should have the default variant styles - original", () => { + const Button2 = extendVariants(BaseButton, { defaultVariants: { size: "sm", }, @@ -171,12 +162,11 @@ describe("extendVariants function - no slots", () => { expect(button).toHaveClass("px-3 min-w-16 h-8 text-tiny gap-2 rounded-small"); }); - test("should include the compound variant styles - extended", () => { - const Button2 = createExtendNoSlotsComponent(); + it("should include the compound variant styles - extended", () => { const {container} = render( - + Press me - , + , ); const button = container.querySelector("button"); @@ -184,34 +174,38 @@ describe("extendVariants function - no slots", () => { expect(button).toHaveClass("scale-150"); }); - test("should include the compound variant styles - original", () => { - const Button2 = createExtendNoSlotsComponent({ + it("should include the compound variant styles - original", () => { + const Button2 = extendVariants(BaseButton, { + variants: { + size: { + custom: "size--custom", + }, + }, compoundVariants: [ { isScalable: true, - size: "lg", - class: "scale-150", + size: "custom", + class: "scale-[custom_scale]", }, ], }); const {container} = render( - + Press me , ); const button = container.querySelector("button"); - expect(button).toHaveClass("scale-150"); + expect(button).toHaveClass("scale-[custom_scale]"); }); - test("as prop should change rendered element to anchor", () => { - const Button2 = createExtendNoSlotsComponent(); + it("as prop should work with polymorphic base component", () => { const {container} = render( - + Link Button - , + , ); const link = container.querySelector("a"); @@ -221,9 +215,8 @@ describe("extendVariants function - no slots", () => { expect(link).toHaveTextContent("Link Button"); }); - test("as prop should change rendered element to div", () => { - const Button2 = createExtendNoSlotsComponent(); - const {container} = render(Div Button); + it("as prop should change rendered element to div", () => { + const {container} = render(Div Button); const div = container.querySelector("div"); @@ -236,50 +229,46 @@ describe("extendVariants function - no slots", () => { expect(button).not.toBeInTheDocument(); }); - test("ref should work with polymorphic component as anchor", () => { + it("ref should work with polymorphic component as anchor", () => { const ref = React.createRef(); - const Button2 = createExtendNoSlotsComponent(); render( - + Link - , + , ); expect(ref.current).toBeInstanceOf(HTMLAnchorElement); expect(ref.current).toHaveAttribute("href", "/test"); }); - test("variant styles should persist with 'as' prop", () => { - const Button2 = createExtendNoSlotsComponent(); + it("variant styles should persist with 'as' prop", () => { const {container} = render( - + Link - , + , ); const link = container.querySelector("a"); expect(link).toHaveClass("size--2xl"); - // isScalable=true triggers scale-125, but compound variant for size=2xl + isScalable overrides to scale-150 expect(link).toHaveClass("scale-150"); }); - test("compound variant styles should work with 'as' prop", () => { - const Button2 = createExtendNoSlotsComponent(); + it("compound variant styles should work with 'as' prop", () => { const {container} = render( - + Link - , + , ); const link = container.querySelector("a"); - expect(link).toHaveClass("scale-150"); // compound variant for size="2xl" + isScalable + expect(link).toHaveClass("scale-150"); }); - test("should respect defaultVariants.className", () => { - const Button2 = extendVariants(Button, { + it("should respect defaultVariants.className", () => { + const Button2 = extendVariants(BaseButton, { defaultVariants: { className: "w-full text-medium rounded-small", color: "primary", @@ -295,8 +284,8 @@ describe("extendVariants function - no slots", () => { expect(button).toHaveClass("rounded-small"); }); - test("should merge defaultVariants.className with props.className", () => { - const Button2 = extendVariants(Button, { + it("should merge defaultVariants.className with props.className", () => { + const Button2 = extendVariants(BaseButton, { defaultVariants: { className: "w-full text-medium rounded-small", color: "primary", @@ -318,32 +307,27 @@ describe("extendVariants function - no slots", () => { describe("extendVariants function - with slots", () => { it("should render correctly", () => { - const Card2 = createExtendSlotsComponent(); - const wrapper = render(); + const wrapper = render(); expect(() => wrapper.unmount()).not.toThrow(); }); it("ref should be forwarded", () => { const ref = React.createRef(); - const Card2 = createExtendSlotsComponent(); - render(); + render(); expect(ref.current).not.toBeNull(); }); - test("should render with given text", () => { - const Card2 = createExtendSlotsComponent(); - - render(Card Content); + it("should render with given text", () => { + render(Card Content); expect(screen.getByText("Card Content")).toBeInTheDocument(); }); - test("should override the base styles", () => { - const Card2 = createExtendSlotsComponent(); + it("should override the base styles", () => { const {container} = render( - Card Content, + Card Content, ); const card = container.querySelector("div"); @@ -351,18 +335,22 @@ describe("extendVariants function - with slots", () => { expect(card).toHaveClass("px-3 py-2 rounded-medium hover:opacity-80"); }); - test("should have the default variant styles - (base slot) extended", () => { - const Card2 = createExtendSlotsComponent(); - const {getByTestId} = render(Card Content); + it("should have the default variant styles - (base slot) extended", () => { + const {getByTestId} = render(Card Content); const baseEl = getByTestId("base"); expect(baseEl).toHaveClass("shadow-xl"); }); - test("should have all slots styles", () => { - const Card2 = createExtendSlotsComponent(); - const {getByTestId} = render(Card Content); + it("should have all slots styles", () => { + const {getByTestId} = render( + + Header + Body + Footer + , + ); const baseEl = getByTestId("base"); const headerEl = getByTestId("header"); @@ -379,19 +367,23 @@ describe("extendVariants function - with slots", () => { expect(footerEl).toHaveClass("rounded-b-xl"); }); - test("should override the slots styles", () => { - const Card2 = createExtendSlotsComponent(); - const {getByTestId} = render(Card Content); + it("should override the slots styles", () => { + const {getByTestId} = render( + + Card Content + , + ); const baseEl = getByTestId("base"); expect(baseEl).toHaveClass("shadow-xs"); }); - test("should override all slots styles", () => { - const Card2 = createExtendSlotsComponent(); + it("should override all slots styles", () => { const {getByTestId} = render( - Card Content, + + Header + , ); const baseEl = getByTestId("base"); @@ -401,13 +393,11 @@ describe("extendVariants function - with slots", () => { expect(headerEl).toHaveClass("rounded-none"); }); - test("should include the compound variant styles - extended", () => { - const Card2 = createExtendSlotsComponent(); - + it("should include the compound variant styles - extended", () => { const {getByTestId} = render( - - Card Content - , + + Header + , ); const baseEl = getByTestId("base"); @@ -417,45 +407,55 @@ describe("extendVariants function - with slots", () => { expect(headerEl).toHaveClass("scale-75"); }); - test("should include the compound variant styles - original", () => { - const Card2 = createExtendSlotsComponent({ + it("should include the compound variant styles - original", () => { + const Card2 = extendVariants(BaseCard, { + variants: { + shadow: { + test: "", + }, + }, compoundVariants: [ { - shadow: "none", + shadow: "test", radius: "sm", - class: "rounded-xl", + class: "rounded-[test_sm]", }, { - radius: "sm", + shadow: "test", class: { - header: "scale-150", + header: "scale-[test]", }, }, ], }); const {getByTestId} = render( - - Card Content + + Header , ); const baseEl = getByTestId("base"); const headerEl = getByTestId("header"); - expect(baseEl).toHaveClass("rounded-xl"); - expect(headerEl).toHaveClass("scale-150"); + expect(baseEl).toHaveClass("rounded-[test_sm]"); + expect(headerEl).toHaveClass("scale-[test]"); }); - test("should override base component slots with direct slots option", () => { - const Card2 = extendVariants(Card, { + it("should override base component slots with direct slots option", () => { + const Card2 = extendVariants(BaseCard, { slots: { header: "!font-bold !text-lg", footer: "!bg-red-500", }, }); - const {getByTestId} = render(Card Content); + const {getByTestId} = render( + + Header + Footer + , + ); const headerEl = getByTestId("header"); const footerEl = getByTestId("footer"); @@ -465,8 +465,8 @@ describe("extendVariants function - with slots", () => { expect(footerEl).toHaveClass("!bg-red-500"); }); - test("should merge direct slots with variant-based slots", () => { - const Card2 = extendVariants(Card, { + it("should merge direct slots with variant-based slots", () => { + const Card2 = extendVariants(BaseCard, { slots: { header: "!font-bold", }, @@ -482,7 +482,11 @@ describe("extendVariants function - with slots", () => { }, }); - const {getByTestId} = render(Card Content); + const {getByTestId} = render( + + Header + , + ); const baseEl = getByTestId("base"); const headerEl = getByTestId("header"); @@ -491,8 +495,8 @@ describe("extendVariants function - with slots", () => { expect(headerEl).toHaveClass("!font-bold"); }); - test("direct slots should override variant-based slots for the same slot", () => { - const Card2 = extendVariants(Card, { + it("direct slots should override variant-based slots for the same slot", () => { + const Card2 = extendVariants(BaseCard, { slots: { base: "!bg-blue-500", }, @@ -508,13 +512,11 @@ describe("extendVariants function - with slots", () => { }, }); - const {getByTestId} = render(Card Content); + const {getByTestId} = render(); const baseEl = getByTestId("base"); - // Direct slots should be applied expect(baseEl).toHaveClass("!bg-blue-500"); - // Variant-based slots should also be applied (they merge) expect(baseEl).toHaveClass("shadow-xl"); }); }); diff --git a/packages/core/system-rsc/src/extend-variants.d.ts b/packages/core/system-rsc/src/extend-variants.d.ts index 53c8e4d60f..41e838c8dc 100644 --- a/packages/core/system-rsc/src/extend-variants.d.ts +++ b/packages/core/system-rsc/src/extend-variants.d.ts @@ -70,6 +70,7 @@ export type ExtendVariantProps = { }; export type ExtendVariantWithSlotsProps = { + slots?: Record; variants?: Record>>; defaultVariants?: Record>; compoundVariants?: Array>>; diff --git a/packages/core/system-rsc/src/extend-variants.js b/packages/core/system-rsc/src/extend-variants.js index b9e43ae9ac..60a8316a10 100644 --- a/packages/core/system-rsc/src/extend-variants.js +++ b/packages/core/system-rsc/src/extend-variants.js @@ -123,32 +123,36 @@ function getClassNamesWithProps({ export function extendVariants(BaseComponent, styles = {}, opts = {}) { const {variants, defaultVariants, compoundVariants, slots: directSlots} = styles || {}; - const inferredSlots = getSlots(variants); + const inheritedVariants = BaseComponent.__variants ?? {}; + + const mergedVariants = { + ...inheritedVariants, + ...variants, + }; + + const inferredSlots = getSlots(mergedVariants); const slots = directSlots ? {...inferredSlots, ...directSlots} : inferredSlots; const hasSlots = typeof slots === "object" && Object.keys(slots).length !== 0; const ForwardedComponent = React.forwardRef((originalProps = {}, ref) => { - // Extract 'as' prop if present - const {as: Component = BaseComponent, ...restProps} = originalProps; - const newProps = React.useMemo(() => getClassNamesWithProps( { slots, - variants, + variants: mergedVariants, compoundVariants, - props: restProps, // Use restProps without 'as' + props: originalProps, defaultVariants, hasSlots, opts, }, - [restProps], + [originalProps], ), ); - return React.createElement(Component, {...restProps, ...newProps, ref}); + return React.createElement(BaseComponent, {...originalProps, ...newProps, ref}); }); // Add collection node function for collection-based components @@ -156,7 +160,7 @@ export function extendVariants(BaseComponent, styles = {}, opts = {}) { ForwardedComponent.getCollectionNode = (itemProps) => { const newProps = getClassNamesWithProps({ slots, - variants, + variants: mergedVariants, compoundVariants, props: itemProps, defaultVariants, @@ -171,5 +175,7 @@ export function extendVariants(BaseComponent, styles = {}, opts = {}) { // To make dev tools show a proper name ForwardedComponent.displayName = `Extended(${BaseComponent.displayName || BaseComponent.name})`; + ForwardedComponent.__variants = mergedVariants; + return ForwardedComponent; }