[material-ui][ListItem] Removing deprecated props #41566

thathva · 2024-03-20T01:31:22Z

BREAKING CHANGES

ListItem's props autoFocus, button, disabled, and selected, deprecated in v5, have been removed. To replace the button prop, use ListItemButton instead. The other removed props are available in the ListItemButton component as well.

-<ListItem button />
+<ListItemButton />

Use this codemod to migrate your project to the ListItemButton component:

npx @mui/codemod@next v6.0.0/list-item-button-prop <path/to/folder>

As the ListItem no longer supports these props, the class names related to these props were removed. You should use the listItemButtonClasses object instead.

-import { listItemClasses } from '@mui/material/ListItem';
+import { listItemButtonClasses } from '@mui/material/ListItemButton';

- listItemClasses.button
+ listItemButtonClasses.root

- listItemClasses.focusVisible
+ listItemButtonClasses.focusVisible

- listItemClasses.disabled
+ listItemButtonClasses.disabled

- listItemClasses.selected
+ listItemButtonClasses.selected

mui-bot · 2024-03-20T01:33:51Z

Netlify deploy preview

docs/data/material/migration/migrating-to-v6/migrating-to-v6.md

ListItem: parsed: -19.53% 😍, gzip: -17.97% 😍
@material-ui/core: parsed: -0.29% 😍, gzip: -0.16% 😍

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against ac75cc6

DiegoAndai

Hey @thathva, thanks for working on this, here's my initial review.

Besides the comments, we should add these removals as breaking changes in https://github.com/DiegoAndai/material-ui/blob/next/docs/data/material/migration/migration-v5/migration-v5.md. You might have to rebase/merge next into your branch.

packages/mui-material/src/ListItem/ListItem.d.ts

test/integration/material-ui/components.spec.tsx

packages/mui-material/src/ListItem/ListItem.spec.tsx

thathva · 2024-03-20T23:49:37Z

I have made the changes as commented. Let me know if there are any more changes!

DiegoAndai · 2024-03-22T15:27:06Z

Nicely done @thathva!

Regarding the migration guide:

We need to add these removals to the guide in this PR. But the guide was added after the thathva:material-ui-listitem-remove-props branch was created, so the files do not exist. That is why you'll have to do:

git checkout next
git pull upstream next
git checkout material-ui-listitem-remove-props
git merge next

Which will bring the latest changes of the next branch into the material-ui-listitem-remove-props branch. Then you'll be able to find the /docs/data/material/migration/migration-v5/migration-v5.md file and add the removals to it. You can follow this similar migration guide to follow: https://github.com/mui/material-ui/blob/next/docs/data/system/migration/migration-v5/migration-v5.md

thathva · 2024-03-22T23:52:14Z

Thank you for the references and the detailed steps @DiegoAndai!
I have updated the migration document.

DiegoAndai

Hey @thathva, thanks for incorporating the suggested changes 🙌🏼

I left a suggestion on the migration guide structure and text.

Besides that, I think we should add a codemod for this breaking change in this same PR. The codemod will replace all occurrences of <ListItem button /> to <ListItemButton />. Would you be willing to work on it? If so, I can set it up and guide you through it.

docs/data/material/migration/migration-v5/migration-v5.md

thathva · 2024-03-25T17:48:38Z

Hi @DiegoAndai
Sure, I will rephrase it as you suggested.
And the codemod, yes I would love to add that! If there is a guide, I can refer that and try it out.

DiegoAndai · 2024-03-26T17:44:11Z

And the codemod, yes I would love to add that! If there is a guide, I can refer that and try it out.

I'll set it up today/tomorrow and reach out 😊

DiegoAndai · 2024-03-27T14:11:23Z

Hey @thathva, I set up the codemods for v6 🎉

First you'll have to merge next into your branch:

git checkout next
git pull upstream
git checkout material-ui-listitem-remove-props
git merge next

