chore(eslint-plugin-sonarjs): add missing @typescript-eslint/parser dependency

[rtritto]

For eslint-plugin-sonarjs, fix issue missing transitive peer dependency @typescript-eslint/parser for @typescript-eslint/eslint-plugin.

After eslint-plugin-sonarjs is installed, with yarn explain peer-requirements:

p72bb8 → ✓ eslint-plugin-sonarjs@npm:2.0.2 [0f3ca] provides @babel/core@npm:7.24.3 to @babel/eslint-parser@npm:7.24.1 [49e17] and 92 other dependencies
p6eeed → ✓ eslint-plugin-sonarjs@npm:2.0.2 [0f3ca] doesn't provide @types/babel__core to @babel/eslint-parser@npm:7.24.1 [49e17] and 92 other dependencies
p5afe6 → ✓ eslint-plugin-sonarjs@npm:2.0.2 [0f3ca] doesn't provide @types/typescript to @typescript-eslint/eslint-plugin@npm:7.16.1 [49e17] and 5 other dependencies
peb354 → ✓ eslint-plugin-sonarjs@npm:2.0.2 [0f3ca] doesn't provide @types/typescript-eslint__parser to @typescript-eslint/eslint-plugin@npm:7.16.1 [49e17] and 2 other dependencies
p6c73a → ✘ eslint-plugin-sonarjs@npm:2.0.2 [0f3ca] provides @typescript-eslint/parser@npm:8.6.0 [0f3ca] to @typescript-eslint/eslint-plugin@npm:7.16.1 [49e17] and 2 other dependencies
pb1c15 → ✓ eslint-plugin-sonarjs@npm:2.0.2 [0f3ca] provides typescript@patch:typescript@npm%3A5.5.4#optional!builtin<compat/typescript>::version=5.5.4&hash=379a07 to @typescript-eslint/eslint-plugin@npm:7.16.1 [49e17] and 5 other dependencies

There is an article written by author of yarn package manager: Implicit Transitive Peer Dependencies

Note

Please run npm i --lockfile-version 3 to update packages\jsts\src\rules\package-lock.json

[ericmorand-sonarsource]

Hello @rtritto ,

If I'm not mistaken, the eslint-plugin-sonarjs package doesn't depend on the @typescript-eslint/parser. There is no reason to add it as a dependency - or even a peer dependency - since the plugin never imports @typescript-eslint/parser.

It may be imported by one of the plugin's direct or indirect dependencies, though, but we can't know about that.

The article that you mention makes a statement that is quite debatable and partially wrong:

TL;DR: If you write a package that depends on Foo, and if Foo has a peer dependency, you must provide it in either the dependencies or peer dependencies fields.

If you follow this principle, it means that you have to add a dependency for every peer dependencies of your own dependencies, direct or indirect. This is not only practically impossible (it would mean for example that we have to add react as a dependency of our plugin, among many others), but also wrong: direct dependencies are just that, direct dependencies that your package depends on.

You won't "implicitly inherit" the peer dependencies declared in Foo.

This is not true, at least for npm, since version 7: https://docs.npmjs.com/cli/v10/configuring-npm/package-json#peerdependencies

In npm versions 3 through 6, peerDependencies were not automatically installed, and would raise a warning if an invalid version of the peer dependency was found in the tree. As of npm v7, peerDependencies are installed by default.

That yarn doesn't implement such a feature is a design choice of yarn's team. For the record, let me say that I believe yarn is right there, and that npm is wrong: peer dependencies should not be installed by default because they are meant to remain under strict control of the consumer of the package: i.e., that we have to install manually all peer dependencies is exactly why the concept of peer dependencies exist.

On a side note, one of the consequences of npm@7+ behaviour is that kind of issue: npm/cli#7057. That yarn decided to remain true to the concept of peer dependencies is a relief.

[rtritto]

