mui / material-ui

Material UI: Ready-to-use foundational React components, free forever. It includes Material UI, which implements Google's Material Design.
https://mui.com/material-ui/
MIT License
92.04k stars 31.64k forks source link

[material-ui][ListItemSecondaryAction] Deprecate component #42251

Closed aarongarciah closed 1 week ago

aarongarciah commented 2 weeks ago

We intended to deprecate ListItemSecondaryAction in v5 in favor of the secondaryAction prop in ListItem, but we forgot to do it officially so we can't remove it in v6. This PR adds a new ListItemSecondaryAction deprecation entry in the docs for v6 so we can remove it in v7. See https://github.com/mui/material-ui/pull/26446 for context.

Preview link: https://deploy-preview-42251--material-ui.netlify.app/material-ui/migration/migrating-from-deprecated-apis/#listitemsecondaryaction

image
aarongarciah commented 2 weeks ago

@siriwatknp some notes:

mui-bot commented 2 weeks ago

Netlify deploy preview

Bundle size report

No bundle size changes (Toolpad) No bundle size changes

Generated by :no_entry_sign: dangerJS against ae4992cedb50602ec4ca8e78391f2340caf3c361

DiegoAndai commented 2 weeks ago

I don't have answers for all questions but:

Is a codemod provided in these scenarios?

We can read the user's implementation and try to apply the changes accordingly, but if it would be too difficult to know the correct replacement, then it's better not to have a codemod. If that's the case, we should explain in detail how to adapt.

As of today, there's no way of displaying a component as deprecated in the API pages e.g. Hidden is marked as deprecated in the usage page but not the API page.

We could implement this capability, what do you think @alexfauquette @danilo-leal?

Should we remove ListItemSecondaryAction from the list of APIs in the Lists docs page?

Yes

aarongarciah commented 2 weeks ago

Should we remove ListItemSecondaryAction from the list of APIs in the Lists docs page?

We have a specific rule to validate that every public component is included in at least one docs md page. Removing ListItemSecondaryAction from the Lists page results in this error while building the docs:

Screenshot 2024-05-16 at 18 28 59
alexfauquette commented 2 weeks ago

The rendering part you're looking to modify is here:

https://github.com/mui/material-ui/blob/df6516ea9bd7a1377c59dda667ee907838c9bbf1/docs/src/modules/components/ApiPage.js#L251-L261

The description is generated here: https://github.com/mui/material-ui/blob/df6516ea9bd7a1377c59dda667ee907838c9bbf1/packages/api-docs-builder/ApiBuilders/ComponentApiBuilder.ts#L459

DiegoAndai commented 2 weeks ago

We have a specific rule to validate that every public component is included in at least one docs md page. Removing ListItemSecondaryAction from the Lists page results in this error while building the docs:

Could we add a "skip" list so we can skip some of them?

alexfauquette commented 2 weeks ago

Could we add a "skip" list so we can skip some of them?

Another option could be to keep it in the APIs but indicate it is deprecated

image

aarongarciah commented 2 weeks ago

I think we should look into being able to mark components as @deprecated in JSDoc without it ending up in the component docs description so we can:

Also, consumers would see components as deprecated in their IDE, which is not the case at the moment afaik.

But probably in another PR. I'm lacking quite some context on how docs are built so tell me if this is not a good idea.

aarongarciah commented 2 weeks ago

I opened a draft PR with a potential approach we could use to treat JSDoc comments as the source of truth for deprecations: https://github.com/mui/material-ui/pull/42280