Add exec-staged plugin

[ItsNickBarry]

I wrote exec-staged because lint-staged doesn't support Knip. This PR adds Knip support for exec-staged.

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/knip@1153

commit: 05c1f68

[ItsNickBarry]

@webpro Can this be merged? I have about a dozen packages I want to add exec-staged to, and would rather not add a knip config to each one.

[webpro]

I've been holding back for a bit, because currently it looks like the exec-staged package is not very popular yet once merged I am the person maintaining the added code.

On the other hand, it does provide a way for users to have something like lint-staged but with support for Knip, iiuc. Personally I'm not a fan of such tooling in general (who actually wants to wait for Knip to finish just to commit something?), but that's irrelevant I guess.

I'll review in the coming days.

[wbern]

I've been holding back for a bit, because currently it looks like the exec-staged package is not very popular yet once merged I am the person maintaining the added code.

On the other hand, it does provide a way for users to have something like lint-staged but with support for Knip, iiuc. Personally I'm not a fan of such tooling in general (who actually wants to wait for Knip to finish just to commit something?), but that's irrelevant I guess.

I'll review in the coming days.

@webpro In the age of AI enabled coding Knip.js is an absolute godsend as a commit hook. The time we save using AI and Knip.js preventing unused code from being committed is a godlike combo!

[ItsNickBarry]

lint-staged has rejected my PR for Knip support due to "too many commits," so exec-staged appears to still be the only way to properly run Knip on commit, for those who desire such functionality.

[cylewaitforit]

@ItsNickBarry Just curious if you’ve already given lefthook a try. I haven’t personally tried using it with Knip but have had a good experience switching to it over husky+lint-staged for other pre-commit checks.

https://lefthook.dev/examples/stage_fixed.html#stage-fixed-files

[ItsNickBarry]

I hadn't heard of lefthook. Based on the examples, I don't think it would work with Knip. It appears to have a higher level syntax for selecting input files ({staged_files} rather than lint-staged's --diff-filter), but I think this still relies on the assumption that a linter expects a list of files.

I don't see how lefthook replaces lint-staged, however, so I might be missing something.

[iiroj]

To clarify a but, I'm slow to add new features to lint-staged because I'm the only maintainer and then it's my problem to keep it working.

I did just release --hide-unstaged in 16.2.0 and adding the second flag --hide-untracked isn't that far away. After these it should be possible to run Knip against staged changes using lint-staged, right?

I also probably won't use it myself, instead running Knip separately in the CI or pre-push hook.

[ItsNickBarry]

@iiroj Yeah, all open-source maintainers are slow to add features. I recently merged at least one PR from as far back as 2022. I felt bad about disrespecting the author's contribution, but everyone involved understands that open-source moves at its own pace.

What I haven't done is request changes from a PR author, let them continually resolve merge conflicts for three months, regret the consequences of the requested changes, and then reimplement the PR myself without including that author's commits.

Why comment here anyway? You feel that it's important to mention that lint-staged will soon support Knip, so that exec-staged can be dismissed as redundant? (It's better, actually. See the readme for some ideas.)

@webpro It's really a shame that the Knip "plugin" system is a walled garden with a fixed list of supported packages. Reminds me of the Google Play store. Such an environment stifles innovation and rewards rent seekers. What's especially disappointing is that the weeks of work I have wasted were specifically for the purpose of improving the compatibility and usefulness of Knip itself.

[cylewaitforit]

Here is an example of configuring knip with lefthook . It seems to work exactly as I would have expected. Since knip runs on the whole repo you don't need to pass it the {staged_files} in lefthook. However, if you wanted to run knip --fix you would probably want to set stage_fixed: true.

Unless I am missing something, I think with husky + lint-staged, I personally would run knip in husky. Since lint-staged is doing the work to just to get the subset of staged files which knip doesn't need.

[ItsNickBarry]

@cylewaitforit Looks like lefthook is a direct replacement for husky, but not lint-staged. Running Knip as you have configured it won't work because Knip will run against unstaged changes and untracked files that may be present.

For example, maybe you add a new dependency to package.json and import that dependency in script.ts. You run git add script.ts and commit, but forget to add package.json. Knip will not notice that the repository is in a broken state (missing dependency) because the local version of package.json is correct.

[ItsNickBarry]

Note that the same is true for a pre-push hook.

[cylewaitforit]

@ItsNickBarry Thanks for the explanation, I figured there was some information I was missing. I wasn't aware that lint-staged stashed/unstashed the unstaged files.

I personally think I would still handle that in a hook directly rather than bringing in an additional dependency.

Something like:

pre-commit:
  commands:
    knip:
      run: |
        git stash push --keep-index -m "lefthook-knip" 2>/dev/null || true
        trap 'git stash list | grep -q "lefthook-knip" && git stash pop 2>/dev/null' EXIT
        npx knip

I'm certain that husky could do the same.