pzavolinsky / ts-unused-exports

ts-unused-exports finds unused exported symbols in your Typescript project
MIT License
749 stars 49 forks source link

Use of exit codes breaks pre-push hooks (husky #258

Closed rubengmurray closed 1 year ago

rubengmurray commented 1 year ago

If unused exports are found when running ts-unused-exports as part of a pre-push hook with husky, the hook fails completely and a push is prevented.

For initial implementations of ts-unused-exports, it may be cumbersome/undesirable to go and fix all of the newly found unused exports. The visibility is in-and-of-itself valuable and would allow developers to address these as time passes, but the package currently forces all unused exports to be fixed before implementing as part of a hook.

I had a similar issue with depcheck and it was down to the error code being used. For ts-unused-exports, the cli output is a lot less ambiguous and it undoubtedly looks like the same cause:

husky - pre-push hook exited with code 1 (error)
error: failed to push some refs to '...'

I'm wondering if this is a known side-effect of the error codes being used?

In any case, I have found a workaround:

package.json

"scripts": {
  ...
  "ts-unused-exports": "ts-unused-exports tsconfig.json"
  ...
}

.husky/pre-push.sh

npm run ts-unused-exports || true

source: https://stackoverflow.com/a/41656045/10732370

mrseanryan commented 1 year ago

hi @rubengmurray thank you for the feedback.

The exit code is working as designed, same as eslint.

We could add yet another option to modify exit code meaning but that adds complexity and is maintenance overhead.

The use case with hooks seems a special case where exit code should only be non zero if there was a serious process fail. This was discussed before and that’s how we ended up with the current approach that copies eslint.

So the workaround you have made seems the most practical approach.

mrseanryan commented 1 year ago

If a ton more people want this extra option, we can consider it of course…