Update types and props for compatibility with React 18 #7150
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes type error in NonContiguousDayRangePicker related to react-day-picker Caption component prop:
```
Type 'FC<CaptionProps>' is not assignable to type '(props: CaptionProps) => Element | null'.
Type 'ReactNode' is not assignable to type 'Element | null'.
Type 'undefined' is not assignable to type 'Element | null'.
```
This reverts commit 77bf286.
This reverts commit 67a68f9.
gluxon
previously approved these changes
Jan 10, 2025
| @@ -24,7 +24,7 @@ import { H4 } from "../html/html"; | |||
|
|
|||
| import { Hotkey, type HotkeyProps } from "./hotkey"; | |||
|
|
|||
| export interface HotkeysProps extends Props { | |||
| export interface HotkeysProps extends Props, React.PropsWithChildren<object> { | |||
There was a problem hiding this comment.
It looks like we're expecting children to be React.ReactElement<HotkeyProps>. Should we use that type over object?
Or was the goal to preserve the same type definition of HotkeyProps and avoid compile breaks on consumers?
There was a problem hiding this comment.
This one is a nitpick, but I've personally been avoiding React.PropsWithChildren<...> since I see the React docs usually write the children prop directly.
https://react.dev/learn/typescript#typing-children
Having the children prop directly in the interface also makes it easier to add a JSDoc on the field.
| @@ -36,7 +36,7 @@ export interface ColumnWidths { | |||
| defaultColumnWidth: number; | |||
| } | |||
|
|
|||
| export interface ColumnHeaderProps extends HeaderProps, ColumnWidths, ColumnIndices { | |||
| export interface ColumnHeaderProps extends React.PropsWithChildren<HeaderProps>, ColumnWidths, ColumnIndices { | |||
There was a problem hiding this comment.
nitpick: Similar comment around potentially writing the children field directly.
policy-bot
bot
dismissed
gluxon’s stale review
Invalidated by push of 21a62fc
Fixing type error:
```
../table/src/cell/editableCell.tsx:198:17 - error TS2322: Type 'Element' is not assignable to type '(ReactElement<HotkeyProps, string | JSXElementConstructor<any>>[] & (boolean | ReactChild | ReactFragment | ReactPortal | null)) | undefined'.
Type 'ReactElement<any, any>' is not assignable to type 'ReactElement<HotkeyProps, string | JSXElementConstructor<any>>[] & ReactPortal'.
Type 'ReactElement<any, any>' is missing the following properties from type 'ReactElement<HotkeyProps, string | JSXElementConstructor<any>>[]': length, pop, push, concat, and 29 more.
198 <Hotkey
~~~~~~~
199 key="edit-cell"
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
...
203 onKeyDown={this.handleEdit}
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
204 />
~~~~~~~~~~~~~~~~~~
../core/lib/esm/components/hotkeys/hotkeys.d.ts:17:5
17 children?: Array<React.ReactElement<HotkeyProps>>;
~~~~~~~~
The expected type comes from property 'children' which is declared here on type 'IntrinsicAttributes & IntrinsicClassAttributes<Hotkeys> & Pick<Readonly<HotkeysProps> & Readonly<...>, "className" | "children"> & InexactPartial<...> & InexactPartial<...>'
../table/src/table.tsx:504:25 - error TS2322: Type '(Element | Element[] | undefined)[]' is not assignable to type '(ReactElement<HotkeyProps, string | JSXElementConstructor<any>>[] & (boolean | ReactChild | ReactFragment | ReactPortal | null)) | undefined'.
Type '(Element | Element[] | undefined)[]' is not assignable to type 'ReactElement<HotkeyProps, string | JSXElementConstructor<any>>[]'.
Type 'Element | Element[] | undefined' is not assignable to type 'ReactElement<HotkeyProps, string | JSXElementConstructor<any>>'.
Type 'undefined' is not assignable to type 'ReactElement<HotkeyProps, string | JSXElementConstructor<any>>'.
504 return <Hotkeys>{hotkeys.filter(element => element !== undefined)}</Hotkeys>;
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../core/lib/esm/components/hotkeys/hotkeys.d.ts:17:5
17 children?: Array<React.ReactElement<HotkeyProps>>;
~~~~~~~~
The expected type comes from property 'children' which is declared here on type 'IntrinsicAttributes & IntrinsicClassAttributes<Hotkeys> & Pick<Readonly<HotkeysProps> & Readonly<...>, "className" | "children"> & InexactPartial<...> & InexactPartial<...>'
```
Switch to React.ReactNode for HotkeysProps children declarationBuild artifact links for this commit: documentation | landing | table | demoThis is an automated comment from the deploy-preview CircleCI job. |
gluxon
approved these changes
Jan 10, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Copied from React 18 Upgrade feature branch #7142
Proposed changes:
Updates types for forward compatibility with React 18.