Don't pollute enclosing project hooks

[ericsoco]

I just ran npm install on a package within my project's node_modules. This package has husky listed as a devDependency. husky ended up writing hooks into my project's .git/hooks, instead of the package within node_modules. Now I can't commit code in my own project without removing the hooks / --no-verifying.

[typicode]

Hi @ericsoco,

Not sure to understand why you want to run npm install in node_modules?
But if you're making some tests, you can run npm rm husky --save-dev to get rid of it and installed hooks.

[ericsoco]

I was debugging a package and needed to rebuild it to pick up my changes.

My apologies, I don't know how husky does what it does, so I don't know if
it's feasible for it to be aware of context (installed within an npm
installed package or not), but it would be great if it could be smart
enough to not install hooks two levels up.

On Friday, December 4, 2015, typicode notifications@github.com wrote:

Hi @ericsoco https://github.com/ericsoco,

Not sure to understand why you want to run npm install in node_modules?
But if you're making some tests, you can run npm rm husky --save-dev to
get rid of it and installed hooks.

—
Reply to this email directly or view it on GitHub
#36 (comment).

ditditdit on my mobile phone

[iamstarkov]

cant wrap my head about this problem. @ericsoco can you elaborate a bit?

[ericsoco]

It's been a long time @iamstarkov, but best as I remember, my initial comment describes it well -- I npm installed a package in my project that uses husky. In order to debug that package, I had to run npm install from within its folder (e.g. myproject/node_modules/package-that-uses-husky). Doing so installed git hooks on my project (myproject/.git/hooks) instead of on the package (myproject/node_modules/package-that-uses-husky/.git/hooks). That made things go wonky on my project.

If possible, husky should know where to install its hooks to avoid this problem. This seems an edge case, though.

[ljharb]

I had the same problem (with ghooks). This caused git hooks I didn't write, audit, or know about to be installed in my company's private repo.

Automatically injecting git hooks on install (as opposed to requiring the package consumer to explicitly add a command in a postinstall script, which would be far safer) is dangerous enough - it should certainly not traverse above the folder the closest package.json (that references husky) is in under any circumstances.

[typicode]

@ericsoco next version of Husky will prevent this case. There's already a test and code for this in master.

So, Husky won't install git hooks if it detects that it's in a sub-node_module directory. For example:

project$ cd node_modules/json-server
project/node_modules/json-server$ npm install

won't install hooks in project/.git

[typicode]

it should certainly not traverse above the folder the closest package.json (that references husky) is in under any circumstances.

@ljharb thanks for the feedback. At first, this was the behaviour, Husky was supposing that .git directory was at the same level than package.json (as you would expect it to be in most Open Source projects).

But actually, there's many people that seem to have a different layout, for example:

project/front-code/package.json
project/server-code
project/.git

That's why Husky looks for the first .git directory it can find.

That said, I would be interesting in better understanding your use case. This way Husky could be improved.

Do you have a workflow example?

Because, except for the edge case described by @ericsoco, I can't see a case where Husky is defined in devDependency and shouldn't install hooks in .git.

Automatically injecting git hooks on install (as opposed to requiring the package consumer to explicitly add a command in a postinstall script, which would be far safer)

Actually, I don't really see how it would differ in the end?

That said, adding a command in postinstall script may be the recommended way for the next version, as yarn seems to work differently than npm regarding npm hooks.

[ljharb]

All I know is that I've never done any development on video.js, they used ghooks as a dev dep, and somehow my company's private git repo ended up with git hooks i didn't write or audit.

I think doing anything like this automatically on install is HIGHLY dangerous. Users should have to opt-in by running an explicit command first.

[phiresky]

Our project has multiple submodules placed in a root node_modules folder to make dependencies easier, see this description. Is there any way to get husky to work with this directory structure?

[cdaringe]

@ljharb, fair concerns. although it's unconventional for non-node_modules/-contained effects to occur oninstall of a package, the purpose of this package is precisely that side-effect. it's explicit opt-in. there's some danger, but the risk is the same risk as trusting whatever tarball comes down the line per usual, no? further, it only happens in dev mode, so prod shipments are unaffected.

@typicode, what i'd be interested in seeing is some indexing into the .git/hooks/pre-commit file.

that is, with the following structure:

/.git
/packages/A/...
/packages/B/...

execute the following psuedocode:

• if no pre-commit file, generate one
• if a pre-commit file, add an easily identifiable include: . ./husky-pre-commit (+ whatever malarkey windows needs)
• do all activities in husky-pre-commit
• on husky install from A | B
  • detect path to A | B
  • add entry into husky-pre-commit pointing to A | B
  • perform all existing husky workflows for each entry

in this fashion:

• there's no clobbering
• multi-project support works
• single project support works
• single project w/ weird structuring works

and everybody is happy! 🎉 thoughts?

[ljharb]

@cdaringe What would be explicit opt-in is if the requirement was to add "postinstall": "ghooks --init" or similar to "scripts". This is implicit, not explicit, because simply by adding a dependency, something other than installing happens.

[tilgovi]

I just discovered this behavior when I realized that I had husky hooks installed in a repo that doesn't use husky.

I'd like to use husky in a project that will be distributed as a tarball. I don't want husky to install hooks when users build the project.

Would it be possible to default to the old behavior of assuming that the git directory is at the same level as package.json, install no hooks if it's not found, and add a configuration switch for setting an explicit root? That would make the default safer while allowing those with different layouts the ability to still use husky.

[tilgovi]

Would it be possible to default to the old behavior of assuming that the git directory is at the same level as package.json, install no hooks if it's not found, and add a configuration switch for setting an explicit root?

This would also handle the case of node_modules, as described at the top of this issue and in #304.

Without this, I'm afraid I might have to remove husky from my project.

[tilgovi]

I even tried seeing if I could get husky to skip installing the hook scripts by having an empty config:

// husky.config.js
const fs = require('fs');
const path = require('path');

if (fs.existsSync(path.join(__dirname, '.git'))) {
  module.exports = {
    hooks: {
      'pre-commit': 'lint-staged',
    },
  };
} else {
  module.exports = {};
}

[typicode]

Closing as husky 5 doesn't automatically install hooks anymore. Unless explicitly specified in package.json > postinstall . Thanks for the feedbacks 👍