Skip to content

Bug: Vue + TypeScript project slowdown with v8 alpha #9312

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

Closed
4 tasks done
jfrs opened this issue Jun 8, 2024 · 13 comments
Closed
4 tasks done

Bug: Vue + TypeScript project slowdown with v8 alpha #9312

jfrs opened this issue Jun 8, 2024 · 13 comments
Labels
accepting prs Go ahead, send a pull request that resolves this issue bug Something isn't working locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. vue issues relating to vue support
Milestone

Comments

@jfrs
Copy link

jfrs commented Jun 8, 2024

Before You File a Bug Report Please Confirm You Have Done The Following...

  • I have tried restarting my IDE and the issue persists.
  • I have updated to the latest version of the packages.
  • I have searched for related issues and found none that matched my issue.
  • I have read the FAQ and my problem is not listed.

Issue Description

After upgrading a fairly standard Vue + TypeScript project (ESLint from v8 to v9, and typescript-eslint from v7 to v8), with a similar configuration the linting time goes from +/- 20 seconds to 3 minutes.

The command is eslint src, where src is a directory containing only .ts and .vue files.

Old .eslintrc.json:

{
  "root": true,
  "parser": "vue-eslint-parser",
  "parserOptions": {
    "parser": "@typescript-eslint/parser",
    "project": true,
    "extraFileExtensions": [".vue"]
  },
  "extends": [
    "plugin:@typescript-eslint/strict-type-checked",
    "plugin:@typescript-eslint/stylistic-type-checked",
    "plugin:vue/vue3-recommended",
    "prettier"
  ],
  "overrides": [
    {
      "files": ["src/types/*.d.ts"],
      "rules": {
        "@typescript-eslint/no-extraneous-class": ["error", { "allowConstructorOnly": true }]
      }
    }
  ]
}

New eslint.config.js

import eslintConfigPrettier from 'eslint-config-prettier';
import pluginVue from 'eslint-plugin-vue';
import tseslint from 'typescript-eslint';

export default tseslint.config(
  ...tseslint.configs.strictTypeChecked,
  ...tseslint.configs.stylisticTypeChecked,
  ...pluginVue.configs['flat/recommended'],
  eslintConfigPrettier,
  {
    languageOptions: {
      parserOptions: {
        parser: tseslint.parser,
        projectService: true,
        extraFileExtensions: ['.vue']
      }
    }
  },
  {
    rules: {
      '@typescript-eslint/no-unused-vars': ['error', { caughtErrorsIgnorePattern: 'ignore' }]
    }
  },
  {
    files: ['src/types/*.d.ts'],
    rules: {
      '@typescript-eslint/no-extraneous-class': ['error', { allowConstructorOnly: true }]
    }
  },
  {
    ignores: ['eslint.config.js']
  }
);

Reproduction Repository Link

Private repository but I can give people access to it

Repro Steps

  1. clone the repo
  2. pnpm install
  3. pnpm lint

Versions

package version
typescript-eslint 8.0.0-alpha.29
TypeScript 5.4.5
ESLint 9.4.0
node 22.2.0
@jfrs jfrs added bug Something isn't working triage Waiting for team members to take a look labels Jun 8, 2024
@bradzacher bradzacher added the vue issues relating to vue support label Jun 8, 2024
@higherorderfunctor
Copy link
Contributor

This may be more to do with ProjectServices than anything.

There is the general eslint-plugin-import issue. I personally use import-x on my repos and just manually patched it for now.

Also, the way the vue-eslint-parser works, type checking template code destroys performance. Best suggestion I've seen right now is to send to the js parser (espree) and not use this plugin's rules.

I've poked around a little and I think every template {{ code }} block is treated as a separate file. Using tsserver's ProjectService seems to trigger a whole bunch of module resolution and type checking per file. #8835 (comment) may also be related to this behavior where files that don't seem to be part of the ProjectService's initialization (which these template blocks may or may not be) are treated as orphaned causing some overhead.

I'm pretty new to tsserver's editor API, but I'm kind of wondering if there is better way to deal with "incremental" project changes in some way to work with vue's behavior and for post initialization changes when using the vscode eslint lsp plugin.

@JoshuaKGoldberg JoshuaKGoldberg added this to the 8.0.0 milestone Jun 10, 2024
@higherorderfunctor
Copy link
Contributor

higherorderfunctor commented Jun 11, 2024

As was discovered on #8835 (comment), moving service.setHostConfiguration to be set once, probably with the ProjectService, seems like low hanging fruit. Calling it on every file triggers a full project reload and cancels previous synchronizations which spirals out of control with vue template blocks each getting treated as a file.

@higherorderfunctor
Copy link
Contributor

I have #9336 ready to go if the maintainers agree with this solution. Thanks!

@JoshuaKGoldberg
Copy link
Member

This is fantastic, thanks so much for the deep dives & splitting out @higherorderfunctor!

I'm in-and-out this week and the next for conferences and trying to find the time to get to this ASAP. Apologies if there are delays between reviews - this is high priority IMO.

@JoshuaKGoldberg
Copy link
Member

