Add a proposal to deal with peer dependency noise.

[alloy]

⚠️ ⚔️

FYI I’d love to work on this, but didn’t want to start before this was agreed upon and how it would work.

[alloy]

@bestander Could you tell me what/if I should be doing next to receive feedback on this?

[arcanis]

Sorry, I missed your RFC - we've been discussing a similar feature (optional peer dependencies) here: yarnpkg/yarn#6487

[alloy]

While I do think having an explicit way to mark peer dependencies as optional is a good thing to have, that issue seems largely orthogonal to mine.

My proposal is not about labelling peer dependencies of a package as optional or dealing with side-effects of implementation details (of PnP), but rather a way for a consuming project to make decisions based on warnings and deal with them. This is something that, from my perspective, is needed regardless of the other issue being implemented or not.

Having said that, my proposal would probably allow for dealing with the PnP issue in a more timely manner, because even after adding a way for packages to list optional peer dependencies it’s going to take a while for all packages to use them. (Some packages may never use the feature while actually having optional peer dependencies.)

Off-topic: I’m slightly surprised about that other issue not being discussed as an RFC. Did I misinterpret what types of things should be discussed as RFCs?

[Gudahtt]

This proposal seems reasonable. It solves the immediate problem of unwanted peer dependency warnings pretty well, with few drawbacks.

Unfortunately this doesn't get to the root of the problem, which is that peer dependency warnings exist in the first place in situations where they aren't addressable. But fixing that problem is a lot more difficult. Perhaps we can talk about that for a moment here.

Which situations have this condition of peer dependency warnings that are safe to ignore? I'm not convinced by the example given of the version constraint being too restrictive. The library author has the ability to make whatever restriction they want on the version of a peer dependency. In that situation, typically I would think that the library should be updated or forked, rather than this warning be ignored.

"peer dependencies" are intended for mandatory dependencies. Usually when I see a peer dependency warning that is safe to ignore, it's because the peer dependency is optional. An optional dependency is not expressible right now (whether it's a "main" dependency, a devDependency or a peerDependency). The misleadingly-named optionalDependencies don't solve this problem for either of those three categories. They just allow certain packages to be allowed to fail during installation, which primarily is used to facilitate platform-specific dependencies. Optional dependencies that are opt-in for the package consumer are inexpressible.

Is that the primary case where peer dependency warnings can be ignored, or are there others?

And about that situation... is using peerDependencies for optional peer dependencies advisable, or is there a better alternative? Personally I have recommended that library authors leave truly "optional" packages out of package.json altogether, and document them instead.

Edit: Oh... right. Reading into that linked issue, it looks like purely documented optional dependencies would result in Yarn PnP never being able to find them. That's a problem.

I’m slightly surprised about that other issue not being discussed as an RFC. Did I misinterpret what types of things should be discussed as RFCs?

Nope - this issue is perfect for an RFC. You interpreted that correctly. Because the criteria for what should or should not be an RFC are fuzzy, we aren't always strict about moving RFC-worthy discussions into this repo - particularly when they arose organically from a discussion elsewhere. Also, RFC's are for proposals - discussion often occurs elsewhere when a specific solution hasn't been fleshed out enough to be written up as a proposal yet. But you're right, that decision in the linked thread should be made into an RFC as well.

[Gudahtt]

If we decide to add an optionalPeerDependencies field or support an custom version range syntax for optional peerDependencies (as discussed here), would we still want this new ignoredPeerDependencies field as well?

I'm leaning toward yes, because library authors won't all make use of this hypothetical new Yarn-specific feature. Ignoring peer dependency warnings would still be useful for application authors that use libraries that are "misbehaving" either by declaring an overly-restrictive version on a peerDependency or by including a peerDependency that isn't required at all.

[alloy]

I'm not convinced by the example given of the version constraint being too restrictive. The library author has the ability to make whatever restriction they want on the version of a peer dependency. In that situation, typically I would think that the library should be updated or forked, rather than this warning be ignored.

In theory I absolutely agree with you. In reality, however, I already maintain so many forks and try to get those in upstream, that sometimes you need to pick your battles. Even if I wanted to, in more than one case in the past have I had trouble getting a library maintainer to release a new patch release of an older major/minor line (there may be a good reason for me to run an older version).

Ultimately, what it boils down to for me is that while I absolutely would like all version requirements to be perfect, even going forward, the reality is that there will always be reasons for me to make a different decision for my project. So when I have verified that a certain version works absolutely fine with a different transitive dependency then imo I should ultimately be in control over my workspace and be able to call it a day.

"peer dependencies" are intended for mandatory dependencies. Usually when I see a peer dependency warning that is safe to ignore, it's because the peer dependency is optional.

I didn’t even know they were strictly mandatory. I have definitely used peer dependencies as optionals, in rare cases. TIL, thanks.

Nonetheless, yeah they absolutely are being used so by more people than me right now and I think there should be a way to express that separately, so I’m 👍 on yarnpkg/yarn#6487.

(It’s really unfortunate that optionalDependencies is being used for those packages whose installation is known to be precarious.)

[alloy]

If we decide to add an optionalPeerDependencies field or support an custom version range syntax for optional peerDependencies, would we still want this new ignoredPeerDependencies field as well?

I'm leaning toward yes, because library authors won't all make use of this hypothetical new Yarn-specific feature. Ignoring peer dependency warnings would still be useful for application authors that use libraries that are "misbehaving" either by declaring an overly-restrictive version on a peerDependency or by including a peerDependency that isn't required at all.

Agreed. This goes back to “[I should] ultimately be in control over my workspace and be able to call it a day”.

Put otherwise, this field is for app/project authors to override based on their informed needs, not for library authors.

[Gudahtt]

I didn’t even know they were strictly mandatory.

Well.... they're intended to support plugins, which is a situation in which the peer is required. A lot of people use them for optional packages as well, so I probably worded that too strongly. I don't know what the authors of the original implementation thought about optional peer dependencies, just that it doesn't deal with them very well right now.

Put otherwise, this field is for app/project authors to override based on their informed needs, not for library authors.

That's a great way of explaining it, thanks.

[alloy]

(Btw, I now realise that I never stated that I’d love to work on this, but didn’t want to start before this was agreed upon and how it would work. Updated the OP.)

[arcanis]

I didn’t even know they were strictly mandatory. I have definitely used peer dependencies as optionals, in rare cases. TIL, thanks.

Peer dependencies are not strictly mandatory - it's just that, if missing, we'll print a warning. We won't crash the install or anything - instead it will throw at runtime if accessing the peer dependency without guard. If you have a guard, then it's perfectly fine.

The warning unfortunately leads to suboptimal UX for packages that have optional integrations with others, so we're looking into ways to allow packages to signal Yarn that the warning is unnecessary.

Ignoring peer dependency warnings would still be useful for application authors that use libraries that are "misbehaving" either by declaring an overly-restrictive version on a peerDependency or by including a peerDependency that isn't required at all.

I think the best feature would be to rework the logging to assign codes + data to the various warnings, and allow our users to select which ones then want to disable from the yarnrc. I'm not fond of anything that would only work for the peer dependencies case.

[ljharb]

Anything that makes npm ls exit nonzero is indeed mandatory - you can’t rely on anything working when your dep graph is invalid. Thus, peer deps are indeed conceptually mandatory, even though it won’t fail npm or yarn install.

[arcanis]

I disagree - the behavior when a peer dependency is missing is clearly defined, so "you can't rely on anything working" isn't quite true. You can rely on not being able to access a missing peer dependency it (ideally for regular installs, enforced with PnP), and you can rely on being able to access any of your other dependencies. It's not an UB that could potentially mess your whole tree.

Whether npm ls or yarn check exit nonzero doesn't really matter - both of those commands have a different semantic than an install, and can afford to be stricter and exit with non-zero codes when a regular install wouldn't (whether they should is a different story).

[Gudahtt]

I think the best feature would be to rework the logging to assign codes + data to the various warnings, and allow our users to select which ones then want to disable from the yarnrc. I'm not fond of anything that would only work for the peer dependencies case.

I don't think storing this configuration in yarnrc would be a great idea. Ignoring peer dependency warnings should be a project-specific configuration, whereas yarnrc is meant for user-specific configuration.

As for how to ignore the warnings, the information we'd want from the user to decide which peer dependencies to ignore is exactly as described by this RFC. We'd want to know which dependency had the peer dependency, and what that peer dependency was exactly (including the version range). Anything less than that, and we would end up silencing warnings we shouldn't.

That is, the configuration would be specific to peer dependencies, and can't be generalized to fit other situations in which you might want to ignore warnings.

Though, if you do think that ignoring warnings on a per-project basis might be desired in other situations and you want to group the configuration together, we could consider something like this instead:

