[material-ui][FormControlLabel] deprecate componentsProps
#41767
Conversation
Netlify deploy previewFormControlLabel: parsed: +3.63% , gzip: +3.22% Bundle size reportDetails of bundle changes (Toolpad) |
| ownerState, | ||
| }); | ||
|
|
||
| delete typographySlotProps.ownerState; |
There was a problem hiding this comment.
Not entirely sure why deleting ownerState worked, but i looked previous PR's and accordion followed same pattern here https://github.com/mui/material-ui/pull/40418/files#diff-bbc688afec7bc090b1ad7bfd26cc2bedf991eca6fa3d2941b3dcbe04b3ee70a6R183
There was a problem hiding this comment.
The accordion case was due to #40653
Is something similar happening in the typography component?
There was a problem hiding this comment.
fixed in this commit 60d13f6
There was a problem hiding this comment.
May I ask you to split that fix into a different PR? That way, the history is cleaner, and we can revert the changes independently if necessary. Thanks!
There was a problem hiding this comment.
sure, here is the PR -> #41903. once it is merged, i'll merge next branch and ping you
There was a problem hiding this comment.
@DiegoAndai as #41903 is merged, this PR is ready for review
| ownerState, | ||
| }); | ||
|
|
||
| delete typographySlotProps.ownerState; |
There was a problem hiding this comment.
The accordion case was due to #40653
Is something similar happening in the typography component?
| @@ -18,17 +18,11 @@ describe('<FormControlLabel />', () => { | |||
| render, | |||
| muiName: 'MuiFormControlLabel', | |||
| testVariantProps: { disabled: true }, | |||
| testLegacyComponentsProp: true, | |||
There was a problem hiding this comment.
Is it necessary to remove this? If not, I would remove it when we remove componentsProps
There was a problem hiding this comment.
got it makes sense, restored this line and made testLegacyComponentsProp to false
There was a problem hiding this comment.
I was thinking we keep
testLegacyComponentsProp: true,
Or does that fail?
There was a problem hiding this comment.
Tests are failing on keeping it to true.
There was a problem hiding this comment.
Ok, I see what happened. The testLegacyComponentsProp only applies to the slots-related tests. Previously, it was true, but the slots-related tests were skipped, so testLegacyComponentsProp: true had no effect. As we stop skipping the slots tests, this flag has an effect, and because the component only has componentsProps and no components, the test fails.
Let's remove testLegacyComponents: true as you did initially. It should've never been added and was probably copy-pasted. Sorry for the confusion; my bad.
There was a problem hiding this comment.
no problem :)
removed testLegacyComponents
25d6373 to
a09473e
Compare
574599e to
8b21fdb
Compare
part of #41279