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

Is isInNodeModules necessary? #304

Closed
mapleeit opened this issue Jun 17, 2018 · 2 comments
Closed

Is isInNodeModules necessary? #304

mapleeit opened this issue Jun 17, 2018 · 2 comments

Comments

@mapleeit
Copy link

mapleeit commented Jun 17, 2018

Hi, thanks for your great project. I am a little confused about why this isInNodeModules condition is necessary.

This prevents the case where someone would want to debug a node_module that has husky as devDependency and run npm install from node_modules directory

I saw this comment, but it's reasonable for me to install husky even when someone debug some node_module because the package.json has husky as a devDep. And it will not produce any side effects.

if (isInNodeModules(huskyDir)) {
    console.log(
      'Trying to install from node_modules directory, skipping Git hooks installation.'
    )
    return
  }

If you think avoiding installing in node_modules is a good practice, could you consider that adding a parameter like --force to force install?

@typicode
Copy link
Owner

typicode commented Jul 2, 2018

Hi @mapleeit,

Thanks for the comment and question.

Maybe I'm missing something, but personally, I can't really think of a use case where you would want to install Husky from a node_modules project.

Here's an example where isInNodeModules is useful, let's say you debug module B

A/node_modules/B/package.json # contains husky as devDependency

Since B isn't a cloned repository, you can't commit changes or run git commands related to B module. That's why Husky avoids installing hooks for B module in the first place.

But if hooks were installed in A/.git/hooks and B has "pre-commit": "npm test", you would end up with this behavior:

A$ git commit -m 'Change to A'
# would run tests for B every time

You can also read #36 for more context. Hope it clarifies a bit :)

@mapleeit
Copy link
Author

mapleeit commented Jul 4, 2018

Got it now, thanks for your explanation!

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

2 participants