Should treat optional peer dependencies as similar to Unlisted dependencies

[voxpelli]

In a project of mine I correctly got:

Unlisted dependencies (1)
pg  lib/advanced-types.d.ts

Though since I want that dependency to be optional I added:

"peerDependencies": {
  "@types/pg": "^8.10.2",
  "pg": "^8.11.2"
},
"peerDependenciesMeta": {
  "@types/pg": {
    "optional": true
  },
  "pg": {
    "optional": true
  }
},

Since its optional there's still no guarantee that it will be available, hence I expected the Unlisted dependencies error to remain or be replaced with something like Relying on optional dependency

Bonus feature request:

Since I myself only directly use these for types I use a trick I started using years ago in declaration files: I add a @ts-ignore on top of them:

// @ts-ignore Avoid strict dependency on 'pg'
import type { Pool, PoolClient } from 'pg';

That makes it so that Pool and PoolClient has their correct values when the types are available and else they will be any.

If knip could detect this and silence a Relying on optional dependency error but still do a Unlisted dependencies if I haven't listed them as optional peer dependencies, then that would be very very sweet.

(Tell me if you would want me to try and make a PR for this)

[webpro]

Yes good idea, I had this in the back of my mind, but never got around to implement it.

Feel free to start working on this! I'll notify here if/when I start myself (I don't have time next ~10 days).

We can look at the bonus after landing this.

[voxpelli]

@webpro Do you think it should be aRelying on optional dependency error or the same Unlisted dependencies?

[webpro]

I think this warrants a new issue type indeed, since it's not "unlisted".

[webpro]

Scratch that, the reasoning is not that it's unlisted or not. It is unlisted, but it's good to make it clear that it's not a regular (dev) dependency. So yes a new issue type (but for another reason).

I also distinguish regular exports and namespaced exports. Good UX, imho.

[webpro]

Started looking into this. Just to be sure, all of this is in --production --strict mode, right?

[voxpelli]

No, in ordinary mode for me

[webpro]

🚀 This issue has been resolved in v2.21.0-op.0. See Release 2.21.0-op.0 for release notes.

[webpro]

Added the op dist tag (e.g. npm install knip@op). Didn't merge to main yet as I'd like to test it a bit more myself, including looking at how this works in tandem with --production, etc.

Would be great if you could try it as well :)

[voxpelli]

Note to self: When opening an issue like this, include a personal reference to which project you experienced the issue in 😆

@webpro I'll try testing it, I will just need to find the correct project again 👍

[webpro]

Haha no worries. You could also work in (and maybe improve) the fixtures/peer-dependencies-optional in this repo. But yeah, real-world is better :)

[voxpelli]

Found it: https://github.com/voxpelli/umzeption

And yeah, it works 👍

One reflection: If I add "ignoreDependencies": ["@types/pg", "pg"] then they do are ignored, but I also get:

Configuration issues (2)
Unused item in ignoreDependencies: @types/pg
Unused item in ignoreDependencies: pg

(Also, I'm not using --production or --strict in that project or any of my others, if you want to give it a look and come with suggestions on how to make it work, then I would like to possibly become stricter)

[webpro]

🚀 This issue has been resolved in v2.22.0. See Release 2.22.0 for release notes.

[webpro]

Thanks for the insights Pelle, this has landed.