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

import/no-extraneous-dependencies and import/order pretty slow #1793

Open
osdiab opened this issue Jun 1, 2020 · 6 comments
Open

import/no-extraneous-dependencies and import/order pretty slow #1793

osdiab opened this issue Jun 1, 2020 · 6 comments

Comments

@osdiab
Copy link

osdiab commented Jun 1, 2020

Performance not great when I useTIMING=1 with eslint, I don't know how these particular rules are implemented but if there's low hanging fruit to make them run quicker it'd be a boon for my project.

Screen Shot 2020-06-01 at 20 26 28

@ljharb
Copy link
Member

ljharb commented Jun 1, 2020

This is because the first import rule that needs it has to build up the dependency graph across your entire codebase. I'm open to general performance improvements, but I doubt there's low-hanging fruit.

(I'd suggest using jest-runner-eslint for a large codebase; the parallelization helps a lot)

@osdiab
Copy link
Author

osdiab commented Nov 15, 2021

@ljharb bringing this back - setting aside import/no-extraneous-dependencies, does import/order really need to build up a dependency graph? It seems like it would be able to alphabetize my imports without actually needing that knowledge.

@ljharb
Copy link
Member

ljharb commented Nov 15, 2021

@osdiab it doesn't need the entire graph, but it does need to know what kind of import each thing is, which requires some of the ExportMap to be built up.

There might be a way to only partially traverse in the order rule, and then fill that out later if other rules need the full information, but that sounds like a lot of complexity given that the rules that do require all of it are so critically important.

@thany
Copy link

thany commented Dec 1, 2021

I just ran into this issue today as well. Our project has the following timings:

Rule                              | Time (ms) | Relative
:---------------------------------|----------:|--------:
import/no-extraneous-dependencies | 15763.183 |    18.2%
import/extensions                 | 11078.163 |    12.8%
import/order                      |  9825.438 |    11.4%
deprecation/deprecation           |  8934.147 |    10.3%
import/no-cycle                   |  7922.312 |     9.2%
import/no-self-import             |  4429.868 |     5.1%
import/no-duplicates              |  4408.646 |     5.1%
import/no-named-as-default        |  3513.506 |     4.1%
import/no-named-as-default-member |  3500.599 |     4.1%
import/named                      |  1678.111 |     1.9%

The total time taken can be extrapolated to be around 86 seconds (time / relative * 100 = total). But here's the thing. I decided to disable the first two, and indeed, roughly the time neccesary for those rules are subtracted:

Rule                              | Time (ms) | Relative
:---------------------------------|----------:|--------:
import/order                      | 12254.752 |    19.6%
deprecation/deprecation           |  9173.969 |    14.6%
import/no-cycle                   |  8027.005 |    12.8%
import/no-duplicates              |  4428.442 |     7.1%
import/no-self-import             |  4406.546 |     7.0%
import/no-named-as-default        |  3487.111 |     5.6%
import/no-named-as-default-member |  3470.090 |     5.5%
import/named                      |  1708.323 |     2.7%
indent                            |  1637.275 |     2.6%
react/jsx-no-bind                 |   771.113 |     1.2%

The total time is now extrapolated as around 62 seconds, which is only a 2 seconds difference from the first set of timings, sans the first two rules. Therefor, I can conclude that, within a margin of error, disabling one or more import rules almost entirely subtracts their time taken from the duration of the lint command.

And if I may, this means the whole discussion about dependency graph being constructed, while certainly not invalid, might be turn out to be only a tiny fraction of the time taken up by the rules that appear to be the slowest. So, not really a factor in why those rules are slow - They are indeed slow, but not because of dependency graph.

But I love to be corrected if anyone has proof to the contrary. That's what science is all about 😀

@ticktockKK

This comment has been minimized.

@osdiab
Copy link
Author

osdiab commented Oct 20, 2022

So I'm bringing this back - i have a slightly different proposal. For my project I only really care about three groups - code in the current module, code from another project in my monorepo, and all other external code. I'm thinking this rule could be super fast for me if there were a flag or separate rule altogether, such that I could just decide the import group entirely via text pattern matching. For example:

  • if the imported path starts with src/ or is a relative path, it's code from my module
  • if it starts with @mycompany/ then it's part of the monorepo
  • if it starts with anything else it's external.

this way the rule wouldn't have to do any dependency analysis. Would something like this be amenable to the maintainers here?

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

No branches or pull requests

4 participants