Skip to content
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

[mui-codemod] Fix merging of slotProps and componentProps #41323

Merged
merged 4 commits into from Mar 7, 2024

Conversation

sai6855
Copy link
Contributor

@sai6855 sai6855 commented Mar 1, 2024

If same slot is present in slotProps and componentProps, i'd expect codemod to merge componentProps and slotProps with slotProps taking precedence but current codemod entirely removes componentProps.slot. This PR fixes this bug

@sai6855 sai6855 added bug 🐛 Something doesn't work package: codemod Specific to @mui/codemod labels Mar 1, 2024
@sai6855 sai6855 marked this pull request as draft March 1, 2024 12:16
@mui-bot
Copy link

mui-bot commented Mar 1, 2024

Netlify deploy preview

https://deploy-preview-41323--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against f61570b

@sai6855 sai6855 changed the title [mui-codemod] Fix merging of slotProps [mui-codemod] Fix merging of slotProps of componentProps Mar 1, 2024
@@ -31,7 +31,7 @@ describe('@mui/codemod', () => {
const actual = transform(
{ source: read('./test-cases/theme.actual.js') },
{ jscodeshift },
{ printOptions: { trailingComma: true } },
{ printOptions: { trailingComma: false } },
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

codemod was transforming without trailing comma, so changed to false here. Hope it's not an issue

@sai6855 sai6855 requested a review from DiegoAndai March 1, 2024 12:58
@sai6855 sai6855 marked this pull request as ready for review March 1, 2024 12:58
@sai6855 sai6855 changed the title [mui-codemod] Fix merging of slotProps of componentProps [mui-codemod] Fix merging of slotProps and componentProps Mar 1, 2024
@sai6855
Copy link
Contributor Author

sai6855 commented Mar 6, 2024

Hey @DiegoAndai can this PR be reviewed, this PR is blocking my other components, componentProps deprecation PR's

Copy link
Member

@DiegoAndai DiegoAndai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching and fixing this @sai6855 🚀

Sorry for the delay in the review.

@DiegoAndai DiegoAndai merged commit 727cc39 into mui:master Mar 7, 2024
19 checks passed
@sai6855 sai6855 deleted the fix-codemod-props branch March 8, 2024 00:30
mnajdova pushed a commit to mnajdova/material-ui that referenced this pull request Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work package: codemod Specific to @mui/codemod
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants