fix(extendVariants): return component type error #5795
Conversation
🦋 Changeset detectedLatest commit: e5cad93 The changes in this PR will be included in the next version bump. This PR includes changesets to release 37 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@IsDyh01 is attempting to deploy a commit to the HeroUI Inc Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughUpdates the ExtendVariants type signature in packages/core/system-rsc/src/extend-variants.d.ts to unify props mapping across CP and V keys, affecting type-level props resolution only. Adds a changeset entry documenting a patch for @heroui/system-rsc and the extendVariants return type fix (issue #5778). Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/rich-horses-dress.md(1 hunks)packages/core/system-rsc/src/extend-variants.d.ts(1 hunks)
🔇 Additional comments (1)
.changeset/rich-horses-dress.md (1)
1-5: LGTM!The changeset follows the correct format and accurately documents the patch for
@heroui/system-rscwith a reference to issue #5778.
| PropsWithoutRef<{ | ||
| [key in keyof CP | keyof V]?: | ||
| | (key extends keyof CP ? CP[key] : never) | ||
| | (key extends keyof V ? StringToBoolean<keyof V[key]> : never); | ||
| }> & |
There was a problem hiding this comment.
🧩 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.
There was a problem hiding this comment.
@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.
(ᵔᴥᵔ)
There was a problem hiding this comment.
@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'
There was a problem hiding this comment.
🧩 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 withtriggerandvalue
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.
-
@heroui/accordion
@heroui/alert
@heroui/autocomplete
@heroui/avatar
@heroui/badge
@heroui/breadcrumbs
@heroui/button
@heroui/calendar
@heroui/card
@heroui/checkbox
@heroui/chip
@heroui/code
@heroui/date-input
@heroui/date-picker
@heroui/divider
@heroui/drawer
@heroui/dropdown
@heroui/form
@heroui/image
@heroui/input
@heroui/input-otp
@heroui/kbd
@heroui/link
@heroui/listbox
@heroui/menu
@heroui/modal
@heroui/navbar
@heroui/number-input
@heroui/pagination
@heroui/popover
@heroui/progress
@heroui/radio
@heroui/ripple
@heroui/scroll-shadow
@heroui/select
@heroui/skeleton
@heroui/slider
@heroui/snippet
@heroui/spacer
@heroui/spinner
@heroui/switch
@heroui/table
@heroui/tabs
@heroui/toast
@heroui/tooltip
@heroui/user
@heroui/react
@heroui/system
@heroui/system-rsc
@heroui/theme
@heroui/use-aria-accordion
@heroui/use-aria-accordion-item
@heroui/use-aria-button
@heroui/use-aria-link
@heroui/use-aria-modal-overlay
@heroui/use-aria-multiselect
@heroui/use-aria-overlay
@heroui/use-callback-ref
@heroui/use-clipboard
@heroui/use-data-scroll-overflow
@heroui/use-disclosure
@heroui/use-draggable
@heroui/use-form-reset
@heroui/use-image
@heroui/use-infinite-scroll
@heroui/use-intersection-observer
@heroui/use-is-mobile
@heroui/use-is-mounted
@heroui/use-measure
@heroui/use-pagination
@heroui/use-real-shape
@heroui/use-ref-state
@heroui/use-resize
@heroui/use-safe-layout-effect
@heroui/use-scroll-position
@heroui/use-ssr
@heroui/use-theme
@heroui/use-update-effect
@heroui/use-viewport-size
@heroui/aria-utils
@heroui/dom-animation
@heroui/framer-utils
@heroui/react-rsc-utils
@heroui/react-utils
@heroui/shared-icons
@heroui/shared-utils
@heroui/stories-utils
@heroui/test-utils
commit: |
|
@IsDyh01 Hey! I’ve been exploring this issue too and noticed your PR. Would you be okay if I opened a new PR that builds on this one and includes these improvements? |
|
Tip For best results, initiate chat on the files or code changes. @ITBoomBKStudio, it's great to see community collaboration on this fix! While the decision is ultimately between you and @IsDyh01, here are a couple of approaches you might consider: Option 1: Collaborate on this PR
Option 2: New PR
Since you're adding significant improvements beyond the original scope (the CompoundVariants issue), Option 2 might be cleaner for reviewers to assess both fixes independently. However, if the changes are tightly coupled, Option 1 could work well. Either way, crediting the original work and maintaining clear communication (as you're doing) is the right approach. I'm looking forward to seeing the CompoundVariants issue resolved—that sounds like it will be a valuable improvement! |
2 participants
Closes #5778
📝 Description
Only the types related to the attributes of the returned component have been modified.
⛳️ Current behavior (updates)
incorrect variant type
🚀 New behavior
💣 Is this a breaking change (Yes/No):
no
📝 Additional Information
Summary by CodeRabbit
extendVariantsdoes not show variants #5778), improving IDE IntelliSense and reducing compile-time warnings. No runtime behavior changes.