chore: fix Git Hooks

[ehmicky]

This fixes Git hooks with the new version of Husky (see netlify/eslint-config-node#403).

The steps are:

• Upgrade @netlify/eslint-config-node to 4.1.2
• npm uninstall husky
• Remove husky property in package.json
• Add prepare script to package.json

cc @XhmikosR

[XhmikosR]

Nice! I was worried this caused too much trouble for you guys that I wanted to submit a PR to revert the husky update so that you take your time :)

PS. the executable bit is copied
PPS. you should also remove the husky package.json property lines 31-35 :)

[XhmikosR]

@ehmicky @erezrokah Might be worth droping npm-run-all from the direct devDependencies in your projects since it's included in @netlify/eslint-config-node. Just thinking out loud :)

[ehmicky]

I believe you are correct.

Binary symlinks seem to be installed by npm to node_modules/.bin/ for both direct and indirect dependencies. That being said, I am 100% sure this is the case on other package managers, but we could try it out and see if anyone mentions it not working.

[XhmikosR]

Also, just to confirm, are we sure you don't need a prepare script?

I'm trying this locally and I don't see any hooks running

[XhmikosR]

I still think it's probably worth reverting the husky update...

I personally cannot make it work here even with prepare, I get an error that commitlint is not found. :/

It only worked for @netlify/eslint-config-node and it wrongfully made me think it worked, but it's too troublesome.

[ehmicky]

Yes, I can confirm the prepare script does not seem to work here.

That being said, there might still be some ways to be make it work automatically, I'm exploring those right now.

Also, I would be interested in ways to avoid duplicating the contents of each Git hook. For example, if we wanted to pass new flags to commitlint, it'd be better to only have to update it in eslint-config-node as opposed to inside each repository. One possible way to do this would be for eslint-config-node to expose each Git hook as a bin, and let each repository's Git hooks use those shared binaries.

[kodiakhq]

This PR currently has a merge conflict. Please resolve this and then re-add the automerge label.

[XhmikosR]

I'm hear you, I just think the husky update should be reverted, cut a new patch release so that you have things working and experiment in another branch with this.

Better have something working than nothing :)

[ehmicky]

I think I made it work!

I went with your suggestion @XhmikosR of using a prepare script. It calls husky install node_modules/@netlify/eslint-config-node/.husky. prepare is run after npm install, but only for local installs, not for consumers of this module. So no need for developers to setup anything: it is done automatically on npm install.

I have been digging in the husky codebase (which is surprisingly small), and what husky install does here is mostly to just change the core.hooksPath git configuration property to that file path.

This works for me locally. This should work with most package managers and OS, I believe.

Additionally, since each repository targets node_modules/@netlify/eslint-config-node/.husky, no need to copy those Git hooks inside each repository, so the Git hooks code is shared between all repositories.

Please try it out and let me if this works on your machine too!

@XhmikosR Let's see if you still get an error related to not finding the commitlint binary. If so, I believe we can just use npx commitlint to the Git hook and it should fix that.

[XhmikosR]

Still fails here :/

node_modules/@netlify/eslint-config-node/.husky//commit-msg: line 4: commitlint: command not found
husky - commit-msg hook exited with code 127 (error)

[ehmicky]

Thanks for the information.
At least, the git hook is running, which is great. 🎉

I think we should find a solution to get the right binary file path. Could you try to manually edit node_modules/@netlify/eslint-config-node/.husky/commit-msg (directly inside the node_modules directory), replace commitlint by npx commitlint, then try again? If this fix works, I will create a PR to eslint-config-node to make that change.

[XhmikosR]

npx works :)

Another solution would be to extend the $PATH to include the local node_modules/.bin directory, but this is probably easier.

Makes me wonder though, how come no one else had these issues, maybe we are missing something. Or it's because of the shared husky config approach?

[ehmicky]

Great, let's add npx then: netlify/eslint-config-node#417

Personally, my .bashrc adds ./node_modules/.bin to my $PATH, which is why I did not experience this problem. npm run and npx do this too under the hood.

[XhmikosR]

Yeah, you know, Windows 👼

Glad you got it sorted it out after all. I hope the second project you migrate is a lot easier 🙂

[ehmicky]

The npx fix is merged now, so this PR should be ready to ship.

[lukasholzer]

I've tried it and works like a charm thx for fixing 💪