Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/rich-horses-dress.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@heroui/system-rsc": patch
---

fix extendVariants return type error(#5778)
10 changes: 5 additions & 5 deletions packages/core/system-rsc/src/extend-variants.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,11 @@ export type ExtendVariants = {
},
opts?: Options,
): ForwardRefExoticComponent<
PropsWithoutRef<
CP & {
[key in keyof V]?: StringToBoolean<keyof V[key]>;
}
> &
PropsWithoutRef<{
[key in keyof CP | keyof V]?:
| (key extends keyof CP ? CP[key] : never)
| (key extends keyof V ? StringToBoolean<keyof V[key]> : never);
}> &
Comment on lines +106 to +110
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Oct 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify that required component props remain required.

The new mapped type makes all keys optional with ?:, including props from CP (component props). If the base component has required props, marking them optional could weaken type safety and allow consumers to omit required props without type errors, leading to runtime failures.

Run the following script to check for components with required props that use extendVariants:

Additionally, consider testing this change with a component that has required props to confirm they are correctly enforced:

// Test case: Component with required prop
const BaseButton = forwardRef<HTMLButtonElement, { onClick: () => void; label: string }>((props, ref) => {
  return <button ref={ref} onClick={props.onClick}>{props.label}</button>;
});

const ExtendedButton = extendVariants(BaseButton, {
  variants: {
    color: {
      primary: "bg-blue-500",
      secondary: "bg-gray-500"
    }
  }
});

// This should error if onClick and label are required, but won't with the current type
const test = <ExtendedButton color="primary" />;

If this weakens type safety unintentionally, consider preserving the required/optional nature of CP props while adding V props as optional:

PropsWithoutRef<CP & {
  [key in keyof V]?: StringToBoolean<keyof V[key]>;
}> &
  RefAttributes<InferRef<C>>

🏁 Script executed:

#!/bin/bash
# Description: Search for extendVariants usage with components that might have required props.

# Find extendVariants calls
rg -nP --type=ts --type=tsx -C3 'extendVariants\s*\(' -g '!*.d.ts' -g '!node_modules'

Length of output: 27


🏁 Script executed:

#!/bin/bash
# Search for extendVariants usage across .ts and .tsx files
rg -nP 'extendVariants\s*\(' -C3 -g '*.ts' -g '*.tsx' -g '!node_modules'

Length of output: 3378


Preserve required component props in extendVariants mapping
The current [key in keyof CP | keyof V]? makes all CP props optional. Update to:

PropsWithoutRef<CP & { [K in keyof V]?: StringToBoolean<keyof V[K]> }> &
RefAttributes<InferRef<C>>

so CP’s required props stay required and only V’s props are optional.

🤖 Prompt for AI Agents
In packages/core/system-rsc/src/extend-variants.d.ts around lines 106 to 110,
the mapped type `[key in keyof CP | keyof V]?` makes all component props (CP)
optional; change the type so CP stays as-is (preserving required props) and only
V’s mapped props are optional by replacing the current PropsWithoutRef mapping
with PropsWithoutRef<CP & { [K in keyof V]?: StringToBoolean<keyof V[K]> }> &
RefAttributes<InferRef<C>>; ensure the resulting intersection keeps CP required
properties intact and adds optional variant-derived props, and include
RefAttributes<InferRef<C>> as specified.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works fine for me

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ITBoomBKStudio, thank you for confirming! If you've tested this and it's working as expected for your use cases, I'll defer to your judgment on this change.

(ᵔᴥᵔ)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ITBoomBKStudio, thank you for confirming! If you've tested this and it's working as expected for your use cases, I'll defer to your judgment on this change.

(ᵔᴥᵔ)

Can you also fix error which appears when I try to specify classes for slots in compoundVariants inside extendVariants:
compoundVariants: [
{

		color: "secondary",
		variant: "bordered",
		class: {
			trigger: [
				"border-gray border",
				"data-[open=true]:border-gray",
				"data-[focus=true]:border-gray"
			],
			value: "group-data-[has-value=true]:text-secondary",
		}
	}
]

Error: Object literal may only specify known properties, and 'trigger' does not exist in type 'ClassNameArray'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for usage examples of extendVariants with compoundVariants and slots
rg -nP 'compoundVariants.*class.*:' -C5 -g '*.ts' -g '*.tsx' -g '!node_modules' -g '!*.d.ts'

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Look for tailwind-variants type definitions to understand ClassProp
rg -nP 'ClassProp|ClassValue' -C3 -g '*.d.ts' --type=ts

Length of output: 2461


🏁 Script executed:

#!/bin/bash
# Find other components using extendVariants with slots to understand the pattern
ast-grep --pattern $'extendVariants($$$, {
  $$$
  slots: $$$
  $$$
})'

Length of output: 0


@ITBoomBKStudio, I can see the issue! The CompoundVariants type currently only accepts ClassValue (simple strings/arrays), but when you have slots, it should also accept SlotsClassValue<S> (objects with slot keys like trigger, value).

Here's the fix needed in packages/core/system-rsc/src/extend-variants.d.ts:

1. Update the CompoundVariants type to accept slots (Line 51):

type CompoundVariants<V, SV, S> = Array<VariantValue<V, SV> & ClassProp<S extends undefined ? ClassValue : ClassValue | SlotsClassValue<S>>>;