If you follow this principle, it means that you have to add a dependency for every peer dependencies of your own dependencies, direct or indirect. This is not only practically impossible (it would mean for example that we have to add react as a dependency of our plugin, among many others), but also wrong: direct dependencies are just that, direct dependencies that your package depends on.

With a plugin, you can usually have a peer dependency. Imply that when when you add that plugin as dependency you must also add the peer dependency as dependency or dev dependency or peer dependency.
Another use case is when a plugin have an optional peer dependency (litsted in peerDependencyMeta as optional=true). Imply that is you can choose to install that peer dependency if it's needed and the plugin check and use that peer dependency.

With any (updated) package manager, if you do a clean install of eslint-plugin-sonarjs and check logs, you should find a warning where eslint-plugin-sonarjs have missed the @typescript-eslint/parser dependency.
That's because if you want use @typescript-eslint/eslint-plugin you need also to install the @typescript-eslint/parser and eslint dependencies.

As source you can see the related packages:

• [email protected]/package.json

    "dependencies": {
      "@typescript-eslint/eslint-plugin": "7.16.1"
    }

• @typescript-eslint/[email protected]/package.json

    "peerDependencies": {
      "@typescript-eslint/parser": "^8.0.0 || ^8.0.0-alpha.0",
      "eslint": "^8.57.0 || ^9.0.0"
    },

Edit:
Note: if the @typescript-eslint/parser dependency isn't added to eslint-plugin-sonarj, @typescript-eslint/parser is installed anyway (you can check the dependencies folder or tree in lock file).

Edit2:
The @typescript-eslint dependency stack (eslint-plugin, parser, utils) can be replaced with typescript-eslint dependency (preferred to configure ESLint v9)

[ericmorand-sonarsource]

@rtritto

With any (updated) package manager, if you do a clean install of eslint-plugin-sonarjs and check logs, you should find a warning where eslint-plugin-sonarjs have missed the @typescript-eslint/parser dependency.

This is true only for package managers outside of npm. These other package managers made this decision to warn the user that some dependencies need to be installed manually, and, even though I personally think that this behavior is perfectly legit, it is a design decision that does not make npm outdated or less relevant. Actually, the npm decision to install peer dependencies automatically is perfectly suited to their wider and more casual audience.

The @typescript-eslint dependency stack (eslint-plugin, parser, utils) can be replaced with typescript-eslint dependency (preferred to configure ESLint v9)

I'm intrigued. Can you please say more about this?

We will close the PR itself: We think that npm and yarn both behave correctly: npm installs the peer dependencies; yarn clearly explains what needs to be installed manually to make the plugin work. As I explained, declaring a transitive peer dependency as a direct peer dependency means that we also have to do the same for every other transitive peer dependency, recursively, which is impracticable.

[rtritto]

npm and yarn both install missing peer dependencies and both warnings.
eslint-plugin-sonarjs isn't compliant because of missing @typescript-eslint/parser dependency (transitive peer dependency).
If a project uses a dependency with peer dependencies, in that project you have to add that peer dependencies as dependencies or peer dependencies.

[ericmorand-sonarsource]

Maybe I'm missing something: what problem do you actually encounter? Is the plugin not working correctly when installed using yarn or another package manager?

[rtritto]

With auto-install-peer=false (some package managers use default as true), all package manager fails.
By now, you should accept this PR.

The @typescript-eslint dependency stack (eslint-plugin, parser, utils) can be replaced with typescript-eslint dependency (preferred to configure ESLint v9)

I'm intrigued. Can you please say more about this?

ESLint v9 uses flat configs as default (https://eslint.org/blog/2024/04/eslint-v9.0.0-released/#flat-config-is-now-the-default-and-has-some-changes), so to improve (and reduce complexity) settings of typescript-eslint, it's preferred to replace the typescript-eslint-* stack (eslint-plugin, parser, utils) with typescript-eslint (it uses that stack).
Just check the doc: https://typescript-eslint.io/getting-started