Skip to content

feat: React 19 migration - 1 #708

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

Merged
rohanchkrabrty merged 2 commits into from
Mar 18, 2026
Merged

feat: React 19 migration - 1 #708

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
09c4e2c
feat: migrate to React 19
rohanchkrabrty Mar 17, 2026
d3e0b34
fix: breaking box test
rohanchkrabrty Mar 17, 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
18 changes: 7 additions & 11 deletions packages/raystack/components/accordion/accordion-content.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,18 @@

import { Accordion as AccordionPrimitive } from '@base-ui/react';
import { cx } from 'class-variance-authority';
import { ElementRef, forwardRef } from 'react';
import styles from './accordion.module.css';

export const AccordionContent = forwardRef<
ElementRef<typeof AccordionPrimitive.Panel>,
AccordionPrimitive.Panel.Props
>(({ className, children, ...props }, ref) => (
<AccordionPrimitive.Panel
ref={ref}
className={styles['accordion-content']}
{...props}
>
export const AccordionContent = ({
className,
children,
...props
}: AccordionPrimitive.Panel.Props) => (
<AccordionPrimitive.Panel className={styles['accordion-content']} {...props}>
<div className={cx(styles['accordion-content-inner'], className)}>
{children}
</div>
</AccordionPrimitive.Panel>
));
);

AccordionContent.displayName = 'Accordion.Content';
13 changes: 6 additions & 7 deletions packages/raystack/components/accordion/accordion-item.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,19 @@

import { Accordion as AccordionPrimitive } from '@base-ui/react';
import { cx } from 'class-variance-authority';
import { ElementRef, forwardRef } from 'react';
import styles from './accordion.module.css';

export const AccordionItem = forwardRef<
ElementRef<typeof AccordionPrimitive.Item>,
AccordionPrimitive.Item.Props
>(({ className, children, ...props }, ref) => (
export const AccordionItem = ({
className,
children,
...props
}: AccordionPrimitive.Item.Props) => (
<AccordionPrimitive.Item
ref={ref}
className={cx(styles['accordion-item'], className)}
{...props}
>
{children}
</AccordionPrimitive.Item>
));
);

AccordionItem.displayName = 'Accordion.Item';
98 changes: 44 additions & 54 deletions packages/raystack/components/accordion/accordion-root.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import { Accordion as AccordionPrimitive } from '@base-ui/react';
import { cx } from 'class-variance-authority';
import { ElementRef, forwardRef } from 'react';
import styles from './accordion.module.css';

type AccordionSingleProps = Omit<
Expand All @@ -27,61 +26,52 @@ type AccordionMultipleProps = Omit<

export type AccordionRootProps = AccordionSingleProps | AccordionMultipleProps;

export const AccordionRoot = forwardRef<
ElementRef<typeof AccordionPrimitive.Root>,
AccordionRootProps
>(
(
{
className,
multiple = false,
value,
defaultValue,
onValueChange,
...rest
},
ref
) => {
// Convert value to array format for Base UI
const baseValue = value
? Array.isArray(value)
? value
: [value]
: undefined;
export const AccordionRoot = ({
className,
multiple = false,
value,
defaultValue,
onValueChange,
...rest
}: AccordionRootProps) => {
// Convert value to array format for Base UI
const baseValue = value
? Array.isArray(value)
? value
: [value]
: undefined;

const baseDefaultValue = defaultValue
? Array.isArray(defaultValue)
? defaultValue
: [defaultValue]
: undefined;
const baseDefaultValue = defaultValue
? Array.isArray(defaultValue)
? defaultValue
: [defaultValue]
: undefined;
Comment on lines +38 to +48
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 17, 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:

For @base-ui/react Accordion, is an empty string ("") a valid item value? Please link official docs/API reference.

💡 Result:

Yes. In @base-ui/react Accordion, Accordion.Item’s value prop is typed as any and is described as “a unique value that identifies this accordion item.” An empty string ("") is still a (string) value, so it’s valid as long as it’s unique among items. If you omit value entirely, Base UI will generate an ID instead. [1]

Official docs / API reference:

Related note (initially-open behavior requires an explicit value):

Sources: [1] https://base-ui.com/react/components/accordion, [2] https://base-ui.com/react/overview/releases/v1-0-0-beta-4

🏁 Script executed:

find . -name "accordion-root.tsx" -type f | head -5

Repository: raystack/apsara

Length of output: 118

🏁 Script executed:

cat -n ./packages/raystack/components/accordion/accordion-root.tsx | sed -n '30,60p'

Repository: raystack/apsara

Length of output: 1032

🏁 Script executed:

cat -n ./packages/raystack/components/accordion/accordion-root.tsx | sed -n '1,20p'

Repository: raystack/apsara

Length of output: 747

🏁 Script executed:

cat -n ./packages/raystack/components/accordion/accordion-root.tsx | sed -n '20,36p'

Repository: raystack/apsara

Length of output: 524

Use explicit !== undefined checks to preserve falsy values like empty strings.

The truthy checks on lines 38 and 44 will incorrectly convert empty strings ("") to undefined. Since @base-ui/react Accordion accepts empty strings as valid item values, this causes silent data loss. Replace value ? with value !== undefined and defaultValue ? with defaultValue !== undefined.

Proposed patch 
-  const baseValue = value
+  const baseValue = value !== undefined
     ? Array.isArray(value)
       ? value
       : [value]
     : undefined;

-  const baseDefaultValue = defaultValue
+  const baseDefaultValue = defaultValue !== undefined
     ? Array.isArray(defaultValue)
       ? defaultValue
       : [defaultValue]
     : undefined;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const baseValue = value
? Array.isArray(value)
? value
: [value]
: undefined;
const baseDefaultValue = defaultValue
? Array.isArray(defaultValue)
? defaultValue
: [defaultValue]
: undefined;
const baseDefaultValue = defaultValue
? Array.isArray(defaultValue)
? defaultValue
: [defaultValue]
: undefined;
const baseValue = value !== undefined
? Array.isArray(value)
? value
: [value]
: undefined;
const baseDefaultValue = defaultValue !== undefined
? Array.isArray(defaultValue)
? defaultValue
: [defaultValue]
: undefined;
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/raystack/components/accordion/accordion-root.tsx` around lines 38 -
48, The computations for baseValue and baseDefaultValue incorrectly use truthy
checks which convert falsy-but-valid values (e.g. empty string) to undefined;
update the conditional expressions in the baseValue and baseDefaultValue
assignments to use explicit !== undefined checks (use value !== undefined and
defaultValue !== undefined) so that Array.isArray wrapping still occurs for
non-array values while preserving empty strings and other falsy-but-valid
values.


const handleValueChange = (
newValue: string[],
eventDetails: AccordionPrimitive.Root.ChangeEventDetails
) => {
if (onValueChange) {
if (multiple) {
(onValueChange as (value: string[]) => void)(newValue);
} else {
(onValueChange as (value: string | undefined) => void)(
newValue[0] || undefined
);
}
const handleValueChange = (
newValue: string[],
eventDetails: AccordionPrimitive.Root.ChangeEventDetails
) => {
if (onValueChange) {
if (multiple) {
(onValueChange as (value: string[]) => void)(newValue);
} else {
(onValueChange as (value: string | undefined) => void)(
newValue[0] || undefined
);
}
};
}
};

return (
<AccordionPrimitive.Root
ref={ref}
className={cx(styles.accordion, className)}
multiple={multiple}
value={baseValue}
defaultValue={baseDefaultValue}
onValueChange={handleValueChange}
{...rest}
/>
);
}
);
return (
<AccordionPrimitive.Root
className={cx(styles.accordion, className)}
multiple={multiple}
value={baseValue}
defaultValue={baseDefaultValue}
onValueChange={handleValueChange}
{...rest}
/>
);
};

AccordionRoot.displayName = 'Accordion.Root';
AccordionRoot.displayName = 'Accordion';
13 changes: 6 additions & 7 deletions packages/raystack/components/accordion/accordion-trigger.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,22 @@
import { Accordion as AccordionPrimitive } from '@base-ui/react';
import { ChevronDownIcon } from '@radix-ui/react-icons';
import { cx } from 'class-variance-authority';
import { ElementRef, forwardRef } from 'react';
import styles from './accordion.module.css';

export const AccordionTrigger = forwardRef<
ElementRef<typeof AccordionPrimitive.Trigger>,
AccordionPrimitive.Trigger.Props
>(({ className, children, ...props }, ref) => (
export const AccordionTrigger = ({
className,
children,
...props
}: AccordionPrimitive.Trigger.Props) => (
<AccordionPrimitive.Header className={styles['accordion-header']}>
<AccordionPrimitive.Trigger
ref={ref}
className={cx(styles['accordion-trigger'], className)}
{...props}
>
{children}
<ChevronDownIcon className={styles['accordion-icon']} />
</AccordionPrimitive.Trigger>
</AccordionPrimitive.Header>
));
);

AccordionTrigger.displayName = 'Accordion.Trigger';
80 changes: 35 additions & 45 deletions packages/raystack/components/alert-dialog/alert-dialog-content.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import { AlertDialog as AlertDialogPrimitive } from '@base-ui/react';
import { cx } from 'class-variance-authority';
import { type ElementRef, forwardRef } from 'react';
import styles from '../dialog/dialog.module.css';
import { CloseButton } from './alert-dialog-misc';

Expand All @@ -18,51 +17,42 @@ export interface AlertDialogContentProps
showNestedAnimation?: boolean;
}

export const AlertDialogContent = forwardRef<
ElementRef<typeof AlertDialogPrimitive.Popup>,
AlertDialogContentProps
>(
(
{
className,
children,
showCloseButton = true,
overlay,
width,
style,
showNestedAnimation = true,
...props
},
ref
) => {
return (
<AlertDialogPrimitive.Portal>
<AlertDialogPrimitive.Backdrop
{...overlay}
export const AlertDialogContent = ({
className,
children,
showCloseButton = true,
overlay,
width,
style,
showNestedAnimation = true,
...props
}: AlertDialogContentProps) => {
return (
<AlertDialogPrimitive.Portal>
<AlertDialogPrimitive.Backdrop
{...overlay}
className={cx(
styles.dialogOverlay,
overlay?.blur && styles.overlayBlur,
overlay?.className
)}
Comment on lines +32 to +38
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 17, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Strip overlay.blur before forwarding backdrop props.

Line 33 spreads the full overlay object into AlertDialogPrimitive.Backdrop, so the wrapper-only blur flag is forwarded downstream as if it were a backdrop prop. Destructure blur (and className) first, then spread only the remaining backdrop props.

Proposed fix 
 export const AlertDialogContent = ({
   className,
   children,
   showCloseButton = true,
   overlay,
   width,
   style,
   showNestedAnimation = true,
   ...props
 }: AlertDialogContentProps) => {
+  const {
+    blur: overlayBlur,
+    className: overlayClassName,
+    ...overlayProps
+  } = overlay ?? {};
+
   return (
     <AlertDialogPrimitive.Portal>
       <AlertDialogPrimitive.Backdrop
-        {...overlay}
+        {...overlayProps}
         className={cx(
           styles.dialogOverlay,
-          overlay?.blur && styles.overlayBlur,
-          overlay?.className
+          overlayBlur && styles.overlayBlur,
+          overlayClassName
         )}
       />
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/raystack/components/alert-dialog/alert-dialog-content.tsx` around
lines 32 - 38, The Backdrop is receiving wrapper-only flags because the
component spreads the entire overlay object into AlertDialogPrimitive.Backdrop;
update the render to destructure overlay (const { blur, className,
...backdropProps } = overlay) and pass only backdropProps to
AlertDialogPrimitive.Backdrop while using blur and className for composing
className (e.g., styles.dialogOverlay, overlayBlur, overlay.className) so blur
is not forwarded as a DOM/backdrop prop.

/>
<AlertDialogPrimitive.Viewport className={styles.viewport}>
<AlertDialogPrimitive.Popup
className={cx(
styles.dialogOverlay,
overlay?.blur && styles.overlayBlur,
overlay?.className
styles.dialogContent,
showNestedAnimation && styles.showNestedAnimation,
className
)}
/>
<AlertDialogPrimitive.Viewport className={styles.viewport}>
<AlertDialogPrimitive.Popup
ref={ref}
className={cx(
styles.dialogContent,
showNestedAnimation && styles.showNestedAnimation,
className
)}
style={{ width, ...style }}
{...props}
>
{children}
{showCloseButton && <CloseButton className={styles.closeButton} />}
</AlertDialogPrimitive.Popup>
</AlertDialogPrimitive.Viewport>
</AlertDialogPrimitive.Portal>
);
}
);
style={{ width, ...style }}
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 17, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Let the explicit width prop win over style.width.

Line 47 currently uses { width, ...style }, so style.width overrides the dedicated width prop whenever both are set. That makes the component-specific API easy to bypass accidentally.

Proposed fix 
-          style={{ width, ...style }}
+          style={width === undefined ? style : { ...style, width }}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
style={{ width, ...style }}
style={width === undefined ? style : { ...style, width }}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/raystack/components/alert-dialog/alert-dialog-content.tsx` at line
47, In AlertDialogContent where the inline style is built using { width,
...style }, change the merge order so the explicit width prop wins over
style.width by spreading style first and then applying width (i.e., use ...style
then width); update the JSX where style={{ width, ...style }} or style={ {
width, ...style } } is used so that style becomes style={{ ...style, width }}
and ensure this logic is applied inside the AlertDialogContent component
rendering path.

{...props}
>
{children}
{showCloseButton && <CloseButton className={styles.closeButton} />}
</AlertDialogPrimitive.Popup>
</AlertDialogPrimitive.Viewport>
</AlertDialogPrimitive.Portal>
);
};

AlertDialogContent.displayName = 'AlertDialog.Content';
Loading
Loading