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

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

Conversation

aarongarciah
Copy link
Member

@aarongarciah aarongarciah commented May 15, 2024

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 #26446 for context.

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

image

@aarongarciah aarongarciah added docs Improvements or additions to the documentation component: list This is the name of the generic UI component, not the React module! deprecation New deprecation message package: material-ui Specific to @mui/material labels May 15, 2024
@aarongarciah
Copy link
Member Author

aarongarciah commented May 15, 2024

@siriwatknp some notes:

  • I tried to mark ListItemSecondaryAction as @deprecated using JSDoc but it was picked up as the component description in the docs. Is there a way to mark components as @deprecated in the code? Other deprecated components like Hidden aren't marked either.
    Screenshot 2024-05-15 at 17 37 32
  • I also thought about marking listItemSecondaryActionClasses and related stuff as @deprecated, but it doesn't make much sense if we don't mark the component as @deprecated.
  • Lastly, is a codemod provided in these scenarios? The codemod could work or not depending on the user's implementation e.g. if they are using <ListItemSecondaryAction> + the ContainerComponent prop in ListItem the codemod can break user's implementation.
  • 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. This is unfortunate because ListItemSecondaryAction doesn't have a dedicated usage page so it's hard for consumers to know it's deprecated unless they visit the Migrating from deprecated APIs page and we also don't mark components as @deprecated in the code.
  • Should we remove ListItemSecondaryAction from the list of APIs in the Lists docs page?

@mui-bot
Copy link

mui-bot commented May 15, 2024

@DiegoAndai
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member

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

{componentDescription ? (
<React.Fragment>
<br />
<br />
<span
dangerouslySetInnerHTML={{
__html: componentDescription,
}}
/>
</React.Fragment>
) : null}

The description is generated here:

componentDescription: reactApi.description,

@DiegoAndai
Copy link
Member

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
Copy link
Member

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
Copy link
Member Author

aarongarciah commented May 16, 2024

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:

  • Automatically display a deprecation callout in the usage and API pages.
  • Remove the entry (or display as deprecated) in the related APIs list.
  • Make any other automation we can think of.

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
Copy link
Member Author

I opened a draft PR with a potential approach we could use to treat JSDoc comments as the source of truth for deprecations: #42280

@aarongarciah aarongarciah changed the title [material-ui][ListItemSecondaryAction] Deprecate ListItemSecondaryAction component [material-ui][ListItemSecondaryAction] Deprecate component May 21, 2024
@aarongarciah aarongarciah force-pushed the aarongarcia/deprecate-list-item-secondary-action branch 2 times, most recently from 171972b to 18ffff4 Compare May 22, 2024 14:22
@aarongarciah aarongarciah force-pushed the aarongarcia/deprecate-list-item-secondary-action branch from 18ffff4 to ae4992c Compare May 22, 2024 14:30
@aarongarciah aarongarciah merged commit 1f8c0b6 into mui:next May 23, 2024
22 checks passed
@aarongarciah aarongarciah deleted the aarongarcia/deprecate-list-item-secondary-action branch May 23, 2024 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: list This is the name of the generic UI component, not the React module! deprecation New deprecation message docs Improvements or additions to the documentation package: material-ui Specific to @mui/material
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants