[TypeScript] Fix usage of ReactElement when it should be ReactNode
#11046
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR improves type safety by replacing overly restrictive ReactElement types with the more appropriate ReactNode type across multiple components. This allows components to accept a broader range of valid React content including strings, numbers, fragments, and null values.
Key changes:
- Updated prop types from
ReactElementtoReactNodefor props that can accept any renderable content (labels, icons, toolbars, actions, etc.) - Removed explicit return type annotations where TypeScript can infer the correct type
- Updated JSDoc comments to reflect the new, more flexible types
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| SavedQueryFilterListItem.tsx | Removed explicit ReactElement return type annotation |
| RemoveSavedQueryIconButton.tsx | Removed explicit ReactElement return type annotation |
| FilterListItem.tsx | Changed label and icon props to accept ReactNode instead of string | ReactElement |
| MenuItemLink.tsx | Changed leftIcon prop to accept ReactNode |
| TranslatableInputs.tsx | Changed selector prop type and JSDoc to ReactNode |
| SelectInput.tsx | Changed emptyText prop to accept ReactNode |
| ReferenceError.tsx | Changed label prop to ReactNode (includes false implicitly) |
| TabbedForm.tsx | Updated JSDoc to reflect ReactNode for toolbar and form tabs |
| SimpleForm.tsx | Updated JSDoc to reflect ReactNode for children and toolbar |
| TranslatableFields.tsx | Changed selector prop type and JSDoc to ReactNode |
| ReferenceOneField.tsx | Changed emptyText to string | ReactNode |
| Tab.tsx | Changed label prop to string | ReactNode |
| Show.tsx | Updated JSDoc to reflect ReactNode for actions |
| EditView.tsx | Changed actions prop to ReactNode | false |
| CreateView.tsx | Changed actions and aside props to ReactNode |
| DeleteButton.tsx | Updated JSDoc to reflect ReactNode for icon |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
| record?: RaRecord; | ||
| resource?: string; | ||
| selector?: ReactElement; | ||
| selector?: ReactNode; |
There was a problem hiding this comment.
Not sure it makes much sense to pass anything other than a React Element here 🤷
There was a problem hiding this comment.
There's no reason to explicitly require an Element when you don't clone it IMO
| import { ReactNode } from 'react'; | ||
| import TextField from '@mui/material/TextField'; | ||
|
|
||
| export const ReferenceError = ({ |
There was a problem hiding this comment.
Is this component used somewhere? 🤔
Looks like it's not. We should probably mark it as deprecated and remove it on the next major version.
| export interface TranslatableInputsProps extends UseTranslatableOptions { | ||
| className?: string; | ||
| selector?: ReactElement; | ||
| selector?: ReactNode; |
Problem
There are still some prop types that accept
ReactElementwhere they should acceptReactNodeSolution
Switch from
ReactElementtoReactNodewhere it makes sense.How to test
ReactNodetoo)