typescript-eslint / typescript-eslint

:sparkles: Monorepo for all the tooling which enables ESLint to support TypeScript
https://typescript-eslint.io
Other
15.13k stars 2.71k forks source link

⚡️ Performance: Default parserOptions.jsDocParsingMode to 'none' or 'type-info' #9857

Open JoshuaKGoldberg opened 3 weeks ago

JoshuaKGoldberg commented 3 weeks ago

Overview

Following conversations in #7997 and https://github.com/typescript-eslint/typescript-eslint/pull/9783#discussion_r1725038605: we'd previously added a parserOptions.jsDocParsingMode that API consumers can use to skip parsing JSDoc comments. JSDoc comment parsing can be quite expensive.

We'd previously been blocked from doing so because at least one popular ecosystem project, eslint-plugin-deprecation, uses them. Our initial version of a deprecation rule does too: #9783. So this issue is blocked until we create a suitable alternative, such as our own quick regexp parser.

See also #7906 for tracking running a performance comparison.

💖

uhyo commented 3 weeks ago

Hello, coming over from an X thread. I maintain another third-party plugin that relies on parsed JSDoc information, though not very popular. 😅

https://github.com/uhyo/eslint-plugin-import-access

The plugin sees JSDoc tags @package, @private and @access and uses this information to decide whether importing a binding form another module is allowed. Also it depends on TypeScript's module resolution because the plugin has an inter-module linting logic.

I'm not severly requesting to keep the slower jsDocParsingMode default; I'm happy to guide my users to manually set jsDocParsingMode in order to make my plugin work.

I just wanted to share my use case of JSDoc parsing and also my humble hope that the planned alternative JSDoc parser (#9858) could be extended so that it can be used for third-party use cases too. Thank you 🙂

silverwind commented 2 weeks ago

Is this the reason why @typescript-eslint/no-deprecated is slow?


Rule                                   | Time (ms) | Relative
:--------------------------------------|----------:|--------:
@typescript-eslint/no-deprecated       |  1434.011 |    32.5%
deprecation/deprecation                |   140.861 |     3.2%
Josh-Cena commented 2 weeks ago

@silverwind The first type-aware rule that gets loaded is always significantly slower than the others because it has to retrieve uncached type info. You probably need to run with other type-aware lint rules too to compare apples to apples.

(@typescript-eslint/triage-team I actually think this has been mentioned many times but was never put into the performance FAQ)

bradzacher commented 2 weeks ago

@silverwind please don't hop on unrelated threads to ask questions! This issue is about changing the default for parserOptions.jsDocParsingMode, not about the new no-deprecated rule. If you would like to ask questions -- please use our discord.

But Josh is right -- the first rule bears the brunt of the type info that gets lazily computed and cached. If you isolated the test runs you'd see they should be equivalent perf.

Josh-Cena commented 2 weeks ago

This issue is about changing the default for parserOptions.jsDocParsingMode, not about the new no-deprecated rule. If you would like to ask questions -- please use our discord.

This issue is motivated by the new rule, because we are now in the business of checking JSDoc. :)

silverwind commented 2 weeks ago

@Josh-Cena thanks for clarification, I will run my benchmark on these rules in isolation so that each rule has to bear the jsDoc loading cost. I expect the two rules to perform similarily under those conditions.

bradzacher commented 2 weeks ago

If you find it's slower - please file a new issue!