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

[core] Remove unused classes #23473

Merged
merged 5 commits into from
Nov 24, 2020
Merged

[core] Remove unused classes #23473

merged 5 commits into from
Nov 24, 2020

Conversation

jens-ox
Copy link
Contributor

@jens-ox jens-ox commented Nov 10, 2020

Hi all!

This is just a minor addition to the eslint plugin, which detects unused classes in makeStyles declarations.
I saw that this was requested in different places (e.g. here and here).

Cheers!

Closes #23492

@mui-pr-bot
Copy link

mui-pr-bot commented Nov 10, 2020

Details of bundle changes

Generated by 🚫 dangerJS against a8b0f0e

Copy link
Member

@oliviertassinari oliviertassinari 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 sharing. I have tried to enable it on the repository but I couldn't get it work. Any idea?

@oliviertassinari
Copy link
Member

oliviertassinari commented Nov 16, 2020

@jens-ox If you have time to look into the issue, please do, we would be happy to include this eslint rule :).

Now, we also plan to remove the withStyles and makeStyles, progressively (v6), so I'm closing, as it was inactive and the core have no bandwidth to look into it.

@jens-ox
Copy link
Contributor Author

jens-ox commented Nov 23, 2020

Hi @oliviertassinari! Thanks for getting back.

Could you quickly explain how I can enable it in the repository?

@oliviertassinari oliviertassinari changed the title add unused classes detection rule [eslint] Add unused classes detection rule Nov 23, 2020
@oliviertassinari oliviertassinari added the new feature New feature or request label Nov 23, 2020
@oliviertassinari oliviertassinari changed the title [eslint] Add unused classes detection rule [core] Remove unused classes Nov 23, 2020
@oliviertassinari
Copy link
Member

@jens-ox I had a second look at the pull request. I could make it work on the codebase, and even better remove dead classes in our demos :). However, I could also spot a couple of limitations:

  • Doesn't support withStyles
  • Doesn't detect when a style rule is used as a selector to increase specificity only, e.g. &$focused
  • I have fixed the support of @global
  • Doesn't detect when a classes is provided as a prop to another component.

In the end, I have removed the new eslint rule in a8b0f0e based on the direction we are taking with #22342. Getting this eslint rule to a great state will require a non-negligible amount of work. If you want to keep polishing, please do! It would be great to link the documentation to this eslint rule hosted under a different repository.

@oliviertassinari oliviertassinari merged commit bd774c5 into mui:next Nov 24, 2020
@jens-ox
Copy link
Contributor Author

jens-ox commented Dec 1, 2020

Just created a separate package for the rule: https://github.com/jens-ox/eslint-plugin-material-ui-unused-classes

Should I create a PR to link to it in the docs?

@oliviertassinari
Copy link
Member

Should I create a PR to link to it in the docs?

Yes, I think that it would be interesting :)

@oliviertassinari
Copy link
Member

@jens-ox Could you add documentation for the eslint plugin in the new repository? Regarding linking it in the docs, maybe under https://next.material-ui.com/discover-more/related-projects/#ide-tools?

@jens-ox
Copy link
Contributor Author

jens-ox commented Dec 4, 2020

Updated the docs in the repo, created PR (#23843) ☺️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[styles] ESLint plugin to catch unused styles
3 participants