-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
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
[docs] Bring back *Component
and *Props
codemods and deprecation messages
#44383
Conversation
Netlify deploy previewBundle size report |
*Component
and *Props
deprecation messages*Component
and *Props
codemods and deprecation messages
There are few PR's which were closed due to unsure of direction, do you think they can be reopened? |
Yep, we can re-open them if the contributor are still interested in continuing the effort. |
@@ -87,6 +87,36 @@ You can also manually update your theme as shown in the snippet below: | |||
|
|||
This reduces the API surface and lets you define variants in other slots of the component. | |||
|
|||
## Accordion |
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.
This page could become confusing. If we plan to continue to add these sections here, we should include the version in which the new API was introduced. Otherwise people may run the codemod while installing an older version (e.g. 6.0.0).
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.
I'll look into how to do this
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.
@mnajdova what do you think about this approach:
- For APIs introduced in 6.0.0, we don't add any callout
- For APIs introduced after 6.0.0, we add a warning callout specifying the version in which the API was added
If you agree, this PR doesn't need any change, as these APIs were introduced in 6.0.0. The deprecation messages were omitted while we discussed the direction of the slot pattern, which is why we are bringing them back now.
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.
Agree, ok I see, I thought the APIs were introduced later.
*Component
and *Props
codemods and deprecation messages*Component
and *Props
codemods and deprecation messages
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.
I was worried when seeing all the complexity in the useSlot()
hooks implementation, so I went I checked the performance of the Autocomplete after #41875 (also introduced so many regressions that I got suspicious about it being the right API, is it a foot gun).
The results:
v4.12.4 oliviertassinari@354cd19
noop (baseline):
02.56 ±00.30ms
Grid (html):
66.07 ±07.55ms ⬅️
v5.0.0
? didn't test
v6.0.0-alpha.6, v6.0.0-alpha.7, v6.1.7, all around
noop (baseline):
02.59 ±00.31ms
Grid (html):
135.15 ±07.87ms ⬅️
So no noticeable difference, it looks like pushing further with the slots prop API is the right move (and continue to find ways to simplify that logic). Even with a compound component ejection, this is likely still needed. I guess it's really JSS -> Emotion that was a major performance regression. cc @romgrk for the fun fact.
- PaperComponent={CustomPaperComponent} | ||
- PopperComponent={CustomPopperComponent} | ||
- ListboxComponent={CustomListboxComponent} | ||
+ slots={{ | ||
+ paper: CustomPaperComponent, | ||
+ popper: CustomPopperComponent, | ||
+ }} | ||
+ slotProps={{ | ||
+ listbox: { | ||
+ component: CustomListboxComponent, | ||
+ }, | ||
+ }} |
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.
Correct spacing coding style
- PaperComponent={CustomPaperComponent} | |
- PopperComponent={CustomPopperComponent} | |
- ListboxComponent={CustomListboxComponent} | |
+ slots={{ | |
+ paper: CustomPaperComponent, | |
+ popper: CustomPopperComponent, | |
+ }} | |
+ slotProps={{ | |
+ listbox: { | |
+ component: CustomListboxComponent, | |
+ }, | |
+ }} | |
- PaperComponent={CustomPaperComponent} | |
- PopperComponent={CustomPopperComponent} | |
- ListboxComponent={CustomListboxComponent} | |
+ slots={{ | |
+ paper: CustomPaperComponent, | |
+ popper: CustomPopperComponent, | |
+ }} | |
+ slotProps={{ | |
+ listbox: { | |
+ component: CustomListboxComponent, | |
+ }, | |
+ }} |
- ChipProps={CustomChipProps} | ||
- ListboxProps={CustomListboxProps} | ||
+ slotProps={{ | ||
+ chip: CustomChipProps, | ||
+ listbox: CustomListboxProps, | ||
+ }} |
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.
- ChipProps={CustomChipProps} | |
- ListboxProps={CustomListboxProps} | |
+ slotProps={{ | |
+ chip: CustomChipProps, | |
+ listbox: CustomListboxProps, | |
+ }} | |
- ChipProps={CustomChipProps} | |
- ListboxProps={CustomListboxProps} | |
+ slotProps={{ | |
+ chip: CustomChipProps, | |
+ listbox: CustomListboxProps, | |
+ }} |
In #42466, we reverted these codemods and deprecation messages as we weren't sure of the direction of the slot pattern. Now that we're continuing this effort, we're bringing the deprecation messages back.
This PR also modifies
Autocomplete
'sListboxComponent
deprecation and codemods to point toslotProps.listbox.component
instead ofslots.listbox
, which is the correct migration path.