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

Thoughts on improving performance/effect on vscode pre-commit check. #630

Closed
devinrhode2 opened this issue Oct 29, 2021 · 5 comments
Closed

Comments

@devinrhode2
Copy link
Contributor

devinrhode2 commented Oct 29, 2021

1.) Commit experience inside vscode, when running XO as pre-commit hook

Context:

I am running xo --fix with husky+lint-staged in a pre-commit git hook.
I also typically use vscode's git sidebar for committing.
I am working on migrating a monorepo from eslint to XO. I'm also updating to latest version of typescript, using tools like ts-migrate, TypeStat, and hopefully TypeWiz, with a sprinkling of my own scripts to cover some edge cases specific to our monorepo.

Poor DX inside vscode.

Due to xo --fix being so slow, there are a number of issues that crop up when committing inside vscode. vscode itself could make some improvements here, but the best solution IMO is for xo --fix to run within about 1-2 seconds for a typical commit. Feel free to skip or skim this section:

When I commit inside vscode, it takes some time, it would be really nice if vscode had a loading spinner while XO is running. Clicking the loading spinner could open up the "Git" output channel (This is what you see when pre-commit check fails, and you view the output). There could also be an "X" button to skip the pre-commit check.

Another issue, caused by XO being slow, is that we can have race conditions. Sometimes I will commit twice. Since there's no really any ui feedback indicating that the pre-commit check is running, I'll hit cmd+enter again, I'm not sure if vscode is queuing up these commit commands, but I get an error on the second one. That would be solved with a "commit processing" loading ui state. Other times, I'll commit inside of vscode, and then realize that was a mistake, and go to the command line and run git commit --no-verify to skip pre-commit check - this creates a race condition, and generates a benign, but annoying, error inside vscode.

2.) Solve all issues in 1.)

First, when we save a file inside vscode, we lint it, and then cache something on disk. We store an md5 hash of the file contents, along with the lint errors+warnings for that file. The structure of this cache should match the structure of your actual source code.

This way, for any file we want to lint, we first check the cache. If the file hasn't changed, we just return the cached lint errors+warnings.

This way when user goes to git commit, the only thing XO has to do is check the cache. In most cases the vscode/editor extension has already linted files and cached the results. I'd imagine this should be extremely fast.

By solving the performance issue, all the edge cases described in 1.) mostly disappear.

We may be able to use or take some ideas/code from Betterer. I personally would like to commit this cache to our git repo, so that our CI/CD pipeline is faster. Since we are migrating a large project to XO, we'd also greatly benefit from all the ideas in Betterer.

@devinrhode2
Copy link
Contributor Author

@sindresorhus @spence-s @fisker - any thoughts?

@sindresorhus
Copy link
Member

XO already does cache, but maybe there's something causing it to invalidate the cache too often. Probably worth looking into.

The biggest performance bottleneck in XO is TypeScript and TypeScript ESLint.

@sindresorhus
Copy link
Member

Maybe you could try using lint-staged to only run changed files through XO?

@devinrhode2
Copy link
Contributor Author

I am indeed using lint-staged only on changed files.

Something that may be a game-changer, which I just recently discovered, are TS project references. I haven't gone down this rabbit hole yet. Talks about running typescript in background, creating .d.ts files as code changes, which are much faster for typescript to parse (instead of the full original source code). This thead is about NX, but it is where the rabbit hole appeared: nrwl/nx#3664 (comment)

@fregante
Copy link
Member

fregante commented Jan 4, 2023

Nothing can be done here that is specific to pre-commits or IDE integration. XO is just a wrapper for ESLint so if (an equivalently-configured) ESLint is slow, so is XO.

Basically a duplicate of #515

@fregante fregante closed this as not planned Won't fix, can't repro, duplicate, stale Jan 4, 2023
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

No branches or pull requests

3 participants