fix(ui): forward refs in Button so Radix asChild slots work#929
Conversation
The custom Button was a plain function component, so when used as a child
of `<DropdownMenu.Trigger asChild>` the ref Radix Slot composed onto it
never reached the underlying `<button>` DOM node. Radix's PopperAnchor
ended up with a null anchor, Floating UI's computePosition never ran, and
the CopyPageDropdown wrapper stayed at the off-screen fallback
`translate(0px, -200%)` so the menu was invisible.
Wrap Button in React.forwardRef and pass the ref through to
createElement; switch ButtonProps to ComponentPropsWithRef so `ref={}`
type-checks for consumers.
📝 WalkthroughWalkthroughThe ChangesButton ref forwarding
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/ui/Button.tsx`:
- Around line 102-107: The implementation uses React.forwardRef and a
monomorphic ref type causing unsafe casts; change Button to the React 19
polymorphic pattern by removing React.forwardRef and making Button a generic
function component that accepts ref as a normal prop. Update ButtonInnerProps
(and any type alias used by ButtonComponent) to be generic over TElement so the
component signature matches ButtonComponent, accept an optional ref prop (e.g.,
ref?: React.Ref<TElement>) in the props, and remove the unsafe casts currently
applied to props and to the exported component (the "props as ..." and "as
unknown as ButtonComponent" usages). Ensure the exported Button has the generic
polymorphic type (ButtonComponent) without using forwardRef.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| type ButtonInnerProps = ButtonOwnProps & Record<string, unknown> | ||
|
|
||
| export const Button: ButtonComponent = React.forwardRef< | ||
| HTMLElement, | ||
| ButtonInnerProps | ||
| >(function Button(props, ref) { |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In React 19 + TypeScript, what is the recommended way to type a polymorphic component with an asprop and forwarded refs without usingas unknown as casts?
💡 Result:
In React 19, the ref prop is now treated as a standard prop, which simplifies the implementation of polymorphic components by removing the need for forwardRef [1][2][3]. To create a type-safe polymorphic component without using as unknown as casts, you should define a generic component that accepts an as prop and destructures ref directly from the props object [2][3]. The recommended approach involves: 1. Defining a generic type for your component props that extends React.ElementType [4][5]. 2. Using React.ComponentPropsWithRef<C> to ensure the component correctly inherits the props and ref type of the element specified by the as prop [6][4]. 3. Destructuring ref and as from the props and passing them directly to the underlying element [2]. Example Pattern: type PolymorphicProps<C extends React.ElementType, Props = {}> = Props & { as?: C } & Omit<React.ComponentPropsWithRef, keyof Props | 'as'>; export const PolymorphicComponent = ({ as, ref,...props }: PolymorphicProps<C, { children?: React.ReactNode }>) => { const Component = as || 'div'; return <Component ref={ref} {...props} />; }; By destructuring ref as a standard prop, you avoid the complex type signatures previously required by forwardRef [2][3]. This pattern allows TypeScript to correctly infer the element type and its associated ref, maintaining full type safety without requiring type assertions [4]. If you are working in a codebase that still requires forwardRef for compatibility, you can continue to use it, but it is no longer necessary for new components in React 19 [1][2].
Citations:
- 1: https://stackoverflow.com/questions/79300987/react-19-ref-as-prop-and-typescript
- 2: https://coldfusion-example.blogspot.com/2025/12/react-19-migration-how-to-fix.html
- 3: https://dev.to/reactchallenges/new-way-of-passing-ref-to-child-in-react-19-22ja
- 4: https://www.tsx-patterns.com/2026/01/effective-typescript-patterns-for.html
- 5: https://javascript.plainenglish.io/building-a-polymorphic-component-in-react-and-typescript-d9f236950af4
- 6: https://www.benmvp.com/blog/forwarding-refs-polymorphic-react-component-typescript/
🏁 Script executed:
# Check React version
fd -e json -e ts -e js package.json tsconfig.json | head -5Repository: TanStack/tanstack.com
Length of output: 166
🏁 Script executed:
# Read the Button.tsx file to see full context
cat -n src/ui/Button.tsx | sed -n '95,150p'Repository: TanStack/tanstack.com
Length of output: 1636
🏁 Script executed:
# Check if there are type definitions for ButtonProps and ButtonComponent
rg -A 5 "type ButtonComponent|type ButtonProps|interface ButtonProps" src/ui/Button.tsxRepository: TanStack/tanstack.com
Length of output: 493
🏁 Script executed:
# Check React version in package.json
rg "react" package.json -A 1 -B 1 | head -20Repository: TanStack/tanstack.com
Length of output: 764
🏁 Script executed:
# Check if ButtonOwnProps is properly generic
rg -B 5 "type ButtonOwnProps" src/ui/Button.tsxRepository: TanStack/tanstack.com
Length of output: 258
🏁 Script executed:
# Check ButtonInnerProps usage and ButtonOwnProps scope
rg "ButtonOwnProps" src/ui/Button.tsxRepository: TanStack/tanstack.com
Length of output: 366
🏁 Script executed:
# Find React version more explicitly
cat package.json | jq '.dependencies.react, .devDependencies.react' 2>/dev/null || rg '"react":\s*"' package.jsonRepository: TanStack/tanstack.com
Length of output: 79
🏁 Script executed:
# Get full ButtonOwnProps and ButtonProps definitions
rg -B 2 -A 15 "type ButtonOwnProps" src/ui/Button.tsxRepository: TanStack/tanstack.com
Length of output: 630
🏁 Script executed:
# Check if the component is being used polymorphically in the codebase
rg "Button\s+as=" --type tsx --type ts | head -10Repository: TanStack/tanstack.com
Length of output: 93
Update to use React 19's simpler polymorphic pattern without forwardRef.
The component has a polymorphic type contract (ButtonComponent is generic over TElement) but the implementation uses monomorphic forwardRef<HTMLElement, ButtonInnerProps>, creating a type mismatch that requires unsafe casts (line 117: props as ..., line 145: as unknown as ButtonComponent). Since the codebase is on React 19.2.3, ref can be treated as a standard prop, eliminating the need for forwardRef entirely and removing the type coupling issue.
♻️ Suggested direction
-type ButtonInnerProps = ButtonOwnProps & Record<string, unknown>
-export const Button: ButtonComponent = React.forwardRef<
- HTMLElement,
- ButtonInnerProps
->(function Button(props, ref) {
+const ButtonImpl = <TElement extends React.ElementType = 'button'>(
+ props: ButtonProps<TElement> & { ref?: React.ForwardedRef<any> }
+) => {
+ const { ref, as, children, variant = 'primary', color = 'blue', size, rounded, className, ...rest } = props
- const {
- as,
- children,
- variant = 'primary',
- color = 'blue',
- size,
- rounded,
- className,
- ...rest
- } = props as ButtonOwnProps & Record<string, unknown>
const Component = as || 'button'
const resolvedSize = size ?? getDefaultSize(variant)
const resolvedRounded = rounded ?? getDefaultRounded(resolvedSize)
const colorStyles =
variant === 'primary'
? primaryColorStyles[color]
: variant === 'icon'
? iconColorStyles[color]
: ''
return React.createElement(
Component,
{
ref,
className: twMerge(
baseStyles,
variantStyles[variant],
sizeStyles[resolvedSize],
roundedStyles[resolvedRounded],
colorStyles,
className,
),
...rest,
},
children,
)
-}) as unknown as ButtonComponent
+}
+
+export const Button = ButtonImpl as ButtonComponentAlso applies to: 117-117, 145-145
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/ui/Button.tsx` around lines 102 - 107, The implementation uses
React.forwardRef and a monomorphic ref type causing unsafe casts; change Button
to the React 19 polymorphic pattern by removing React.forwardRef and making
Button a generic function component that accepts ref as a normal prop. Update
ButtonInnerProps (and any type alias used by ButtonComponent) to be generic over
TElement so the component signature matches ButtonComponent, accept an optional
ref prop (e.g., ref?: React.Ref<TElement>) in the props, and remove the unsafe
casts currently applied to props and to the exported component (the "props as
..." and "as unknown as ButtonComponent" usages). Ensure the exported Button has
the generic polymorphic type (ButtonComponent) without using forwardRef.
1 participant
Summary
ButtoninReact.forwardRefso refs reach the underlying DOM element when RadixSlotcomposes them onto the child.ButtonPropstoComponentPropsWithRefso consumers'ref={}type-checks.Why
The Copy Page Dropdown on docs/blog pages had stopped appearing. DOM was being rendered, but the Radix popper wrapper was stuck at
transform: translate(0px, -200%)— the off-screen fallback Radix uses whileisPositionedisfalse.Root cause:
CopyPageDropdownis the only<DropdownTrigger>consumer that nests our customButton(every other consumer passes a native DOM element). With a plain function component,{...props}spread intoReact.createElementdidn't carry the ref RadixSlotcomposed viacloneElementonto the trigger.PopperAnchor's local ref stayed null,onAnchorChange(null)was the only call, Floating UI'scomputePositionnever ran (no reference element), andisPositionedstayed false forever.Verified: dropdown now opens with real coordinates (
translate(635px, 490px)in a 1440x900 viewport), right-aligned to the trigger and just below it.Test plan
pnpm test:tsc— cleanpnpm test:lint— 0 errors (10 pre-existing warnings unrelated)/blog/announcing-tanstack-query-v5, clicked the chevron next to "Copy page" — menu renders with all 5 items in the expected positionSummary by CodeRabbit