`eslint-plugin-sonar` size

Rules and Languages
1

I’m developing an ESLint sharable config that uses eslint-plugin-sonarjs.
I’ve noticed that eslint-plugin-sonar depends on jsx-ast-utils, a project by jsx-eslint. jsx-eslint depends on lots of polyfills that significantly bloat the size of the plugin. Looking through the source, it looks like depending on jsx-ast-utils was a temporary measure back in 2021 until a new release of of eslint-plugin-jsx-a11y was released including e6bfd5c. Now that jsx-eslint is back to releasing, would it be possible to drop jsx-ast-utils again? Notably, other rules have been added that use jsx-ast-utils, but perhaps it’d be possible to inline the used functions, it just looks like one or two?

3

Hello @lishaduck,

thanks for sharing this.

I agree that the source you pointed to needs to be updated. However, I don’t think removing our dependency to jsx-ast-utils would have any benefit on our bundle size, as eslint-plugin-jsx-a11y still depends on it, so it would still be a transitive dependency for our plugin. We can probably just change our dependency version to match always the version used by eslint-plugin-jsx-a11y, that would avoid having more than one version installed. What do you think?

I created a ticket to handle this.

Thanks again!

1 Like
6

Cool, thanks!

Do y’all depend on eslint-plugin-jsx-a11y though? What I see on eslint-plugin-sonarjs - npm doesn’t show that. Is it a transitive?
Also, as I look closer, you’ve got a dependency on TypeScript. Could that be moved to a peer (like the rest of the ecosystem) to force deduplication there (and possibly with a >=5 instead of ^5 because TS doesn’t follow SemVer)?

7

Hi @lishaduck,

sorry, you are absolutely right. The ESLint plugin would not include eslint-plugin-jsx-a11y. SonarJS does, that’s why I mixed both things. So removing our dependency on jsx-ast-utils would indeed affect the package size. I’ll update the ticket.

About TypeScript, it is an actual dependency as we import and use TypeScript API directly in some of the rules, so it does not fit the definition of a peer dependency. Of course, the rules using typescript will not be executed if the ESlint plugin user does not use TypeScript or does not specify a tsconfig.json. We are very aware that TS is the biggest offender in terms of package size, and we would be happy to find a better solution than the current one (probably optionalDependency?).

About the actual version, we used >=5 because it is wide enough for the big majority of users to avoid another version being installed aside of the user’s TS.

8

sorry, you are absolutely right. The ESLint plugin would not include eslint-plugin-jsx-a11y. SonarJS does, that’s why I mixed both things. So removing our dependency on jsx-ast-utils would indeed affect the package size. I’ll update the ticket.

Alright, thank you!

About TypeScript, it is an actual dependency as we import and use TypeScript API directly in some of the rules, so it does not fit the definition of a peer dependency.

Wait wait wait. You can’t import peers? I need to go see how much of my code is broken lol.
No, it looks like you can import a peerDependency, and you want to make sure you use the version the user is so that they stay in sync, it’s kinda like ESLint is a “ts plugin” (except that that’s an actual thing).

Of course, the rules using typescript will not be executed if the ESlint plugin user does not use TypeScript or does not specify a tsconfig.json. We are very aware that TS is the biggest offender in terms of package size, and we would be happy to find a better solution than the current one (probably optionalDependency?).

Unfortunately, optionalDependencies are installed by default so most projects use optional peerDependencies instead (like I was suggesting)

About the actual version, we used >=5 because it is wide enough for the big majority of users to avoid another version being installed aside of the user’s TS.

Oh, I was saying that it’s not >=5 when it probably should be, especially with ts6 coming soon. It’s currently ^5.

Thanks for bearing with me!

1 Like
9

peerDependencies are also installed by npm (yarn does not, which I would consider a more correct approach). From my previous link:

As of npm v7, peerDependencies are installed by default.

We had some discussions internally, and at the moment we consider to set TS as dependency as more correct approach. Usually ESLint plugins would define ESLint as a peer dependency, as they do not use any ESLint API (but ESLint loads them instead). This is not applicable for our usecase of TS.

As for TS >=5, got it. I agree with you that’s a better alternative to ^5. I created a PR to fix that.

Thank you!

1 Like
10

peerDependencies are also installed by npm (yarn does not, which I would consider a more correct approach).

Oh, maybe I unclear, I was referring to the peerDependenciesMeta.*.optional field. From package.json | npm Docs,

The peerDependenciesMeta field serves to provide npm more information on how your peer dependencies are to be used. Specifically, it allows peer dependencies to be marked as optional. Npm will not automatically install optional peer dependencies. This allows you to integrate and interact with a variety of host packages without requiring all of them to be installed.

We had some discussions internally, and at the moment we consider to set TS as dependency as more correct approach.

Last data point and then I’ll let you alone to run your own plugin :grin::

11

Hello @lishaduck,

I did not know that peerDependenciesMeta field. That is very interesting! Thanks for the advice, we may consider changing TS to that instead. I created a ticket to study this alternative for the next release.

Thanks again!

2 Likes