Berry: [Case Study] Husky

Created on 30 Apr 2019  Â·  8Comments  Â·  Source: yarnpkg/berry

What package is covered by this investigations?

husky

Investigation report

  • Husky uses run-node in order to execute the "right" version of Node. Since it only contains a bash script that cannot be executed straight from the archive, this package needs to be unplugged (why don't they use process.execPath?).

  • Husky accesses the run-node binary through require.resolve('.bin/run-node'), which is invalid (first because it uses .bin as a package name which it isn't, and second because it references the .bin folder, which is an implementation detail). It should instead use the following construct: require.resolve('run-node/run-node').

case study

Most helpful comment

I've been experimenting with "letting Husky know that it should run the script through Yarn":

https://github.com/arcanis/husky/commit/c1ccc9a6816fe584db548d36717954a0034144d5

I haven't yet had a chance to try it myself, but the idea is that with this diff you should just have to add a huskyrc file into the root of the repository, put HUSKY_USE_YARN=1 inside, and voilà.

All 8 comments

Hi @arcanis

Since it only contains a bash script that cannot be executed straight from the archive, this package needs to be unplugged (why don't they use process.execPath?).

Not sure what "unplugged" means but here's some context regarding run-node.

It's for people that use nvm and GUIs which don't have node in their PATH.

In a terminal, the version found in PATH will be left unmodified but in a GUI it'll usually pick the latest nvm installed version.

Initially, first versions of husky used process.execPath but there were some concerns about it. Because unless you reinstall husky, you could end up using an old version of Node for your hooks (see https://github.com/jquery/jquery/issues/2915).

Recently, you can configure husky to load nvm (or in any other Node version manager) in GUIs using ~/.huskyrc (https://github.com/typicode/husky/blob/v2.2.0/DOCS.md#huskyrc).

In the end, GUIs are tricky and it's not a perfect solution, but most of the time run-node does the job at being a fallback when Node can't be found.

It should instead use the following construct: require.resolve('run-node/run-node').

Looks actually better, thanks for the tip.
I'll have a look to change that.

Hey @typicode, thanks for the answer!

Not sure what "unplugged" means but here's some context regarding run-node.

Yarn 2 features something called "zip loading". The concept is simple: we instruct Node to load the JS files directly from within the archives that contain them rather than from the disk. In our case, it means that we usually don't have to unpack the package archives.

The problem is that it only works with Node, and thus packages containing shellscripts don't work under this mode. They need to be "unplugged", ie unpacked on the disk (like in the past). It's not too problematic, but the user experience isn't great.

Initially, first versions of husky used process.execPath but there were some concerns about it. Because unless you reinstall husky, you could end up using an old version of Node for your hooks

That makes sense, especially since Husky runs in a shellscript 🤔 I guess it's up to us to figure out a good way to make it work.

Is this resolved now via https://github.com/typicode/husky/pull/488? It seems not, there's still an issue with husky in that it runs its run.js script using vanilla node (no pnp resolution) via run-node, so it immediately fails because it's unable to load dependencies.

Yep I'm not quite sure yet how to fix that one, but it's on my personal shortlist. Any suggestion how we could do this? It's unfortunate that our magic conflicts with their magic 😢

Pinging @typicode - maybe we could somehow detect that Husky is running inside Yarn and, when it's the case, use yarn node instead of run-node? yarn node is already guaranteed to execute with the version of node used to run yarn.

A few ideas:

  • Write a node wrapper that behaves like yarn node but can be put on $PATH and used instead of native node. Perhaps could be selected by nvm or n.
  • Add an option to husky that allows require.resolve("run-node/run-node") to be customised (e.g. via a husky.node field in package.json, where the default value would be run-node/run-node). Then Yarn 2 install instructions would need to include a note for that.

    Not sure about windows support https://github.com/typicode/husky/blob/8b9989f0696e0f0c330991ebe46e93181d2f9ad8/src/installer/getScript.ts#L103.

  • Add an option to husky that allows a "before script" to execute in each hook, e.g. husky.beforeHookScript that could then set NODE_OPTIONS to include --require … where it requires a pnp hook to setup require resolution.
  • Project scoped .huskyrc support https://github.com/typicode/husky/issues/499#issuecomment-500849719
  • Change the way husky is built to produce a single .js that has no dependencies, like the way yarn is built, so that it's completely portable and doesn't rely on module resolution.
  • Change run.js to detect when it should use Yarn 2 and install the pnp hook before continuing.
  • Skip using husky altogether and build it directly into Yarn 2 as a plugin. There's already a custom shell, and constraints system, what's one more thing 😄. It would actually be really handy if git hooks worked out of the box if you use Yarn 2, and you don't need to install another package to get it done.

@arcanis do any of these sound particularly appealing, or do you have any other thoughts on this?

I've been experimenting with "letting Husky know that it should run the script through Yarn":

https://github.com/arcanis/husky/commit/c1ccc9a6816fe584db548d36717954a0034144d5

I haven't yet had a chance to try it myself, but the idea is that with this diff you should just have to add a huskyrc file into the root of the repository, put HUSKY_USE_YARN=1 inside, and voilà.

Closing - Thanks to @typicode's help, Husky is now PnP compatible starting from 4.0.0-1 🎉

Was this page helpful?
0 / 5 - 0 ratings

Related issues

bradleyayers picture bradleyayers  Â·  3Comments

larixer picture larixer  Â·  4Comments

tiansijie picture tiansijie  Â·  4Comments

thealjey picture thealjey  Â·  4Comments

Santas picture Santas  Â·  3Comments