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

Drop eslint-plugin-sonarjs #434

Open
SukkaW opened this issue Sep 28, 2024 · 6 comments
Open

Drop eslint-plugin-sonarjs #434

SukkaW opened this issue Sep 28, 2024 · 6 comments

Comments

@SukkaW
Copy link

SukkaW commented Sep 28, 2024

https://pkg-size.dev/eslint-plugin-sonarjs

eslint-plugin-sonarjs includes @babel/core, vue-eslint-parser, eslint-plugin-react, eslint-plugin-import, typescript-eslint@v7 all in its dependencies, resulting in an installation size of 87 MiB. It is ridiculous.

I have tried to submit a PR to eslint-plugin-sonarjs, but due to how they organize their monorepo and publish packages on npm, it seems impossible to move those dependencies to peer dependencies.

eslint-config-upleveled only uses a few rules from eslint-plugin-sonarjs, but most of them have a replacement:

  • sonarjs/no-duplicated-branches and sonarjs/no-identical-conditions
  • sonarjs/prefer-single-boolean-return
    • There is no identical eslint rule here, but eslint-plugin-sonarjs doesn't work on type, so the rule can only detect boolean literal, which might be limited.
@karlhorky
Copy link
Member

karlhorky commented Sep 28, 2024

Interesting, thanks for the proposal!

I've been waiting on upgrading to eslint-plugin-sonarjs@^2, because I was looking for a changelog (eslint-plugin-sonarjs@1.0.4 is 11MB). Moving to a different plugin / rules is an interesting alternative... 🤔 I'll have to consider whether any of the v2 rules are good enough to stick with it (or if there are good enough alternatives)

@karlhorky
Copy link
Member

I have tried to submit a PR to eslint-plugin-sonarjs, but due to how they organize their monorepo and publish packages on npm, it seems impossible to move those dependencies to peer dependencies.

I would be interested in seeing the PR too - in case there's any chance that they would still approve and merge it, didn't see one in a first naive search

@SukkaW
Copy link
Author

SukkaW commented Sep 28, 2024

I have tried to submit a PR to eslint-plugin-sonarjs, but due to how they organize their monorepo and publish packages on npm, it seems impossible to move those dependencies to peer dependencies.

I would be interested in seeing the PR too - in case there's any chance that they would still approve and merge it, didn't see one in a first naive search

I had a local copy of the repo and was trying to understand how they manage the monorepo and publish them to npm. But after experimenting here and there I eventually realize that I will have to submit a huge PR to change their building infrastructure completely. That's why I didn't submit a PR yet.

@karlhorky
Copy link
Member

Ok thanks for the details!

cc @yassin-kammoun-sonarsource @vdiez in case you weren't aware of the size / dependency infrastructure issues with eslint-plugin-sonarjs

@SukkaW
Copy link
Author

SukkaW commented Sep 28, 2024

I've been waiting on upgrading to eslint-plugin-sonarjs@^2, because I was looking for a changelog (eslint-plugin-sonarjs@1.0.4 is 11MB).

BTW, it sounds a bit ironic as their eslint-plugin-sonarjs@1 repo (https://github.com/SonarSource/eslint-plugin-sonarjs) is very easy to contribute, but the SonarSource team has abandoned that repo and started maintaining eslint-plugin-sonarjs@2 in another repo (https://github.com/SonarSource/SonarJS) instead. The SonarJS repo has a very bad code structure and a poor infrastructure.

@vdiez
Copy link

vdiez commented Sep 30, 2024

Hello all,

thanks, @karlhorky, for the heads up. We are aware of the issues raised here.

Just to share some insights about why we migrated from the eslint-plugin-sonarjs repo to SonarJS:

  • the ESLint plugin was just a small subset of all the available rules in SonarJS
  • All new rules went directly to SonarJS, making the ESLint plugin feel kind of abandoned
  • The ESLint plugin was also imported by SonarJS, so unifying all rules in a single place made sense for us.

All the ESLint rules now sit under this folder. We are still in the process of improving v2 after this initial and drastic change, but hopefully, once we are in a stable situation, the ESLint plugin from Sonar will be a much better package than v1.

Now eslint-plugin-sonarjs v2 contains all rules from SonarJS (~300 rules instead of ~30 from v1). That was our initial goal.

Things that we have on our list:

  • A changelog needs to be put in place for the ESLint plugin.
  • S125 should use the parser from the ESLint config, instead of a built-in Babel dependency. This rule is the main offender for the dependency problem.
  • As mentioned here, the code structure is not what a public NPM package should look like. SonarJS is a Java project embedding a Node.js program (which runs ESLint). We are investigating ways to improve this.

Thanks

Edit: Some background about this change: https://community.sonarsource.com/t/will-sonarjs-rules-be-available-in-eslint-plugin-sonarjs/42955/6

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

No branches or pull requests

3 participants