Then, read the codemods contributing guide.

The codemod we will add should be named list-item-button-prop. It should do the following:

Find all ListItem components with the button prop
Replace them with ListItemButton, removing the button prop but retaining all other props
Add the ListItemButton import to the file if it doesn't exist
If there are no other ListItem components in the file, remove the ListItem import

The codemod should be added to the v6.0.0 codemods folder.

For further guidance, I recommend going through:

The jscodeshift documentation
The deprecations avatar-props codemod which shows you how to find a certain prop on a certain component

Please reach out if you're stuck and need help 😊

I will be on vacation starting tomorrow (March 28th) until April 8th, so I won't attend to notifications during that time. @siriwatknp may I ask you to provide guidance in the meantime?

thathva · 2024-03-27T14:29:39Z

Awesome, thanks for the guide @DiegoAndai!
I will take a look and reach out if I have any questions. Have a great vacation!

thathva · 2024-04-05T23:40:35Z

Hey @siriwatknp
I apologize for the delay in the PR, I have been occupied with few things. I am facing a bit of issues with the codemod, so would appreciate some help. The code is the first version that I was hoping to get it to work before I could refactor.
I have the code below for the codemod, and the test cases, which seem to fail. I was wondering if you could help me out if I am going in the right direction?

list-item-button-prop.js

import findComponentJSX from '../../util/findComponentJSX';

/**
 * @param {import('jscodeshift').FileInfo} file
 * @param {import('jscodeshift').API} api
 */
export default function transformer(file, api, options) {
  const j = api.jscodeshift;
  const root = j(file.source);
  const printOptions = options.printOptions;

  //Rename components that have ListItem button to ListItemButton
  findComponentJSX(j, { root, componentName: 'ListItem' }, (elementPath) => {
    const index = elementPath.node.openingElement.attributes.findIndex(
      (attr) => attr.type === 'JSXAttribute' && attr.name.name === 'button',
    );
    if (index !== -1) {
      elementPath.node.openingElement.name.name = 'ListItemButton';
      elementPath.node.openingElement.attributes.splice(index, 1);
    }
  });

  //Find if there are ListItem imports/named imports.
  let containsListItemImport = root.find(j.ImportDeclaration).filter(path => path.node.source.value === '@mui/material');
  let containsListItemNamedImport = containsListItemImport.find(j.ImportSpecifier).filter(path => path.node.imported.name === 'ListItem');
  let containsListItem = false;

  //Find components that use ListItem. If they do, we shouldn't remove it
  findComponentJSX(j, { root, componentName: 'ListItem' }, (elementPath) => {
    if(elementPath.node.openingElement.name.name === 'ListItem') {
      containsListItem = true;
    }
  });

  //Remove ListItem import if there is no usage
  if(containsListItemNamedImport.length === 0 || !containsListItem) {
    root.find(j.ImportDeclaration).filter(path => path.node.source.value === '@mui/material/ListItem').remove();
  }

  //If ListItemButton does not already exist, add it at the end
  let imports = root.find(j.ImportDeclaration).filter(path => path.node.source.value === '@mui/material/ListItemButton');

  if(imports.length === 0) {
    let lastImport = root.find(j.ImportDeclaration).at(-1);

    // Insert the import for 'ListItemButton' after the last import declaration
    lastImport.insertAfter(
      j.importDeclaration(
        [j.importDefaultSpecifier(j.identifier('ListItemButton'))],
        j.stringLiteral('@mui/material/ListItemButton')
      )
    );
  }

  return root.toSource(printOptions);
}

actual.js

import ListItem from '@mui/material/ListItem';
import {ListItem as MyListItem} from '@mui/material';

<ListItem button/>;

<MyListItem button/>;

expected.js

import ListItemButton from '@mui/material/ListItem';
import {ListItemButton as MyListItemButton} from '@mui/material';

<ListItemButton />;
<MyListItemButton />;