{
    "ignoredWarnings": {
        "peerDependencies": {
            "styled-reset": ["styled-components@>=4.0.0"]
        }
    }
}

I don't know of any other examples of warnings that should be ignored for a project though. Usually warnings indicate a problem that can be fixed.

[arcanis]

Ignoring peer dependency warnings should be a project-specific configuration, whereas yarnrc is meant for user-specific configuration.

The yarnrc is meant for both. If you have multiple yarnrc in your filesystem hierarchy, they'll all be loaded, from the root one to the closest one. It's a technic that's really handy precisely to define project-level configuration settings, such as the custom registry or a custom Yarn binary (we actually recommend using it to enforce the Yarn version a given team will use, regardless of their global one).

As for how to ignore the warnings, the information we'd want from the user to decide which peer dependencies to ignore is exactly as described by this RFC. We'd want to know which dependency had the peer dependency, and what that peer dependency was exactly (including the version range). Anything less than that, and we would end up silencing warnings we shouldn't.

Yep, that's why I mentioned adding metadata to the warnings. So for example, silencing the peer dependency warning could look like this:

ignore-warnings:
  missing-peer-dependency:
    - name: typescript
      expected-by: ts-node

We would then add the logic in the logger to check whether the given error/metadata have been ignored in the settings or not. That fits much better than adding a specific logic to peer dependencies, imo.

I don't know of any other examples of warnings that should be ignored for a project though. Usually warnings indicate a problem that can be fixed.

A classic one is "Missing license", but there are others.

[Gudahtt]

That all seems pretty reasonable!

Going forward, I suppose yarnrc should be preferred for new project configuration over package.json? It seems safer to add things to yarnrc - less likely to break existing packages that might rely upon package.json configuration.

[alloy]

I like that, moving this to yarnrc, it makes the point of this being for project authors, as opposed to library authors, much clearer.

[arcanis]

Going forward, I suppose yarnrc should be preferred for new project configuration over package.json? It seems safer to add things to yarnrc - less likely to break existing packages that might rely upon package.json configuration.

Yup - the main recent exception was the recent installConfig.pnp flag, which I put in the package.json to encourage interoperability with other package managers, but overall the yarnrc is a good place to keep all project-level settings 🙂

[Gudahtt]

I have a couple more questions.

ignore-warnings:
  missing-peer-dependency:
    - name: typescript
      expected-by: ts-node

That format looks pretty good, but it is missing the version range. Including the range would be useful in the case where the warning is appearing due to a version mismatch. Then Yarn could show the error again if the expected version changed, which might be helpful.

The version range would be pretty useless for the case where the peer dependency is totally optional though. If you never want to use a peer dependency, you might not care what version range is being asked for.

So perhaps the range should be optional.

Second question: should this configuration be exposed via the CLI? The yarn config command is key-value oriented, which doesn't work well here.

Maybe we could add a new subcommand for ignoring warnings (e.g. yarn config ignore-warning <type> [--name <name> --expected-by <expected-by>]. Or we could add an interactive prompt. That does seem a bit complex though, and not strictly necessary. I did want to bring it up because this might be the first yarnrc config that can't easily be specified via yarn config.

[alloy]

That format looks pretty good, but it is missing the version range. Including the range would be useful in the case where the warning is appearing due to a version mismatch. Then Yarn could show the error again if the expected version changed, which might be helpful.

The version range would be pretty useless for the case where the peer dependency is totally optional though. If you never want to use a peer dependency, you might not care what version range is being asked for.

So perhaps the range should be optional.

This is what I actually presumed was meant, but it’s indeed good to have it explicit. I do think we should be able to specify the version range.

[Gudahtt]

It sounds like we've reached a consensus on the configuration being stored in yarnrc instead of in the manifest. Could you update the RFC to reflect this?

[alloy]

Sure thing 👍 Will do this next week when I’m back home.

[epmatsw]

If we've got configuration setting to ignore peerDependency issues, would it make sense to add a similar option to make unaddressed peerDependency issues present as errors instead of warnings? If we have this outlet to ignore them, making unexpected/un-ignored incompatibilities fail loudly until ignored seems like something that would be nice to have.

[arcanis]

Similarly, I think I'd be more in favor of a -Werror-like option where warnings would be treated as errors. Would be more generic than --enforce-peer-dependencies or similar 😃

[alloy]

Alright, updated as per the discussion and added a few more unresolved questions.

[alloy]

Hey all, no rush on my side –I have plenty to do– but just wondering what the next step is so that I’m not accidentally waiting on nothing?