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

build: disable promise/no-multiple-resolved #1534

Merged
merged 1 commit into from
Oct 28, 2023
Merged

build: disable promise/no-multiple-resolved #1534

merged 1 commit into from
Oct 28, 2023

Conversation

sjinks
Copy link
Member

@sjinks sjinks commented Oct 20, 2023

Description

This PR disables the promise/no-multiple-resolved rule because it causes major slowdown for the Lint job.

$ TIMING=1 DEBUG=eslint:linter npx eslint . --ext ts
...
  eslint:linter Linting code for /home/volodymyr/automattic/vip-cli/src/lib/site-import/status.ts (pass 1) +1ms
  eslint:linter Verify +0ms
  eslint:linter With ConfigArray: /home/volodymyr/automattic/vip-cli/src/lib/site-import/status.ts +0ms
  eslint:linter Parsing: /home/volodymyr/automattic/vip-cli/src/lib/site-import/status.ts +0ms
  eslint:linter Parsing successful: /home/volodymyr/automattic/vip-cli/src/lib/site-import/status.ts +17ms
  eslint:linter Scope analysis: /home/volodymyr/automattic/vip-cli/src/lib/site-import/status.ts +0ms
  eslint:linter Scope analysis successful: /home/volodymyr/automattic/vip-cli/src/lib/site-import/status.ts +0ms
  eslint:linter Generating fixed text for /home/volodymyr/automattic/vip-cli/src/lib/site-import/status.ts (pass 1) +5m
...
Rule                                       |  Time (ms) | Relative
:------------------------------------------|-----------:|--------:
promise/no-multiple-resolved               | 302949.116 |    98.5%
prettier/prettier                          |   1762.054 |     0.6%
@typescript-eslint/no-misused-promises     |    683.756 |     0.2%
import/no-duplicates                       |    325.211 |     0.1%
@typescript-eslint/no-floating-promises    |    294.379 |     0.1%
@typescript-eslint/no-unsafe-argument      |    267.663 |     0.1%
@typescript-eslint/no-unsafe-return        |    158.369 |     0.1%
@typescript-eslint/no-unsafe-assignment    |    145.742 |     0.0%
security/detect-bidi-characters            |    105.555 |     0.0%
@typescript-eslint/no-unsafe-member-access |     97.572 |     0.0%

It took 5 minutes(!) to lint src/lib/site-import/status.ts, and the slowest rule was promise/no-multiple-resolved.

This is most likely related to another rule we suppress:

vip-cli/src/lib/site-import/status.ts
  264:24  error  Async arrow function has a complexity of 25. Maximum allowed is 20  complexity

The function is too complex for eslint-plugin-promise to analyze it (or maybe its analysis algorithm is inefficient).

Unfortunately, /* eslint-disable promise/no-multiple-resolved */ does not turn off the rule; it just disables all diagnostics from that rule.

Steps to Test

CI should pass.

@sjinks sjinks self-assigned this Oct 20, 2023
@sjinks
Copy link
Member Author

sjinks commented Oct 20, 2023

@abdullah-kasim this may be of interest for you :-)

@sjinks
Copy link
Member Author

sjinks commented Oct 20, 2023

Was: CI / Lint (pull_request) Successful in 8m
Now: CI / Lint (pull_request) Successful in 59s

@abdullah-kasim
Copy link
Contributor

@Automattic/vip-connoisseurs @chriszarate y'all might be interested in this one too

Amazing research by the way! Never thought of investigating it in that manner.

@sjinks sjinks merged commit 02fb7fe into trunk Oct 28, 2023
9 checks passed
@sjinks sjinks deleted the speedup/linting branch October 28, 2023 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants