Knip detects a script parameter as a package dependency

[Faithfinder]

All it the title.
Repro.

[voxpelli]

deploy is claimed to be missing? This is correct I believe, it's an unspecified dependency. If it's something provided externally on your system then you need to tell knip that you have made sure it's available where it's needed, because knip can't know?

[Faithfinder]

deploy is claimed to be missing? This is correct I believe, it's an unspecified dependency. If it's something provided externally on your system then you need to tell knip that you have made sure it's available where it's needed, because knip can't know?

I implore you to actually run knip on the repro ;)

[webpro]

🚀 This issue has been resolved in v4.2.2. See Release 4.2.2 for release notes.

[webpro]

Bit of an edge case situation, but should be fixed!

[Faithfinder]

Eh, looking at the fix code, I'm not sure this does it. I've used deploy as name in the minimal repro, but that's not a real binary (I had to put it in ignoredBinaries so that repro focueses on correct thing). In reality it's an in-house binary that's called welbi-deploy, and -r stands for repository (ECR). I'm fairly certain there's a billion binaries out there that have their own meaning for -r. I think the list you want to maintain should be inclusive (node, ts-node, etc), rather than exclusions.

If your experience says that -r stand for require fairly universally, fair enough, but I'd add this special case to the docs. If it was there I would update scripts to use full parameter name, rather than add repo name to ignoreDependencies

[webpro]

The reason I mentioned it's an edge case, is because it's not that common to use a binary in package.json#scripts that didn't come with a dependency also listed in the same file.

Knip makes a difference between these scripts and, for instance, binaries in GitHub Actions files, which is totally different context.

I'm definitely not stating that --require is a universal thing. I think it's just fairly common within the npm ecosystem, within package.json#scripts, and only as far as I've seen myself. So within this context I once decided to use this as a default and I still think Knip is on the right side here. Always happy to adjust, though.

The obvious downside and, indeed, a very ugly part of Knip here is that if..

1. a binary is used that's not part of a listed dependency, AND
2. it's using -r, --require and/or --loader argument, AND
3. with a value that's not a local entry file or a dependency

..then it's reported as an unlisted dependency.

So either Knip makes a whitelist or a blacklist for this, and I'm still on the fence with it. It's hard to justify hard-coding welbi-deploy in Knip's codebase, that's for sure.

[Faithfinder]

The binary in my case is a listed dependency. It's internal to the monorepo, but it's in package.json (well, the package name is @welbi/deploy-scripts, welbi-deploy is the binary name). In a rush repo you don't get random things floating around, everything is listed :)

I for sure am not asking for a hardcode of welbi-deploy, or just deploy for that matter, I guess I should have made my example say aBinary :)

And I guess I do agree that people don't often write their own binaries, though they sorta should, it's nice :)

Still, IMO allowlist is a better pattern, because I've only seen require in node and wrappers around node, like ts-node.

Regardless, I think doc updates are in order. Regardless of allow or block list, this is "magical" behaviour and should be documented. Here's a PR: #484