Oh and @higherorderfunctor - could you please send me access? My email's on my GitHub profile. I'm happy to sign NDAs / whatever other standard paperwork 😄

@higherorderfunctor
Copy link
Contributor

higherorderfunctor commented Jun 13, 2024

@JoshuaKGoldberg are you looking for access to a test repo or access to my fix branch: https://github.com/higherorderfunctor/typescript-eslint/tree/patches2upstream?

Assuming the former. We are using on-prem Intranet only enterprise GitLab. I'll see if I can produce a repo though to play with.

@jfrs
Copy link
Author

jfrs commented Jun 13, 2024

@higherorderfunctor I'm not really able to comment on the fix but the investigation was fantastic, well done!

@JoshuaKGoldberg If you meant access to my repo I can add you to it?

@higherorderfunctor
Copy link
Contributor

higherorderfunctor commented Jun 13, 2024

I setup an example repo that combines effect, vuejs/docs, and AllkindsICP (random repo I found but it uses effect which is heavy on complex types). I've connected them with project references in the root of the repo. I tossed in some vue recommend plugin rules to give it something to churn on. This closely mirrors my work repository which is Effect on the backend with Effect/Vue on the front-end with a very similar monorepo structure using project references.

const vueRules = overrideWith(vueOverrides, [
  ...tsRules,
  ...vueRecommended,
  ...compat.extends("@vue/eslint-config-typescript/recommended"),
  ...compat.extends("plugin:vuetify/recommended"),
  ...vueA11yPlugin.configs["flat/recommended"],
  ...compat.extends("@vue/eslint-config-prettier"),
  { rules: { "codegen/codegen": "off" } } // doesn't work with vue
])