The test case to transform the props is failing with these results:

+expected 
-actual

-import {ListItem as MyListItem} from '@mui/material';
-
-import ListItemButton from "@mui/material/ListItemButton";
-
-
-
-<ListItemButton />;
-
-
-
-<ListItemButton />;
+import {ListItemButton as MyListItemButton} from '@mui/material';
+import ListItemButton from '@mui/material/ListItem';
+
+<ListItemButton />;
+<MyListItemButton />;

DiegoAndai · 2024-04-09T21:22:13Z

Hey @thathva, I'm back from vacation so I can help with this.

The code is the first version that I was hoping to get it to work before I could refactor.

Could you commit this first version? That way it will be easier to review.

In the following code:

  //Remove ListItem import if there is no usage
  if(containsListItemNamedImport.length === 0 || !containsListItem) {
    root.find(j.ImportDeclaration).filter(path => path.node.source.value === '@mui/material/ListItem').remove();
  }

We're only removing the @mui/material/ListItem import, that's why the

import {ListItem as MyListItem} from '@mui/material';

is not removed. We have to cover that case as well.

thathva · 2024-04-10T01:15:54Z

Hey @DiegoAndai
Thank you for your guidance again! I have pushed the changes. I have used similar test cases as other codemods, and I am failing 2 of the 6 test cases, which one of them is because of the named import. Would appreciate your help!

DiegoAndai

Hey @thathva! Here are some suggestions to move forward

DiegoAndai · 2024-04-10T20:04:41Z

packages/mui-codemod/src/v6.0.0/list-item-button-prop/list-item-button-prop.js

+  //Find if there are ListItem imports/named imports.
+  let containsListItemImport = root.find(j.ImportDeclaration).filter(path => path.node.source.value === '@mui/material');
+  let containsListItemNamedImport = containsListItemImport.find(j.ImportSpecifier).filter(path => path.node.imported.name === 'ListItem');
+  let containsListItem = false;
+
+  //Find components that use ListItem. If they do, we shouldn't remove it
+  findComponentJSX(j, { root, componentName: 'ListItem' }, (elementPath) => {
+    if(elementPath.node.openingElement.name.name === 'ListItem') {
+      containsListItem = true;
+    }
+  });
+
+  //Remove ListItem import if there is no usage
+  if(containsListItemNamedImport.length === 0 || !containsListItem) {
+    // root.find(j.ImportDeclaration).filter(path => path.node.source.value === '@mui/material').find(j.ImportSpecifier)
+    // .filter(path => path.node.imported.name === 'ListItem').remove();
+    root.find(j.ImportDeclaration).filter(path => path.node.source.value === '@mui/material/ListItem').remove();
+  }


Let's try this (I haven't tried it so it might need some fixing):

Suggested change

//Find if there are ListItem imports/named imports.

let containsListItemImport = root.find(j.ImportDeclaration).filter(path => path.node.source.value === '@mui/material');

let containsListItemNamedImport = containsListItemImport.find(j.ImportSpecifier).filter(path => path.node.imported.name === 'ListItem');

let containsListItem = false;

//Find components that use ListItem. If they do, we shouldn't remove it

findComponentJSX(j, { root, componentName: 'ListItem' }, (elementPath) => {

if(elementPath.node.openingElement.name.name === 'ListItem') {

containsListItem = true;

}

});

//Remove ListItem import if there is no usage

if(containsListItemNamedImport.length === 0 || !containsListItem) {

// root.find(j.ImportDeclaration).filter(path => path.node.source.value === '@mui/material').find(j.ImportSpecifier)

// .filter(path => path.node.imported.name === 'ListItem').remove();

root.find(j.ImportDeclaration).filter(path => path.node.source.value === '@mui/material/ListItem').remove();

}

//Find components that use ListItem. If they do, we shouldn't remove it

