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

Avoid prexxx postxxx names for hooks #99

Closed
vovacodes opened this issue Feb 15, 2017 · 7 comments
Closed

Avoid prexxx postxxx names for hooks #99

vovacodes opened this issue Feb 15, 2017 · 7 comments

Comments

@vovacodes
Copy link

When you run your npm script called xxx, npm will automatically execute the scripts prexxx before and postxxx after the target script if available. husky uses the names with pre and post for the hook scripts a lot , this introduces the limitation for the names of your npm scripts, e.g. #94 (comment)

Can we use names like pre-xxx and post-xxx for the hooks instead?

@typicode
Copy link
Owner

typicode commented Mar 1, 2017

Hi @wizardzloy,

Thanks for the feedback. Actually I didn't thought about this edge case.

Not against changing to raw names, but I'd like to better understand what are the hooks that conflicts for people and if it would be possible to rename them?

@vovacodes
Copy link
Author

@typicode It is definitely possible to rename your scripts to avoid the clashes, but I believe it's still a limitation.
In our project the script that caused the problem was commit. We use commitizen for maintaining the format of our commit messages and the script looks like: "commit": "git-cz". We renamed that to cz eventually, but the name commit makes more sense to me still.

@duclet
Copy link

duclet commented Mar 8, 2017

I'm not sure if that will fully resolve the issue. There is nothing stopping it for whatever reason executing prepre-commit for example. Also, it takes away the ability to name certain scripts because git has a commit hook for that. One of the ones I see is update. Also, what if git decides to add a new hook in the future? So currently I have a script called foo, I upgrade git and now it has a hook called foo as well. All of a sudden, my script will automatically run when it shouldn't. Personally, I like the idea of the hooks being in there separate configuration much like how ghooks had it (that was one of the main reason why I switched from husky to ghooks). While the maintainer have essentially given up and recommend people to using this, I still find the git hooks being in the scripts block rather dangerous. I do want to make this work though so my recommendation is this: Add support for users of this module to specify where they want the hooks to be read from via a configuration in package.json. It can continue to support what it is currently doing as the default. Otherwise, users of the configure it to be in a separate config such as:

{
    ...
    "config": {
        "husky": {
            "hooksInConfig": true,
            "hooks": {
                "commit": "..."
            }
        }
    }
    ...
}

@faceyspacey
Copy link

@duclet excellent. That's what it should be.

@GabeDuarteM
Copy link

@typicode Is this still being looked at? I'd love to configure husky like @duclet specified...

Maybe without the "hooksInConfig": true, part, because if the user puts a config.husky.hooks section inside the package.json file, then it makes sense that it would be implied that it should be used instead of the npm scripts...

So, if config.husky.hooks is present, husky could use that and stop looking at the npm's scripts, otherwise, it continues looking if there's a hook present at npm's scripts.

What do you think?

@typicode
Copy link
Owner

typicode commented Feb 4, 2018

@GabrielDuarteM yes, actually v0.15 version lets you define scripts in a dedicated husky.hooks field using raw names as suggested by @wzrdzl. You can even have a huskyrc file ;)

Corresponding README can be found in the dev branch:
https://github.com/typicode/husky/tree/dev

It has a known issue with GitHub Desktop on Windows (that is going to be fixed), that's why it's currently behind a @next flag and is marked as rc.

Thank you all again for the feedback :)

@typicode typicode closed this as completed Feb 4, 2018
@GabeDuarteM
Copy link

Cool!

Looking forward to v0.15 :)

christian-hawk added a commit to GluuFederation/gluu-passport that referenced this issue Mar 26, 2021
Developer can run it, if desired, using `npx cz`.

Also there are known issues using Husky and Commitizen together
- commitizen/cz-cli#558 (comment)
- commitizen/cz-cli#432 (wont fix)
- typicode/husky#99
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

5 participants