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.

Sign up for GitHub

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

Jump to bottom

[material-ui][avatar] Add props deprecation with a codemod #40853

Merged
merged 11 commits into from
Feb 12, 2024
Merged

[material-ui][avatar] Add props deprecation with a codemod #40853

merged 11 commits into from
Feb 12, 2024

Conversation

siriwatknp
Copy link
Member

Part of #40417

@siriwatknp siriwatknp added package: material-ui Specific to @mui/material package: codemod Specific to @mui/codemod labels Jan 30, 2024
@siriwatknp siriwatknp requested a review from DiegoAndai January 30, 2024 06:52
@siriwatknp siriwatknp added component: avatar This is the name of the generic UI component, not the React module! v6.x labels Jan 30, 2024
@mui-bot
Copy link

mui-bot commented Jan 30, 2024 •
edited
Loading

Netlify deploy preview

@material-ui/lab: parsed: +0.37% , gzip: +0.38%

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against f95a35b

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 31, 2024
@siriwatknp
Merge branch 'master' into deprecation/avatar-props
61e7e47 
Signed-off-by: Siriwat K <siriwatkunaporn@gmail.com>
@github-actions github-actions bot added PR: out-of-date The pull request has merge conflicts and can't be merged and removed PR: out-of-date The pull request has merge conflicts and can't be merged labels Feb 1, 2024
@siriwatknp
Merge branch 'master' into deprecation/avatar-props
1281a30 
Signed-off-by: Siriwat K <siriwatkunaporn@gmail.com>
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 6, 2024
@siriwatknp
Copy link
Member Author

@DiegoAndai I adjust the migration doc format a bit to start with codemod because I think that's what the majority of users are looking for.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 7, 2024
DiegoAndai
DiegoAndai reviewed Feb 7, 2024
View reviewed changes
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.

Some comments/questions 😊

docs/data/material/migration/migrating-from-deprecated-apis/migrating-from-deprecated-apis.md Outdated Show resolved Hide resolved
packages/mui-codemod/README.md Outdated Show resolved Hide resolved
packages/mui-codemod/src/deprecations/avatar-props/avatar-props.js
const root = j(file.source);
const printOptions = options.printOptions;

findComponentJSX(j, { root, componentName: 'Avatar' }, (elementPath) => {
Copy link
Member

Choose a reason for hiding this comment

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

We should look into an abstraction for the slot pattern standardization codemods, as all will be pretty similar. It doesn't need to happen now, we can wait until we have a couple more.

packages/mui-codemod/src/util/findComponentJSX.js Outdated
});
});

root.find(j.ImportDeclaration, { source: { value: '@mui/material' } }).forEach((path) => {
Copy link
Member

Choose a reason for hiding this comment

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

Isn't @mui/material covered above?

@DiegoAndai
Copy link
Member

I adjust the migration doc format a bit to start with codemod because I think that's what the majority of users are looking for.

Thanks! can you rebase and do the same for the recently added Divider one?

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 8, 2024
siriwatknp
siriwatknp commented Feb 8, 2024
View reviewed changes
packages/mui-codemod/src/deprecations/all/deprecations-all.js

/**
* @param {import('jscodeshift').FileInfo} file
* @param {import('jscodeshift').API} api
*/
export default function deprecationsAll(file, api, options) {
file.source = transformAccordionProps(file, api, options);
file.source = transformAvatarProps(file, api, options);
file.source = transformDividerProps(file, api, options);
Copy link
Member Author

Choose a reason for hiding this comment

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

@DiegoAndai For completeness, I added divider transform here. Let me know if this is okay to you, otherwise I will open a separate PR.

Copy link
Member

Choose a reason for hiding this comment

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

That's perfect, we missed it 😅

DiegoAndai
DiegoAndai approved these changes Feb 8, 2024
View reviewed changes
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.

LGTM 🚀

I changed the guide links to point to HEAD

docs/data/material/migration/migrating-from-deprecated-apis/migrating-from-deprecated-apis.md Outdated Show resolved Hide resolved
docs/data/material/migration/migrating-from-deprecated-apis/migrating-from-deprecated-apis.md Outdated Show resolved Hide resolved
docs/data/material/migration/migrating-from-deprecated-apis/migrating-from-deprecated-apis.md Outdated Show resolved Hide resolved
@DiegoAndai
Links target HEAD
f95a35b 
Signed-off-by: Diego Andai <diego@mui.com>
@siriwatknp siriwatknp merged commit a419b48 into mui:master Feb 12, 2024
22 checks passed
@DiegoAndai DiegoAndai mentioned this pull request Feb 14, 2024
Open
@DiegoAndai DiegoAndai mentioned this pull request Feb 26, 2024
Open
35 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DiegoAndai DiegoAndai DiegoAndai approved these changes

Assignees
No one assigned
Labels
component: avatar This is the name of the generic UI component, not the React module! package: codemod Specific to @mui/codemod package: material-ui Specific to @mui/material v6.x
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@siriwatknp @mui-bot @DiegoAndai