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

Replace etc/no-deprecated with eslint-plugin-deprecation #732

Open
EvgenyOrekhov opened this issue Apr 30, 2023 · 9 comments
Open

Replace etc/no-deprecated with eslint-plugin-deprecation #732

EvgenyOrekhov opened this issue Apr 30, 2023 · 9 comments

Comments

@EvgenyOrekhov
Copy link
Owner

https://github.com/gund/eslint-plugin-deprecation

Unless cartant/eslint-plugin-etc#53 gets fixed.

@EvgenyOrekhov
Copy link
Owner Author

Or with sonar/deprecation from https://github.com/un-ts/eslint-plugin-sonar

@voxpelli
Copy link

voxpelli commented May 12, 2023

@EvgenyOrekhov
Copy link
Owner Author

@voxpelli

Just tested the rules on a real-world codebase:

  • etc/no-deprecated - 78 warnings
  • deprecation/deprecation - 76 warnings
  • sonar/deprecation - 76 warnings
  • import/no-deprecated - 54 warnings

So import/no-deprecated catches less cases.

Created #739 to disable import/no-deprecated for TS, since it's superseded by etc/no-deprecated.

@voxpelli
Copy link

Right, makes sense, type resolutions can be more complex

What are the two warnings that differ between the 78 and 76 ones?

@EvgenyOrekhov
Copy link
Owner Author

@voxpelli Seems like etc/no-deprecated just sometimes gives duplicated warnings, like

140:45  error  "pageYOffset" is deprecated: This is a legacy alias of `scrollY`  etc/no-deprecated	
140:45  error  "pageYOffset" is deprecated: This is a legacy alias of `scrollY`  etc/no-deprecated

Also tested time difference:

  • etc/no-deprecated - 47 seconds
  • deprecation/deprecation - 57 seconds
  • sonar/deprecation - 52 seconds

@voxpelli
Copy link

Yikes, yeah, these rules can be really slow, I tried adding deprecation checks to a large monorepo at a recent job and it increased the execution time of the linting so much that I reverted on it before ever suggesting the change 😕

@DeadlyMissile
Copy link

So which one of these is recommended to use lol?

@EvgenyOrekhov
Copy link
Owner Author

@DeadlyMissile I considered migrating away from etc/no-deprecated because it wasn't compatible with TypeScript v5, but now it is, so I'll stick to it for now, especially since it's a bit faster than the others.

But functionality-wise, all three are the same.

@CHE1RON
Copy link

CHE1RON commented Mar 4, 2024

Sorry to add to an old issue @EvgenyOrekhov, but it seems to break with Typescript 5.2, see cartant/eslint-plugin-etc#64

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants