ts-prune icon indicating copy to clipboard operation
ts-prune copied to clipboard

add pre-commit hook

Open jvllmr opened this issue 2 years ago • 4 comments

Using ts-prune in a pre-commit hook could be helpful to better audit dead code before the commit comes through.

More about pre-commit if you don't know it: https://pre-commit.com/#intro.

Besides the pre-commit hook, I updated jest to version 28 as version 25 conflicted with newer typescript version (yarn did not care, but npm does and pre-commit uses npm for node-based hooks).

At last, I changed the error exit code to 65 because I think that it's more specific and doesn't get mistaken with other errors coming from node (1 could mean anything). I took the info about it from /usr/include/sysexits.h.

jvllmr avatar Aug 11 '22 21:08 jvllmr

FYI I'm running ts-prune as a pre-push hook without anything being required of this repo. I presume the same pattern could be applied to pre-commit...

I have npm run ts-prune inside .husky/pre-push in the repos I'm using it in.

...And in package.json > "ts-prune": "ts-prune -p tsconfig.ts-prune.json",

I'm not a maintainer of this package - though I have contributed - but adding this to the repository would, in theory, force me to have this run as a pre-commit when I don't want it to?

rubengmurray avatar Aug 17 '22 17:08 rubengmurray

I hope I can clear misunderstandings by listing facts about the pre-commit cli and its ecosystem:

  1. Your hooks only run if you use the pre-commit cli and run pre-commit install in your repository
  2. pre-commit runs its hooks in isolated environments
  3. You can run user defined hooks like with husky, but they are feature-limited (for example no update via pre-commit autoupdate) and you have to define them yourself, installing and maintaining vendor defined hooks is much easier
  4. Vendor defined hooks (as I defined it in .pre-commit-hooks.yaml) are build from source and because there are TypeScript projects pre-commit has to install the dev dependencies

In short: If you're not using the pre-commit ecosystem the additional config doesn't matter and the jest update shouldn't hurt aswell.

About the exit code: I know this might introduce a breaking change for people using this tool, but I see asserting exit_code == 1 as a bad habit anyway and it only makes more clear where the error came from. Of course this is a decision the maintainer has to make, not me.

jvllmr avatar Aug 17 '22 18:08 jvllmr

Fair enough. I haven't come across this before. I presume the pre-commit cli refers to this: https://pre-commit.com ?

rubengmurray avatar Aug 21 '22 23:08 rubengmurray

Yes, it does.

jvllmr avatar Aug 22 '22 20:08 jvllmr

Stale pull request message

github-actions[bot] avatar Oct 22 '22 13:10 github-actions[bot]