2. Update the ExtendVariants signature to pass S to CompoundVariants (Line 99):

export type ExtendVariants = {
  <
    C extends JSXElementConstructor<any>,
    CP extends ComponentProps<C>,
    S extends ComponentSlots<CP>,
    V extends ComposeVariants<CP, S>,
    SV extends SuggestedVariants<CP, S>,
    DV extends DefaultVariants<V, SV>,
    CV extends CompoundVariants<V, SV, S>,  // ← Add S here
  >(
    BaseComponent: C,
    styles: {
      variants?: V;
      defaultVariants?: DV;
      compoundVariants?: CV;
      slots?: S;
    },
    opts?: Options,
  ): ForwardRefExoticComponent<
    PropsWithoutRef<{
      [key in keyof CP | keyof V]?:
        | (key extends keyof CP ? CP[key] : never)
        | (key extends keyof V ? StringToBoolean<keyof V[key]> : never);
    }> &
      RefAttributes<InferRef<C>>
  >;
};

This change makes CompoundVariants slot-aware, so when slots are defined, the class property in compound variants can accept either:

  • Simple class strings/arrays (ClassValue)
  • Slot-based class objects (SlotsClassValue<S>) like your example with trigger and value

This follows the same pattern used by the Variants and GetSuggestedValues types in the file.


🧬 Code Graph Analysis Results
  • File: packages/core/system-rsc/src/extend-variants.d.ts, snippet: CompoundVariants type

    • Description: Defines how compoundVariants are structured as an array of objects combining VariantValue with ClassProp.

    • Snippet (complete as shown):
      type CompoundVariants<V, SV> = Array<VariantValue<V, SV> & ClassProp>;

    • Location: packages/core/system-rsc/src/extend-variants.d.ts, around the definition of CompoundVariants.

  • File: packages/core/system-rsc/src/extend-variants.d.ts, snippet: ExtendVariantProps

    • Description: Typing for the options exposed by ExtendVariants, including compoundVariants having a particular shape.

    • Snippet (complete as shown):
      export type ExtendVariantProps = {
      variants?: Record<string, Record<string, string>>;
      defaultVariants?: Record<string, string>;
      compoundVariants?: Array<Record<string, boolean | string | Record<string, string>>>;
      };

    • Location: packages/core/system-rsc/src/extend-variants.d.ts, near the section where ExtendVariantProps is defined.

  • File: packages/core/system-rsc/src/extend-variants.d.ts, snippet: ExtendVariants signature (generic parameters)

    • Description: Core function type signature for extendVariants, showing expected props shape including variants, defaultVariants, compoundVariants, and slots.

    • Snippet (complete as shown):
      export type ExtendVariants = {
      <
      C extends JSXElementConstructor,
      CP extends ComponentProps,
      S extends ComponentSlots,
      V extends ComposeVariants<CP, S>,
      SV extends SuggestedVariants<CP, S>,
      DV extends DefaultVariants<V, SV>,
      CV extends CompoundVariants<V, SV>,

      (
      BaseComponent: C,
      styles: {
      variants?: V;
      defaultVariants?: DV;
      compoundVariants?: CV;
      slots?: S;
      },
      opts?: Options,
      ): ForwardRefExoticComponent<
      PropsWithoutRef<{
      [key in keyof CP | keyof V]?:
      | (key extends keyof CP ? CP[key] : never)
      | (key extends keyof V ? StringToBoolean<keyof V[key]> : never);
      }> &
      RefAttributes<InferRef>
      ;
      };

    • Location: packages/core/system-rsc/src/extend-variants.d.ts, near the end where ExtendVariants is defined.

  • File: packages/core/system-rsc/src/extend-variants.js, snippet: runtime handling of variants (compoundVariants)

    • Description: Shows how the runtime extendVariants function destructures its styles and prepares to compute class names, including handling of compoundVariants.

    • Snippet (lines 104-150, as provided):
      export function extendVariants(BaseComponent, styles = {}, opts = {}) {
      const {variants, defaultVariants, compoundVariants} = styles || {};

      const slots = getSlots(variants);
      const hasSlots = typeof slots === "object" && Object.keys(slots).length !== 0;

      const ForwardedComponent = React.forwardRef((originalProps = {}, ref) => {
      const newProps = React.useMemo(() =>
      getClassNamesWithProps(
      {
      slots,
      variants,
      compoundVariants,
      props: originalProps,
      defaultVariants,
      hasSlots,
      opts,
      },
      [originalProps],
      ),
      );

      return React.createElement(BaseComponent, {...originalProps, ...newProps, ref});
      

      });

      // Add collection node function for collection-based components
      if (BaseComponent.getCollectionNode) {
      ForwardedComponent.getCollectionNode = (itemProps) => {
      const newProps = getClassNamesWithProps({
      slots,
      variants,
      compoundVariants,
      props: itemProps,
      defaultVariants,
      hasSlots,
      opts,
      });

        return BaseComponent.getCollectionNode({...itemProps, ...newProps});
      };
      

      }

      // To make dev tools show a proper name
      ForwardedComponent.displayName = Extended(${BaseComponent.displayName || BaseComponent.name});

      return ForwardedComponent;
      }

    • Location: packages/core/system-rsc/src/extend-variants.js, lines 104-150.

RefAttributes<InferRef<C>>
>;
};
Expand Down
Loading