findComponentJSX(j, { root, componentName: 'ListItem' }, (elementPath) => {

if(elementPath.node.openingElement.name.name === 'ListItem') {

containsListItem = true;

}

});

//Find if there are ListItem imports/named imports.

let containsListItemImport = root.find(j.ImportDeclaration).filter(path => path.node.source.value === '@mui/material');

let containsListItemNamedImport = containsListItemImport.find(j.ImportSpecifier).filter(path => path.node.imported.name === 'ListItem');

let containsListItem = false;

// Remove ListItem imports if there is no usage

if(!containsListItem) {

// Remove named imports

root.find(j.ImportDeclaration).filter(path => path.node.source.value === '@mui/material').find(j.ImportSpecifier).filter(path => path.node.imported.name === 'ListItem').remove();

// Remove default imports

root.find(j.ImportDeclaration).filter(path => path.node.source.value === '@mui/material/ListItem').remove();

}

DiegoAndai · 2024-04-10T20:05:21Z

packages/mui-codemod/src/v6.0.0/all/deprecations-all.js

+ * @param {import('jscodeshift').API} api
+ */
+export default function deprecationsAll(file, api, options) {
+  file.source = transformerListItemButtonProps(file, api, options);


This should be added to the v6-all one, as it's not a deprecation. And we should remove this file.

thathva · 2024-04-11T23:54:03Z

Hey @DiegoAndai!
Thank you for your inputs! I used the code you mentioned and was able to fix that test case. I made the following assumptions:

Renaming the imported name from ListItem to ListItemButton since we don't need to worry about the alias of that component.
Using the same assumption, not changing the alias in the code, but just removing the button prop from it.
Let me know if this sounds good.
Apart from this, there is a test case failing for the theme, which I am not familiar with. Can you direct me to any other codemod that modifies the theme? To my understanding, we would need to remove the defaultProps from MuiListItem and replace it with MuiListItemButton. The test case:
I was able to fix all the test cases. But, I still have some doubt over the correctness of the actual.js and expected.js files for themes and props. Can you let me know if this looks good, so that I could perform the codemod for the entire codebase?

mnajdova · 2024-07-31T11:49:33Z

packages/mui-codemod/src/v6.0.0/list-item-button-prop/test-cases/theme.expected.js

@@ -0,0 +1,21 @@
+fn({


@DiegoAndai , I am not sure about these transformations honestly, as there could be different interpretation on what was the intention. I did what felt most intuitive to me, but I am honestly not happy with it. I think I would rather not handle this and let people do what they think should be best.

If I understand correctly, the logic is: If MuiListItem is present and has button as default prop, then remove button and create a MuiListItemButton entry copying the other default prop values, right? That is the most likely migration to me.

What other interpretations of the intention are you thinking about?

mnajdova · 2024-07-31T11:50:25Z

@DiegoAndai I improved a lot the codemod in regards to the replacement of the open tags & removing the non-used imports. I am not sure about the theming codemod, honestly I would just drop that logic. I am letting you do the final review.

docs/data/material/migration/migrating-to-v6/migrating-to-v6.md

Signed-off-by: Marija Najdova <mnajdova@gmail.com>

siriwatknp · 2024-08-08T06:36:04Z

packages/mui-material/src/ListItem/ListItem.d.ts

-      'li'
-    >
-  >;
+declare const ListItem: OverridableComponent<ListItemTypeMap<{}, 'li'>>;


docs/data/material/migration/migrating-to-v6/migrating-to-v6.md

Co-authored-by: Siriwat K <siriwatkunaporn@gmail.com> Signed-off-by: Marija Najdova <mnajdova@gmail.com>

siriwatknp

For the codemod, I could not think of other edge cases and I would not worry about it that much. It should not block the migration.

docs/data/material/migration/migrating-to-v6/migrating-to-v6.md

Signed-off-by: Marija Najdova <mnajdova@gmail.com>

mnajdova · 2024-08-08T07:03:32Z

@thathva thanks for kicking off this effort. I pushed few commits and merged it :)

thathva · 2024-08-08T13:26:27Z

Thank you so much @mnajdova and I apologize for not finishing it completely. The changes you made looks great!

Removing deprecated props from material-ui ListItem

2efb7a1

thathva changed the title ~~[material-ui][ListItem] Removing deprecated props from material-ui ListItem~~ [material-ui][ListItem] Removing deprecated props Mar 20, 2024

danilo-leal added component: list This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material labels Mar 20, 2024

DiegoAndai self-requested a review March 20, 2024 15:17

DiegoAndai reviewed Mar 20, 2024

View reviewed changes

packages/mui-material/src/ListItem/ListItem.d.ts Outdated Show resolved Hide resolved

test/integration/material-ui/components.spec.tsx Outdated Show resolved Hide resolved

packages/mui-material/src/ListItem/ListItem.spec.tsx Show resolved Hide resolved

Removed ListItem's dependency on ExtendButtonBase

6a09a81

Yenuga, Thathva Sri Sai Reddy added 3 commits March 22, 2024 18:48

Merge branch 'next' into material-ui-listitem-remove-props

5fde6a0

Updated migration document

5a84d05

Adding space between headings in markdown

418bf7a

DiegoAndai reviewed Mar 25, 2024

View reviewed changes

docs/data/material/migration/migration-v5/migration-v5.md Outdated Show resolved Hide resolved

Rephrasing breaking changes section in migration doc

8f8db62

DiegoAndai mentioned this pull request Mar 26, 2024

[codemod] Setup v6 codemod structure #41668

Merged

Yenuga, Thathva Sri Sai Reddy added 2 commits April 9, 2024 20:17

Merge branch 'next' into material-ui-listitem-remove-props

503d663

Codemod changes for converting ListItem to ListItemButton

6e14621

DiegoAndai reviewed Apr 10, 2024

View reviewed changes

Fixing failed test case related to component

86242b4

Fixing failing theme related test case

9b31019

mnajdova added 2 commits July 31, 2024 10:17

improve codemod

fdb20e9

lint issues

2e38c09

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 31, 2024

mnajdova added 2 commits July 31, 2024 13:04

improve codemod

60dfaf5

finish codemod

a9861fe

mnajdova reviewed Jul 31, 2024

View reviewed changes

mnajdova added 3 commits July 31, 2024 14:39

fixed some issues

5e78016

docs:typescript:formatted

0e23f10

remove style assosiated with some of the removed classes

c312da3

mnajdova requested a review from DiegoAndai July 31, 2024 12:51

mnajdova added 2 commits July 31, 2024 14:55

remove unused import

842d1f6

add codemod instructions, fix theme codemod

21bc5f4

mnajdova reviewed Aug 5, 2024

View reviewed changes

docs/data/material/migration/migrating-to-v6/migrating-to-v6.md Outdated Show resolved Hide resolved

Update docs/data/material/migration/migrating-to-v6/migrating-to-v6.md

5bde93a

Signed-off-by: Marija Najdova <mnajdova@gmail.com>

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 5, 2024

Merge branch 'next' into material-ui-listitem-remove-props

b40c0f5

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 8, 2024

siriwatknp reviewed Aug 8, 2024

View reviewed changes

Fix usage of wrong class names

ffa33da

siriwatknp reviewed Aug 8, 2024

View reviewed changes

docs/data/material/migration/migrating-to-v6/migrating-to-v6.md Show resolved Hide resolved

Update docs/data/material/migration/migrating-to-v6/migrating-to-v6.md

30aa518

Co-authored-by: Siriwat K <siriwatkunaporn@gmail.com> Signed-off-by: Marija Najdova <mnajdova@gmail.com>

siriwatknp approved these changes Aug 8, 2024

View reviewed changes

mnajdova reviewed Aug 8, 2024

View reviewed changes

docs/data/material/migration/migrating-to-v6/migrating-to-v6.md Outdated Show resolved Hide resolved

Update docs/data/material/migration/migrating-to-v6/migrating-to-v6.md

ac75cc6

Signed-off-by: Marija Najdova <mnajdova@gmail.com>

mnajdova merged commit d3432b9 into mui:next Aug 8, 2024
22 checks passed

mnajdova added the breaking change label Aug 8, 2024

DiegoAndai mentioned this pull request Aug 8, 2024

[material-ui][ListItem] Remove props which have been deprecated since 2021 #41296

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[material-ui][ListItem] Removing deprecated props #41566

[material-ui][ListItem] Removing deprecated props #41566

thathva commented Mar 20, 2024 •

edited by mnajdova

Loading

mui-bot commented Mar 20, 2024 •

edited

Loading

DiegoAndai left a comment

thathva commented Mar 20, 2024 •

edited

Loading

DiegoAndai commented Mar 22, 2024

thathva commented Mar 22, 2024 •

edited

Loading

DiegoAndai left a comment

thathva commented Mar 25, 2024

DiegoAndai commented Mar 26, 2024

DiegoAndai commented Mar 27, 2024

thathva commented Mar 27, 2024

thathva commented Apr 5, 2024

DiegoAndai commented Apr 9, 2024

thathva commented Apr 10, 2024

DiegoAndai left a comment

DiegoAndai Apr 10, 2024

DiegoAndai Apr 10, 2024

thathva commented Apr 11, 2024 •

edited

Loading

mnajdova Jul 31, 2024

DiegoAndai Jul 31, 2024

mnajdova commented Jul 31, 2024

siriwatknp Aug 8, 2024

siriwatknp left a comment

mnajdova commented Aug 8, 2024

thathva commented Aug 8, 2024 •

edited

Loading

[material-ui][ListItem] Removing deprecated props #41566

[material-ui][ListItem] Removing deprecated props #41566

Conversation

thathva commented Mar 20, 2024 • edited by mnajdova Loading

BREAKING CHANGES

mui-bot commented Mar 20, 2024 • edited Loading

Netlify deploy preview

Bundle size report

DiegoAndai left a comment

Choose a reason for hiding this comment

thathva commented Mar 20, 2024 • edited Loading

DiegoAndai commented Mar 22, 2024

thathva commented Mar 22, 2024 • edited Loading

DiegoAndai left a comment

Choose a reason for hiding this comment

thathva commented Mar 25, 2024

DiegoAndai commented Mar 26, 2024

DiegoAndai commented Mar 27, 2024

thathva commented Mar 27, 2024

thathva commented Apr 5, 2024

DiegoAndai commented Apr 9, 2024

thathva commented Apr 10, 2024

DiegoAndai left a comment

Choose a reason for hiding this comment

DiegoAndai Apr 10, 2024

Choose a reason for hiding this comment

DiegoAndai Apr 10, 2024

Choose a reason for hiding this comment

thathva commented Apr 11, 2024 • edited Loading

mnajdova Jul 31, 2024

Choose a reason for hiding this comment

DiegoAndai Jul 31, 2024

Choose a reason for hiding this comment

mnajdova commented Jul 31, 2024

siriwatknp Aug 8, 2024

Choose a reason for hiding this comment

siriwatknp left a comment

Choose a reason for hiding this comment

mnajdova commented Aug 8, 2024

thathva commented Aug 8, 2024 • edited Loading

thathva commented Mar 20, 2024 •

edited by mnajdova

Loading

mui-bot commented Mar 20, 2024 •

edited

Loading

thathva commented Mar 20, 2024 •

edited

Loading

thathva commented Mar 22, 2024 •

edited

Loading

thathva commented Apr 11, 2024 •

edited

Loading

thathva commented Aug 8, 2024 •

edited

Loading