Support optional peerDependencies

[edmorley]

Do you want to request a feature or report a bug?
Feature

What is the current behavior?
With Yarn's new Plug and Play feature, if a package uses another package, it must include that other package either in dependencies or peerDependencies otherwise there will be a runtime resolution failure.

However some projects intentionally omit the dependency if it's for an optional feature. For example in:
waysact/webpack-subresource-integrity#90

At the moment such projects have a choice between:

• not adding the optional peer dep, and Yarn PnP not working
• adding the peer dep and making everyone install a potentially unnecessary dep to silence the "missing peer dependency" warnings during yarn install

What is the expected behavior?
To have a (backwards-compatible) way to declare a peer dependency as optional, which would allow the PnP resolver to still work, but if the optional peer dep package was not installed, there would not be a "missing peer dependency" warning shown during yarn install.

In waysact/webpack-subresource-integrity#90 (comment) this form was proposed:

{
  "peerDependencies": {
    "html-webpack-plugin": "optional:^x.y.z"
  }
}

And in waysact/webpack-subresource-integrity#90 (comment), this:

{
  "optionalPeerDependencies": {
    "html-webpack-plugin": "^x.y.z"
  }
}

Please mention your node.js, yarn and operating system version.
Yarn 1.12.0, all OSes.

CC @arcanis, @jscheid

[jscheid]

Thanks @edmorley for picking this up, it would be great to have a feature like this.

(Also, regardless of PnP, it would provide a way to express "I don't need this package, but when present it has to be version range XYZ").

I was actually thinking optionalPeerDependencies to just be a list of names, as in "optionalPeerDependencies": ["html-webpack-plugin"]. Admittedly it's potentially confusing that peerDependencies is an object and this an array, perhaps calling it optionalPeerDependencyPackages (or Names) would alleviate that?

Just thinking out loud, I don't really have the full picture of all the implications with regard to backward compatibility and compat with other package managers.

[arcanis]

Regarding optional:, imo the best argument in favor of it is that it would work regardless of the package manager (worst case there would be a warning). It also has a slightly better story for composition (we could imagine stacking the tags: optional+dev:*). Downside is that it wouldn't allow more complex schemes with parameters, but we don't have any and I don't see us doing such drastic changes.

Regarding optionalPeerDependencies, it would work a bit by chance on package managers that wouldn't support it (just like when the dependency wasn't listed at all in the peerDependencies). Since even now npm still doesn't support link: dependencies, it seems important to keep that in mind. It also wouldn't compose well with other tags (optionalDevPeerDependencies when? :p).

Another option, maybe more radical, is to make the value type of peerDependencies accept an object with metadata. I'm not sure how well package managers (including Yarn's past releases) would support it though. Maybe they would just crash or ignore the peer dependency 😐

{
  "peerDependencies": {
    "html-webpack-plugin": {
      "range": "^x.y.z",
      "optional": true
    }
  }
}

[jscheid]

Regarding optionalPeerDependencies, it would work a bit by chance on package managers that wouldn't support it (just like when the dependency wasn't listed at all in the peerDependencies).

I'm not arguing against your approach at all, looks great. But just to clarify, what I was suggesting would be backward compatible not just by chance and would compose well. It would look something like this:

{
  "peerDependencies": {
    "html-webpack-plugin": "^x.y.z",
    "webpack": "^x.y.z"
  },
  "optionalPackages": ["html-webpack-plugin"]
}

(Edited to rename optionalPeers to optionalPackages)

[arcanis]

Oh I see, sorry! Yep, would work indeed. optionalPeers would actually be a better name since we already have optionalDependencies (which are a weird design by themselves, and mainly used nowadays for fsevents).

[jscheid]

What about optional dev dependencies though?

Not to blow up the scope of this ticket, but since you mentioned composability - I remember wanting for that feature once.

[arcanis]

Things like optional dev dependencies require a relatively huge investment since we would have to support a "better dependencies" field that would allow meta information to be passed to the package manager on top of the range (or doubling down to the antipattern that are fields such as optionalDevDependencies), so I think they're out of scope for this issue.

Considering the pros/cons, I think adding a syntax to the peerDependencies range is the less intrusive way to solve the problem described in this issue. So what would be the best syntax?:

1. "react": "optional:^1.0.0"
2. "react": "?^1.0.0"
3. "react": "^1.0.0?"

Note that ? is an invalid character per semver, so it should be fine to use it.

[arcanis]

Also, ping @iarna and @zkochan - feel free to share your thoughts 🙂

[Gudahtt]

I was wondering why this problem hasn't come up with npm's tink as well. It looks like tink still looks in the node_modules folder for packages first, so I'm guessing that optional dependencies can be resolved from there even if they aren't listed in package.json anywhere.

So if we add a new field for optionalPeerDependencies (or allow it to be specified on an existing field in a way that doesn't break npm somehow), then library authors can safely adopt this new field to remain compatible with both npm and yarn, including PnP and tink. That sounds promising.

It would be nice to support other types of optional dependencies as well, especially given that these were possible prior to PnP but won't work with it. It is definitely a more difficult problem to solve, and probably outside the scope of this issue. But I think having a long-term goal in mind for other optional dependencies would be helpful in deciding which path to take on optional peer dependencies in the short term.

This discussion should be moved to an RFC. This change is definitely significant enough to warrant one.