import-js / eslint-plugin-import

ESLint plugin with rules that help validate proper imports.
MIT License
5.49k stars 1.57k forks source link

Some rules unnecessarily slow because of unneeded creation of import map #2953

Open jorisw opened 8 months ago

jorisw commented 8 months ago

Hi,

While investigating the performance of our linter setup, I ran a TIMING=1 type performance scan of my linter rules, and found that unfortunately, when any of the used rules needs an import map, it will look like the first rule who uses it, takes 90+% of the work. Example:

> DEBUG=eslint:cli-engine TIMING=1 yarn lint --debug --verbose

All files pass linting.

Rule                              | Time (ms) | Relative
:---------------------------------|----------:|--------:
import/namespace                  | 10247.586 |    98.0%
import/no-cycle                   |   114.264 |     1.1%
@nx/enforce-module-boundaries     |    30.531 |     0.3%
import/no-unresolved              |    10.439 |     0.1%
@typescript-eslint/no-unused-vars |     6.393 |     0.1%

eslint-plugin-import seems to suffer from this most, in fact, when I disable all import/* rules, the above issue goes away: the time spent between rules is more balanced. Example:

Rule                              | Time (ms) | Relative
:---------------------------------|----------:|--------:
@nx/enforce-module-boundaries     |    16.948 |    22.5%
@typescript-eslint/no-unused-vars |    13.912 |    18.4%
jest/no-duplicate-hooks           |     2.725 |     3.6%
sonarjs/cognitive-complexity      |     1.869 |     2.5%
sonarjs/no-gratuitous-expressions |     1.668 |     2.2%

Discussing with the ESLint maintainers how I might overcome the misbalance in reported time spent, one notable statement came back regarding eslint-plugin-import:

Overall, eslint-plugin-import rules always try to resolve every import even when they don't need to (looking at you, no-unused-imports), and that is, in fact, what makes them slow. The maintainers seem to think that other plugins/rules will prepopulate the import map for them, but that's just not the case in the current ecosystem.

Would you say this is accurate? Could the the rules who don't need the import map, be made not to trigger the building of one? Or the other way around? Granted I don't fully understand the implication, or its implications.

ljharb commented 8 months ago

That doesn't sound accurate to me; where was that statement made?

The only thing that populates the export map is one of the rules that uses it, so yes, currently any usage of these rules will cause one of them to be very slow (because eslint lacks a "prep" stage/phase where this could be done when needed).

The rules that do not need the export map already do not trigger the building of one.

jorisw commented 8 months ago

where was that statement made?

In thread of mine on Discord.

soryy708 commented 3 weeks ago

How do we move forward with this?

We build a full ExportMap only in rules that use the ExportMap. Are there any rules that use ExportMap that can be optimized to avoid it?