Skip to content

fix(Breadcrumb)!: BREAKING CHANGE use composition and remove cloneElement #695

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
paanSinghCoder wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fix(Breadcrumb)!: BREAKING CHANGE use composition and remove cloneElement #695

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3c406e7
fix: use composition and remove cloneElement
paanSinghCoder Mar 12, 2026
1fc3160
Merge branch 'main' into fix/breadcrumb-composition
paanSinghCoder Mar 20, 2026
db4db91
Merge branch 'main' into fix/breadcrumb-composition
paanSinghCoder Mar 20, 2026
4481c5e
feat: replace as with useRender.
paanSinghCoder Mar 20, 2026
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
2 changes: 1 addition & 1 deletion apps/www/src/components/playground/breadcrumb-examples.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export function BreadcrumbExamples() {
Products
</Breadcrumb.Item>
<Breadcrumb.Separator />
<Breadcrumb.Item href='/products/shoes' as={<a />} current>
<Breadcrumb.Item href='/products/shoes' render={<a />} current>
Shoes
</Breadcrumb.Item>
</Breadcrumb>
Expand Down
4 changes: 2 additions & 2 deletions apps/www/src/content/docs/components/breadcrumb/demo.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,9 @@ export const asDemo = {
type: 'code',
code: `
<Breadcrumb>
<Breadcrumb.Item href="/home" as={<NextLink href="/" />}>Home</Breadcrumb.Item>
<Breadcrumb.Item href="/" render={<NextLink />}>Home</Breadcrumb.Item>
<Breadcrumb.Separator/>
<Breadcrumb.Item href="/playground" as={<NextLink />}>Playground</Breadcrumb.Item>
<Breadcrumb.Item href="/playground" render={<NextLink />}>Playground</Breadcrumb.Item>
<Breadcrumb.Separator/>
<Breadcrumb.Item href="/docs" current>Docs</Breadcrumb.Item>
</Breadcrumb>`
Expand Down
13 changes: 8 additions & 5 deletions apps/www/src/content/docs/components/breadcrumb/index.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ Groups all parts of the breadcrumb navigation.

### Item

Renders an individual breadcrumb link.
Renders an individual breadcrumb link. Ref is forwarded to the rendered element when using `render` (not when using `dropdownItems`).

<auto-type-table path="./props.ts" name="BreadcrumbItem" />

Expand Down Expand Up @@ -88,15 +88,18 @@ Breadcrumb items can include icons either alongside text or as standalone elemen

Breadcrumb items can include dropdown menus for additional navigation options. Specify the dropdown items using the `dropdownItems` prop.

**Note:** When `dropdownItems` is provided, the `as` and `href` props are ignored.
**Note:** When `dropdownItems` is provided, the `render` and `href` props are ignored.

<Demo data={dropdownDemo} />

### As
### Render

Use the `as` prop to render the breadcrumb item as a custom component. By default, breadcrumb items are rendered as `a` tags.
Use the `render` prop to render the breadcrumb item as a custom component. Pass a JSX element (e.g. `render={<NextLink />}`) and we will replace the default `<a>` while merging props. By default, items render as `a` tags.

When a custom component is provided, the props are merged, with the custom component's props taking precedence over the breadcrumb item's props.
```tsx
// Correct: set href on the item (it will be forwarded into the rendered element)
<Breadcrumb.Item href="/" render={<NextLink />}>Home</Breadcrumb.Item>
```
Comment on lines +95 to +102
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 20, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Base UI render prop prop merging behavior

💡 Result:

Base UI's render prop uses intelligent prop merging via the mergeProps utility to combine component props with user-provided render props. Props are merged left-to-right (rightmost wins for most keys), with special handling for className (concatenated right-to-left), style (deep merged, rightmost overwrites), event handlers (merged and executed right-to-left), and ref (rightmost ref kept). This ensures custom components receive Base UI's behavioral props (like event handlers, data attributes) while preserving user styles and handlers. In render prop usage: - Element form: <Component render={} /> - mergeProps automatically combines props. - Function form: render={(props) => <Custom {...props} />} or render={(props, state) => ...} - use mergeProps manually for merging, e.g., <Custom {...mergeProps(internalProps, userProps)} />. Call event.preventBaseUIHandler to prevent Base UI's internal logic. The useRender hook implements render prop logic, merging props before spreading on the rendered element.

Citations:

🌐 Web query:

Next.js Link component href required prop

💡 Result:

Yes, the href prop is required for the Next.js Link component in both App Router and Pages Router.

Citations:

🏁 Script executed:

# Check the actual file and context around the example
fd -p "breadcrumb/index.mdx" apps/www/src/content/docs/components/

Repository: raystack/apsara

Length of output: 116

🏁 Script executed:

# Read the actual markdown file to see the context and full example
cat -n apps/www/src/content/docs/components/breadcrumb/index.mdx | sed -n '90,110p'

Repository: raystack/apsara

Length of output: 981

🏁 Script executed:

# Check if there are type definitions or examples elsewhere for breadcrumb
fd -p "breadcrumb" apps/www/src --type f | head -20

Repository: raystack/apsara

Length of output: 288

🏁 Script executed:

# Check the breadcrumb component implementation
cat apps/www/src/components/playground/breadcrumb-examples.tsx

Repository: raystack/apsara

Length of output: 1127

🏁 Script executed:

# Check the demo file to see if there are live examples
cat apps/www/src/content/docs/components/breadcrumb/demo.ts

Repository: raystack/apsara

Length of output: 4341

🏁 Script executed:

# Check the actual Breadcrumb.Item component implementation to understand prop forwarding
fd -p "breadcrumb" apps/www --type f -e "ts" -e "tsx" | xargs grep -l "Breadcrumb.Item\|render" | head -5

Repository: raystack/apsara

Length of output: 230

🏁 Script executed:

# Find the main component source files
fd -p "breadcrumb" --type f -e "ts" -e "tsx" | grep -v "node_modules\|dist" | head -20

Repository: raystack/apsara

Length of output: 585

🏁 Script executed:

# Check the Breadcrumb.Item component implementation
cat packages/raystack/components/breadcrumb/breadcrumb-item.tsx

Repository: raystack/apsara

Length of output: 3017

🏁 Script executed:

# Also check the main breadcrumb file
cat packages/raystack/components/breadcrumb/breadcrumb.tsx

Repository: raystack/apsara

Length of output: 391

🏁 Script executed:

# Check the types used by useRender to understand render prop expectations
rg "useRender" packages/raystack/components/breadcrumb/ -A 5

Repository: raystack/apsara

Length of output: 1712

🏁 Script executed:

# Check if there are any type definitions or interfaces for the render prop
rg "ComponentProps" packages/raystack/components/breadcrumb/ -B 2 -A 2

Repository: raystack/apsara

Length of output: 2677

🏁 Script executed:

# Check if there's any tsconfig or type relaxations that might allow this
fd -p "tsconfig" --type f

Repository: raystack/apsara

Length of output: 233

🏁 Script executed:

# Look for actual uses of NextLink in the codebase to see how they handle it
rg "NextLink" --type tsx --type ts -A 2 -B 2 | head -50

Repository: raystack/apsara

Length of output: 86

🏁 Script executed:

# Look for actual uses of NextLink in the codebase
rg "NextLink" -A 2 -B 2 | head -80

Repository: raystack/apsara

Length of output: 2773

🏁 Script executed:

# Check the tsconfig to see if there's any type relaxation
cat apps/www/tsconfig.json

Repository: raystack/apsara

Length of output: 720

🏁 Script executed:

# Check the props.ts file to see the exact type definition for render
cat apps/www/src/content/docs/components/breadcrumb/props.ts

Repository: raystack/apsara

Length of output: 1689

🏁 Script executed:

# Try to understand how the render prop type is defined - check if it allows optional props
rg "render\?" packages/raystack/components/breadcrumb/ -A 1 -B 1

Repository: raystack/apsara

Length of output: 41

🏁 Script executed:

# Check Next.js Link type definition to confirm href is required
rg "export.*Link" node_modules/next/link.d.ts -A 10 | head -30

Repository: raystack/apsara

Length of output: 126

🏁 Script executed:

# Check if there are any tests showing how render prop is used
cat packages/raystack/components/breadcrumb/__tests__/breadcrumb.test.tsx | head -100

Repository: raystack/apsara

Length of output: 2916

Use the function form of the render prop for components with required props.

The example render={<NextLink />} will fail TypeScript type-checking because Next.js Link requires the href prop. Base UI's prop merging happens at runtime, after JSX evaluation. Use the callback form instead: render={(props) => <NextLink {...props} />}. If you want to keep the element form, explicitly provide href: render={<NextLink href="/" />}.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@apps/www/src/content/docs/components/breadcrumb/index.mdx` around lines 95 -
102, The example uses the element form render={<NextLink />} which fails
TypeScript because Next.js Link requires href; update the docs and example for
Breadcrumb.Item (component Breadcrumb.Item, prop render) to use the function
form so props are forwarded at render time (render={(props) => <NextLink
{...props} />}) or else show the element form with an explicit href
(render={<NextLink href="/" />}); change the example text and code snippet
accordingly to recommend the callback form for components with required props
and include NextLink/ Breadcrumb.Item references.


<Demo data={asDemo} />

Expand Down
11 changes: 5 additions & 6 deletions apps/www/src/content/docs/components/breadcrumb/props.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export interface BreadcrumbItem {
/**
* Optional array of dropdown items
*
* When `dropdownItems` is provided, the `as` and `href` props are ignored.
* When `dropdownItems` is provided, the `render` and `href` props are ignored.
*/
dropdownItems?: {
/** Optional stable key for list reconciliation. Falls back to index if omitted. */
Expand All @@ -31,13 +31,12 @@ export interface BreadcrumbItem {
}[];

/**
* Custom element used to render the Item.
* Render prop for polymorphism (Base UI `useRender`).
* Pass a JSX element to replace the default rendered `<a>`.
*
* All props are merged, with the custom component's props taking precedence over the breadcrumb item's props.
*
* @default "<a />"
* Example: `render={<NextLink />}`
*/
as?: ReactElement;
render?: ReactElement;
}

export interface BreadcrumbProps {
Expand Down
47 changes: 38 additions & 9 deletions packages/raystack/components/breadcrumb/__tests__/breadcrumb.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { fireEvent, render, screen } from '@testing-library/react';
import React, { forwardRef } from 'react';
import { describe, expect, it, vi } from 'vitest';
import { Breadcrumb } from '../breadcrumb';
import styles from '../breadcrumb.module.css';
Expand Down Expand Up @@ -135,24 +136,32 @@ describe('Breadcrumb', () => {
</Breadcrumb>
);

const link = container.querySelector('a');
expect(link).toHaveClass(styles['breadcrumb-link-active']);
// Current page renders as <span> (non-link), not <a>
const currentEl = container.querySelector(
`span.${styles['breadcrumb-link-active']}`
);
expect(currentEl).toBeInTheDocument();
expect(currentEl).toHaveAttribute('aria-current', 'page');
});

it('renders with custom element using as prop', () => {
const CustomLink = ({ children, ...props }: any) => (
<button {...props}>{children}</button>
it('renders with custom element using render prop', () => {
const CustomLink: React.ComponentType<
React.AnchorHTMLAttributes<HTMLAnchorElement>
> = ({ children, ...props }) => (
<a data-custom-link {...props}>
{children}
</a>
);

const { container } = render(
<Breadcrumb>
<Breadcrumb.Item as={<CustomLink />}>Custom</Breadcrumb.Item>
<Breadcrumb.Item render={<CustomLink />}>Custom</Breadcrumb.Item>
</Breadcrumb>
);

const button = container.querySelector('button');
expect(button).toBeInTheDocument();
expect(button).toHaveClass(styles['breadcrumb-link']);
const link = container.querySelector('[data-custom-link]');
expect(link).toBeInTheDocument();
expect(link).toHaveClass(styles['breadcrumb-link']);
expect(screen.getByText('Custom')).toBeInTheDocument();
});

Expand All @@ -166,6 +175,26 @@ describe('Breadcrumb', () => {
expect(ref).toHaveBeenCalled();
});

it('forwards ref when using render prop (component)', () => {
const CustomLink = forwardRef<
HTMLAnchorElement,
React.AnchorHTMLAttributes<HTMLAnchorElement>
>(({ children, ...props }, ref) => (
<a ref={ref} data-custom-link {...props}>
{children}
</a>
));
const ref = vi.fn();
render(
<Breadcrumb>
<Breadcrumb.Item ref={ref} render={<CustomLink />}>
Custom
</Breadcrumb.Item>
</Breadcrumb>
);
expect(ref).toHaveBeenCalled();
});

it('applies custom className', () => {
const { container } = render(
<Breadcrumb>
Expand Down
86 changes: 54 additions & 32 deletions packages/raystack/components/breadcrumb/breadcrumb-item.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
'use client';

import { mergeProps, useRender } from '@base-ui/react';
import { ChevronDownIcon } from '@radix-ui/react-icons';
import { cx } from 'class-variance-authority';
import React, {
ComponentProps,
cloneElement,
ReactElement,
ReactNode
} from 'react';
import React, { ReactNode } from 'react';
import { Menu } from '../menu';
import styles from './breadcrumb.module.css';

Expand All @@ -17,16 +13,15 @@ export interface BreadcrumbDropdownItem {
onClick?: React.MouseEventHandler<HTMLElement>;
}

export interface BreadcrumbItemProps extends ComponentProps<'a'> {
export interface BreadcrumbItemProps extends useRender.ComponentProps<'a'> {
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 20, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n packages/raystack/components/breadcrumb/breadcrumb-item.tsx

Repository: raystack/apsara

Length of output: 3836

Fix the ref type contract across all render paths in BreadcrumbItem.

BreadcrumbItemProps extends useRender.ComponentProps<'a'>, declaring an anchor-typed ref. However, the dropdown branch at line 69 casts the ref to HTMLButtonElement (used in Menu.Trigger), and the current branch at line 99 casts to HTMLSpanElement (used in <span>). Consumers can pass an anchor-oriented ref and receive a different element type at runtime.

Either split the component variants to have distinct ref types for each case, or remove ref forwarding from the non-anchor branches.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/raystack/components/breadcrumb/breadcrumb-item.tsx` at line 16,
BreadcrumbItemProps currently extends useRender.ComponentProps<'a'> (anchor ref)
but the component sometimes renders Menu.Trigger (button) or a plain <span>,
causing ref type mismatches; fix by making the props/ref contract match render
variants: either (A) split BreadcrumbItem into distinct variants with a
discriminated prop (e.g. kind: 'link' | 'dropdown' | 'current') and define
BreadcrumbItemProps as a union where the 'link' variant extends
useRender.ComponentProps<'a'> (ref: HTMLAnchorElement) while 'dropdown' extends
React.ComponentPropsWithRef<'button'> (ref: HTMLButtonElement) and 'current'
omits forwarded ref or uses React.Ref<HTMLSpanElement); or (B) remove ref
forwarding from non-anchor branches by only forwarding the ref to the anchor
path in the BreadcrumbItem forwardRef implementation and use internal refs for
Menu.Trigger and the <span>, updating usages of Menu.Trigger and the current
span render path accordingly; adjust type declarations and the BreadcrumbItem
forwardRef signature to reflect the chosen approach so consumers never receive
an anchor ref for a non-anchor element.

leadingIcon?: ReactNode;
current?: boolean;
dropdownItems?: BreadcrumbDropdownItem[];
as?: ReactElement;
}

export const BreadcrumbItem = ({
ref,
as,
render,
children,
className,
leadingIcon,
Expand All @@ -35,24 +30,49 @@ export const BreadcrumbItem = ({
dropdownItems,
...props
}: BreadcrumbItemProps) => {
const renderedElement = as ?? <a ref={ref} />;
const label = (
const label = leadingIcon ? (
<>
{leadingIcon && (
<span className={styles['breadcrumb-icon']}>{leadingIcon}</span>
)}
{children && <span>{children}</span>}
<span className={styles['breadcrumb-icon']}>{leadingIcon}</span>
{children != null && <span>{children}</span>}
</>
) : (
children
);

const {
id,
title,
'aria-label': ariaLabel,
'aria-labelledby': ariaLabelledby,
'aria-describedby': ariaDescribedby
} = props;
Comment on lines +42 to +48
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 20, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Dropdown trigger props are being dropped.

This whitelist only forwards id, title, and a few aria-* fields into Menu.Trigger, so data-*, tabIndex, and consumer handlers like onClick never reach the trigger when dropdownItems is set. That regresses the normal prop-forwarding contract and makes the dropdown path behave differently from the link path.

Also applies to: 68-75

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/raystack/components/breadcrumb/breadcrumb-item.tsx` around lines 42
- 48, The destructuring in BreadcrumbItem currently pulls only id, title and a
few aria-* fields and drops all other props (so data-*, tabIndex, onClick, etc.
never reach Menu.Trigger); change the destructuring to capture the rest (e.g.
add ...rest to const { id, title, 'aria-label': ariaLabel, 'aria-labelledby':
ariaLabelledby, 'aria-describedby': ariaDescribedby, ...rest } = props) and
spread {...rest} onto Menu.Trigger (and the analogous element in the other code
path mentioned at lines 68-75) so data-*, tabIndex and consumer handlers are
forwarded unchanged to the trigger element.


const linkElement = useRender({
defaultTagName: 'a',
ref,
render,
props: mergeProps<'a'>(
{
className: cx(styles['breadcrumb-link']),
href,
children: label
},
props
)
});

if (dropdownItems) {
return (
<li className={cx(styles['breadcrumb-item'], className)}>
<Menu>
<Menu.Trigger
ref={ref as React.Ref<HTMLButtonElement>}
className={styles['breadcrumb-dropdown-trigger']}
{...(props as React.ButtonHTMLAttributes<HTMLButtonElement>)}
id={id}
title={title}
aria-label={ariaLabel}
aria-labelledby={ariaLabelledby}
aria-describedby={ariaDescribedby}
>
{label}
<ChevronDownIcon className={styles['breadcrumb-dropdown-icon']} />
Expand All @@ -72,23 +92,25 @@ export const BreadcrumbItem = ({
</li>
);
}
return (
<li className={cx(styles['breadcrumb-item'], className)}>
{cloneElement(
renderedElement,
{
className: cx(
if (current) {
return (
<li className={cx(styles['breadcrumb-item'], className)}>
<span
ref={ref as React.RefObject<HTMLSpanElement>}
className={cx(
styles['breadcrumb-link'],
current && styles['breadcrumb-link-active']
),
href,
...props,
...renderedElement.props,
ref
},
label
)}
</li>
styles['breadcrumb-link-active']
)}
aria-current='page'
{...props}
Comment on lines +98 to +105
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 20, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

current can be stripped of its page semantics.

Because {...props} is spread after aria-current='page', callers can override/remove the current-page state and also reattach interactive props like onClick or tabIndex to what is supposed to be a non-link page indicator. This branch should whitelist neutral attributes and set aria-current last.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/raystack/components/breadcrumb/breadcrumb-item.tsx` around lines 98
- 105, The span currently applies aria-current='page' and then spreads
{...props}, which allows callers to override/remove aria-current or reattach
interactive handlers; fix by filtering incoming props (the `props` object passed
to the span) to a small whitelist of neutral attributes (e.g., className, id,
data-* and non-interactive aria-* except aria-current) or explicitly omit
interactive props like onClick, tabIndex, role, href, and then spread the
filteredProps before setting aria-current; update the JSX in breadcrumb-item.tsx
(the span with ref, cx and styles['breadcrumb-link'] /
styles['breadcrumb-link-active']) so that filteredProps are spread first and
aria-current='page' is set last to guarantee the page semantics can't be
overridden.

>
{label}
</span>
</li>
);
}
return (
<li className={cx(styles['breadcrumb-item'], className)}>{linkElement}</li>
);
};

Expand Down
9 changes: 8 additions & 1 deletion packages/raystack/components/breadcrumb/breadcrumb-root.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,18 @@ export interface BreadcrumbProps
export const BreadcrumbRoot = ({
className,
children,
ref,
size = 'medium',
'aria-label': ariaLabel,
...props
}: BreadcrumbProps) => {
return (
<nav className={breadcrumbVariants({ size, className })} {...props}>
<nav
className={breadcrumbVariants({ size, className })}
ref={ref}
aria-label={ariaLabel ?? 'Breadcrumb'}
{...props}
>
<ol className={styles['breadcrumb-list']}>{children}</ol>
</nav>
);
Expand Down
Loading