-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
refactor(React): initialize default props with serially stable values #14720
refactor(React): initialize default props with serially stable values #14720
Conversation
✅ Deploy Preview for v11-carbon-react ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for v11-carbon-react ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
25c49dd
to
51999af
Compare
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.
Lots to parse in the diff, but this looks good from my view.
Would you mind updating the checklist on #13424 with the ones here that you've migrated over to default function params? If the list ends up being complete we can close that one as well |
Updated, the only one missing is |
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.
Tested on my end and looks good!
@tay1orjones @andreancardona I've updated this PR to include |
2ebd2cb
Closes #10777
Closes #13424
Removes all instances of
defaultProps
throughout the React codebase and reactors them to serially stable default function parameters.Changelog
New
internal/noopFn
export that is used anywhere we were setting a default value on a change handler i.eonChange: () => {}
becomesonChange = noopFn
Changed
Removed
isRequired
props that were not required in other places, or would throw unnecessaryconsole
warnings if the prop was not explicitly provided (even though we set a default).Testing / Reviewing
isRequired
props are okayDataTable
andListbox
components, as these were some of the more heavily modified components