The repo is located at (note the branch if cloning): https://github.com/higherorderfunctor/effect/tree/chore/eslint-testing

  • I converted the effect's eslint config to be flat.
  • It has two patches I added, one for eslint-plugin-import via eslint-module-utils for not filtering projectService and one for the effect's in-house plugin @effect/eslint-plugin which did not play nicely with flat configs.
  • It is preconfigured to the sum of my patches for @typescript-eslint/* via package.json which I haven't put all the PRs in for yet: https://github.com/higherorderfunctor/typescript-eslint/tree/patches2upstream. It will require pnpm@9 (for git deps) to play with. You might have to adjust the specific version of pnpm at packageManager from 9.1.1 to say 9.1.4 or whatever you have installed.
      "@typescript-eslint/types": "higherorderfunctor/typescript-eslint#patches2upstream&path:packages/types",
      "@typescript-eslint/typescript-estree": "higherorderfunctor/typescript-eslint#patches2upstream&path:packages/typescript-estree",

To run some tests:

# install deps
pnpm i
# AllkindsICP
/etc/profiles/per-user/caubut/bin/time -v pnpm exec eslint 'packages/AllkindsICP/{tests,src}/**'

✖ 11479 problems (9513 errors, 1966 warnings)
  5626 errors and 1953 warnings potentially fixable with the `--fix` option.

        Elapsed (wall clock) time (h:mm:ss or m:ss): 1:03.49
        Maximum resident set size (kbytes): 2818164
# vue-docs
/etc/profiles/per-user/caubut/bin/time -v pnpm exec eslint 'packages/vue-docs/{tests,src}/**'

✖ 1668 problems (1569 errors, 99 warnings)
  890 errors and 93 warnings potentially fixable with the `--fix` option.

        Elapsed (wall clock) time (h:mm:ss or m:ss): 0:27.11
        Maximum resident set size (kbytes): 1536552
# whole repo
/etc/profiles/per-user/caubut/bin/time -v pnpm lint

✖ 19186 problems (17101 errors, 2085 warnings)
  7551 errors and 2066 warnings potentially fixable with the `--fix` option.

        Elapsed (wall clock) time (h:mm:ss or m:ss): 12:46.26
        Maximum resident set size (kbytes): 4511236

To undo my combined @typescript-eslint/* patches and just use rc-v8, change to these overrides in package.json. Note that I am leaving the eslint-plugin-import patch; on my work repo it represented a 4-5x speedup.

      "@typescript-eslint/types": "$typescript-eslint",
      "@typescript-eslint/typescript-estree": "$typescript-eslint",
# re-install deps
pnpm i
# AllkindsICP
/etc/profiles/per-user/caubut/bin/time -v pnpm exec eslint 'packages/AllkindsICP/{tests,src}/**'
✖ 11479 problems (9513 errors, 1966 warnings)
  5626 errors and 1953 warnings potentially fixable with the `--fix` option.

        Elapsed (wall clock) time (h:mm:ss or m:ss): 36:32.52
        Maximum resident set size (kbytes): 2894116
# vue-docs
/etc/profiles/per-user/caubut/bin/time -v pnpm exec eslint 'packages/vue-docs/{tests,src}/**'

✖ 1668 problems (1569 errors, 99 warnings)
  890 errors and 93 warnings potentially fixable with the `--fix` option.

        Elapsed (wall clock) time (h:mm:ss or m:ss): 15:38.62
        Maximum resident set size (kbytes): 1939532
# whole repo
/etc/profiles/per-user/caubut/bin/time -v pnpm lint

  # not even going to try

vue-docs had a 97.11% decrease or 34.60x speed up.
AllkindsICP had a 97.10% decrease or 34.48x speed up.
The whole repo took 12m46s with the patches. I'm not even going to try unpatched. We can guess it will be around ~2h23m, maybe faster because not all packages use the vue plugins, but I'm still not excited to try, lol.

Below is everything in my combined patch:

  • Fixes extends not getting processed by the default config used for projectService inferred projects. fix(typescript-estree): adds support for project services using extended config files #9306
  • Adds tsserver logging. feat(typescript-estree): exposes ProjectService logs through the plugin #9337
  • Minimizes how many times setHostConfiguration is called. The majority, if not all, of the performance gain comes from this fix. fix(typescript-estree): only run projectService setHostConfiguration when needed #9336
  • Adds support to use watches with a Trie lookup (without actual FS watching) to update tsserver with changes (e.g., file created). This fixes adding a new file and it not getting associated with the project. There is also some overlap with the existing createWatchProgram implementation. I am pretty busy in the coming week so I don't know if I will be able to deep dive into this until the last week of June. This may only impact LSPs (or I suppose something like eslint_d). fix(typescript-estree): adds project service watch to add new files to project #9353
  • Caches open files with tsserver and closes them using an LRU based on access order. I did not see any measurable performance on the CLI. In my dive into tsserver, it looked like opened files were treated differently for project updates. I'm speculating this may have some impact when run as an LSP where files are created, deleted, and moved but never closed. I'll probably have to setup some mock LSP clients and see if I can measure what the latency actually looks like.
  • Sends incremental diffs instead of re-opening files if already open. Same as above. Didn't see a lot of different on the CLI. More testing and measuring needed to justify.

@JoshuaKGoldberg I planned to break up into several PRs, but due to the time sensitive nature, I can just submit everything in one. That said, I haven't gotten around to writing tests yet for the ones without a PR. Updated my summary above. The last two I don't think I can justify to propose just yet.

@higherorderfunctor
Copy link
Contributor

higherorderfunctor commented Jun 13, 2024

This is the current sum of all changes: https://github.com/higherorderfunctor/typescript-eslint/pull/1/files

It is using the branch: https://github.com/higherorderfunctor/typescript-eslint/tree/patches2upstream-no-build-artifacts

This branch is my working branch with the changes but also build artifacts and some pnpm hacks: https://github.com/higherorderfunctor/typescript-eslint/tree/patches2upstream

I'll try and keep my fork's v8 branch up to do date and make sure changes are applied to both patches2upstream and patches2upstream-no-build-artifactso the diff stays up to date.

@JoshuaKGoldberg
Copy link
Member

@JoshuaKGoldberg are you looking for access to a test repo or access to my fix branch

I'm mainly looking for test repos: i.e. I want to see the reproductions. I'd like to play around with as many as possible to understand them. Especially minimum reproductions that don't have a ton of extra stuff.

#9312 (comment) is an excellent example of this, thanks!

cc @jfrs, anything you can send would be great too!

planned to break up into several PRs

Heh, yeah, that'd definitely be preferable for us. If you don't have time then we can help with splitting up - but very happy to see that you did send a bunch split out. Thanks! 🙌

This issue has some of the best reproductions & reporting from multiple people we've gotten this year. Love it!!

@JoshuaKGoldberg JoshuaKGoldberg added accepting prs Go ahead, send a pull request that resolves this issue and removed triage Waiting for team members to take a look labels Jun 17, 2024
@jfrs
Copy link
Author

jfrs commented Jun 17, 2024

cc @jfrs, anything you can send would be great too!

I've added you to the repository!

2 branches, main is ESLint v8, and eslint-v9 is unsurprisingly ESLint v9, but in both cases pnpm i and pnpm lint should be all you need.

Let me know if there's anything else I can do 😃

@JoshuaKGoldberg
Copy link
Member

JoshuaKGoldberg commented Jun 22, 2024

#9336 was merged into v8, so marking this issue as closed. But linking to #7906 -> #8031 to note that I'll want to set up performance comparisons for Vue users too. I haven't checked the repros for additional regressions (in a bit of a rush this month). Please do post a new issue if your specific one is still noticeably slower and I haven't gotten to the perf comparisons myself yet.

Thanks again all! ❤️

@jfrs
Copy link
Author

jfrs commented Jun 25, 2024

I just realised that I was looking at the wrong output for the time command, so the numbers were a big inflated sorry.
The somewhat updated benchmark for my project is:

eslint typescript-eslint time
8.57.0 7.14.1 ±12 seconds 👍
9.5.0 8.0.0-alpha.30 ±2 minutes 30 seconds 😕
9.5.0 8.0.0-alpha.32 ±15 seconds! 🎉

Thanks to everyone involved, this is great!

@github-actions github-actions bot added the locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. label Jul 3, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepting prs Go ahead, send a pull request that resolves this issue bug Something isn't working locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. vue issues relating to vue support
Projects
None yet
Development

No branches or pull requests

4 participants