Skip to content

[chore] Run pre-commit hooks in parallel#1190

Open
yanfali wants to merge 2 commits into
masterfrom
yanfali-precommit-concurrently
Open

[chore] Run pre-commit hooks in parallel#1190
yanfali wants to merge 2 commits into
masterfrom
yanfali-precommit-concurrently

Conversation

@yanfali

@yanfali yanfali commented Sep 27, 2022

Copy link
Copy Markdown
Collaborator

Description

@yanfali yanfali force-pushed the yanfali-precommit-concurrently branch from dbd7aa9 to 9ef682f Compare September 27, 2022 02:00
@mateossh

Copy link
Copy Markdown
Contributor

I spent some time investigating why pre-commit hook is taking that much time.

I started by checking (on master) husky pre-commit script and eslint src times.

Zrzut ekranu 2022-09-28 o 11 24 57

Oh, prettier is executed twice. Once it's executed by script, and then by eslint.

So I removed ESLint prettier plugin and checked times again.

Zrzut ekranu 2022-09-28 o 11 31 43

.eslintrc.js diff
diff --git a/.eslintrc.js b/.eslintrc.js
index d83a5bc0..68c1db88 100644
--- a/.eslintrc.js
+++ b/.eslintrc.js
@@ -6,7 +6,6 @@ module.exports = {
   plugins: ['cypress'],
   extends: [
     'plugin:vue/recommended',
-    '@vue/prettier',
     'plugin:cypress/recommended'
   ],
   rules: {
(END)

ESLint time went down from +4 seconds to less than 2 seconds on my M1 MacBook Air.

This PR with removed eslint-plugin-prettier gives following results:
Zrzut ekranu 2022-09-28 o 11 43 32

Do you think it's safe to remove eslint-plugin-prettier?

@yanfali

yanfali commented Sep 28, 2022

Copy link
Copy Markdown
Collaborator Author

Oh good catch. If anything I would remove parallel prettier. I think the vue version has special handling for single file components. I'm not sure regular prettier does.

@yanfali

yanfali commented Oct 4, 2022

Copy link
Copy Markdown
Collaborator Author

my vague memory of why we added this feature was people were sending us PRs with unformatted code. VSC IDE that I have setup formats on save so I don't need it personally, but it's useful for the general population of less sophisticated developers. I would say let's drop the call to prettier in npm and see if this causes problems on PRs.

@mateossh

mateossh commented Oct 4, 2022

Copy link
Copy Markdown
Contributor

How about running prettier only on changed files? I think there should be some way to do that

@yanfali

yanfali commented Oct 4, 2022

Copy link
Copy Markdown
Collaborator Author

How about running prettier only on changed files? I think there should be some way to do that

Great idea. Apparently there's a lint staged hook

@mateossh

mateossh commented Oct 9, 2022

Copy link
Copy Markdown
Contributor

I installed lint-staged and configured it in my fork. Should I create new PR?

@yanfali

yanfali commented Oct 9, 2022

Copy link
Copy Markdown
Collaborator Author

I installed lint-staged and configured it in my fork. Should I create new PR?

Sure.

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