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

Bug: eslint-plugin-import is slow when giant files such as typescript.js are included #2416

Closed
kaiyoma opened this issue Mar 26, 2022 · 11 comments

Comments

@kaiyoma
Copy link
Contributor

kaiyoma commented Mar 26, 2022

I initially filed this bug against eslint (eslint/eslint#15728), but that was analyzed and closed and I was told to file against typescript-eslint instead. I did that (typescript-eslint/typescript-eslint#4735) and after some thorough analysis by the maintainers, they concluded that eslint-plugin-import is the culprit, so posting here.

Environment

Node version: v16.13.0
npm version: v8.1.4
Local ESLint version: v8.10.0 (Currently used)
Global ESLint version: Not found
Operating System: win32 10.0.19044

What parser are you using?

@typescript-eslint/parser

What did you do?

Will post separately (GitHub is complaining about the content length).

What did you expect to happen?

For eslint to run in a timely manner.

What actually happened?

eslint is taking almost 2 minutes to lint half a dozen files, none of which are big. In the debug log (posted below), I can see that it's taking over 45 seconds (each) for two particular files to be linted, and neither one is more than 200 lines.

Additional comments

Will post separately (GitHub is complaining about the content length).

@kaiyoma
Copy link
Contributor Author

kaiyoma commented Mar 26, 2022

For the eslint config, code that will repro the problem, debug logs, and an in-depth analysis of why this plugin is to blame, see the previous issue (typescript-eslint/typescript-eslint#4735).

@burtek
Copy link

burtek commented Jul 26, 2022

Same here, working on an eslint plugin, imported typescript and it takes ages to lint a single file:

$ TIMING=1 yarn eslint src/eslint-utils/isMethodCalledOnArray.ts --fix-dry-run

[REDACTED]/src/eslint-utils/isMethodCalledOnArray.ts
  10:23  warning  No magic number: 1  @typescript-eslint/no-magic-numbers
  13:23  warning  No magic number: 2  @typescript-eslint/no-magic-numbers
  16:23  warning  No magic number: 3  @typescript-eslint/no-magic-numbers

✖ 3 problems (0 errors, 3 warnings)

Rule                                   | Time (ms) | Relative
:--------------------------------------|----------:|--------:
import/no-deprecated                   | 27330.375 |    38.6%
import/namespace                       | 25952.951 |    36.6%
import/no-cycle                        | 17408.762 |    24.6%
@typescript-eslint/naming-convention   |    42.714 |     0.1%
n/no-extraneous-import                 |    13.045 |     0.0%
n/no-unpublished-import                |     9.993 |     0.0%
@typescript-eslint/indent              |     9.372 |     0.0%
@typescript-eslint/no-misused-promises |     7.999 |     0.0%
import/order                           |     5.278 |     0.0%
import/no-duplicates                   |     4.737 |     0.0%
Done in 74.57s.

@JounQin
Copy link
Collaborator

JounQin commented Jul 26, 2022

Can you try https://github.com/un-es/eslint-plugin-i to see whether it's much faster?

@JounQin
Copy link
Collaborator

JounQin commented Jul 26, 2022

duplicate of #2348

@JounQin JounQin closed this as not planned Won't fix, can't repro, duplicate, stale Jul 26, 2022
@burtek
Copy link

burtek commented Jul 26, 2022

@JounQin give me a few minutes, will let you know

@burtek
Copy link

burtek commented Jul 26, 2022

@JounQin it is faster, but not much (I only replaced the 3 longest taking rules as I'm using shared config so it'd be hard to replicate all of it and replace import with i)

Rule                                   | Time (ms) | Relative
:--------------------------------------|----------:|--------:
i/no-deprecated                        | 19176.082 |    43.6%
i/namespace                            | 16235.055 |    36.9%
i/no-cycle                             |  8471.943 |    19.2%
@typescript-eslint/naming-convention   |    37.586 |     0.1%
n/no-extraneous-import                 |     8.199 |     0.0%
@typescript-eslint/indent              |     7.042 |     0.0%
@typescript-eslint/no-misused-promises |     6.244 |     0.0%
n/no-unpublished-import                |     5.568 |     0.0%
import/order                           |     2.679 |     0.0%
import/no-duplicates                   |     1.913 |     0.0%
Done in 48.12s.
eslint config that replaces the `import` plugin:
{
    "extends": "<internally-shared-config>",
    "parserOptions": {
        "project": "./tsconfig.json"
    },
    "plugins": ["i"]
    "rules": {
        // https://github.com/import-js/eslint-plugin-import/issues/2416
        "import/no-deprecated": "off",
        "import/namespace": "off",
        "import/no-cycle": "off",
        "i/no-deprecated": "warn",
        "i/namespace": "error",
        "i/no-cycle": "error"
    }
}

@JounQin
Copy link
Collaborator

JounQin commented Jul 26, 2022

@burtek

npm install -D eslint-plugin-import@npm:eslint-plugin-i

It should not needed to change your eslint config at all.

@burtek
Copy link

burtek commented Jul 26, 2022

@JounQin oh, didn't know that's possible 😮

@JounQin
Copy link
Collaborator

JounQin commented Jul 26, 2022

Thanks for sharing, I'd like see if I can help to debug the performance issue tomorrow.

Let's track this issue at #2348.

@JounQin
Copy link
Collaborator

JounQin commented Jul 26, 2022

@JounQin oh, didn't know that's possible 😮

That's documented at https://github.com/un-es/eslint-plugin-i#installation 😏

@burtek
Copy link

burtek commented Jul 26, 2022

@JounQin Imagine reading docs in 2022 🙃😆

My bad

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

3 participants