-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
fix: add React 19 compatibility for getWidget (defaultOptions + remov… #4912
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
1dea32a
ff747d3
11ce7f1
d19cadb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| # 7.x Upgrade Guide | ||
|
|
||
| ## Breaking changes | ||
|
|
||
| ### react-is.isForwardRef() removed | ||
|
|
||
| The usage of `react-is.isForwardRef()` was removed from `getWidget.tsx` because React 19 no longer supports this function. ForwardRef components are now detected using `ReactIs.isValidElementType()`, which provides a robust check for all valid React component types. | ||
|
|
||
| This change should be transparent to most users, as the widget detection logic now properly handles: | ||
|
|
||
| - Regular function components | ||
| - Memoized components (`React.memo`) | ||
| - ForwardRef components (`React.forwardRef`) | ||
| - Other exotic React components | ||
|
|
||
| ## New deprecations | ||
|
|
||
| None in this release. | ||
|
|
||
| ## Removed deprecations | ||
|
|
||
| None in this release. |
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you actually change anything in this file or is it just whitespace. If just whitespace, please revert |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,7 +1,7 @@ | ||||||||||||||||||||
| import { forwardRef, memo, ForwardedRef } from 'react'; | ||||||||||||||||||||
| import { render } from '@testing-library/react'; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| import { FieldPathId, Registry, RJSFSchema, WidgetProps, getWidget, Widget } from '../src'; | ||||||||||||||||||||
| import { FieldPathId, Registry, RJSFSchema, WidgetProps, getWidget } from '../src'; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const subschema: RJSFSchema = { | ||||||||||||||||||||
| type: 'boolean', | ||||||||||||||||||||
|
|
@@ -30,7 +30,7 @@ const schema: RJSFSchema = { | |||||||||||||||||||
| }; | ||||||||||||||||||||
| const schemaStr = JSON.stringify(schema); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const TestRefWidget: Widget = forwardRef<HTMLSpanElement, Partial<WidgetProps>>(function TestRefWidget( | ||||||||||||||||||||
| const TestRefWidget = forwardRef<HTMLSpanElement, Partial<WidgetProps>>(function TestRefWidget( | ||||||||||||||||||||
| props: Partial<WidgetProps>, | ||||||||||||||||||||
| ref: ForwardedRef<HTMLSpanElement>, | ||||||||||||||||||||
| ) { | ||||||||||||||||||||
|
|
@@ -42,27 +42,20 @@ const TestRefWidget: Widget = forwardRef<HTMLSpanElement, Partial<WidgetProps>>( | |||||||||||||||||||
| ); | ||||||||||||||||||||
| }); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| TestRefWidget.defaultProps = { | ||||||||||||||||||||
| options: { id: 'test-id' }, | ||||||||||||||||||||
| }; | ||||||||||||||||||||
| TestRefWidget.defaultProps = { options: { id: 'test-id' } }; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| function TestWidget(props: WidgetProps) { | ||||||||||||||||||||
|
Comment on lines
+45
to
47
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We would then also drop testing with
Suggested change
|
||||||||||||||||||||
| const { options } = props; | ||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Convert defaultProps into a destructure default assignment
Suggested change
|
||||||||||||||||||||
| return <div {...options}>test</div>; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| TestWidget.defaultProps = { | ||||||||||||||||||||
| id: 'foo', | ||||||||||||||||||||
| }; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| function TestWidgetDefaults(props: WidgetProps) { | ||||||||||||||||||||
| const { options } = props; | ||||||||||||||||||||
| return <div {...options}>test</div>; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
Comment on lines
52
to
55
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The replacement for
Suggested change
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| TestWidgetDefaults.defaultProps = { | ||||||||||||||||||||
| options: { color: 'yellow' }, | ||||||||||||||||||||
| }; | ||||||||||||||||||||
| TestWidgetDefaults.defaultProps = { options: { color: 'yellow' } }; | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| const widgetProps: WidgetProps = { | ||||||||||||||||||||
|
Comment on lines
+57
to
60
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||
| id: '', | ||||||||||||||||||||
|
|
||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, once we drop
defaultPropssupport in v7 this whole conditional and themergeWidgetOptions()function will no longer be necessary. Which means that theMergedWidgetfeature will just go away.