Skip to content
This repository has been archived by the owner on Jan 20, 2022. It is now read-only.

Add strictPeerDeps, override ERESOLVE if not true #136

Closed
wants to merge 2 commits into from

Conversation

isaacs
Copy link
Contributor

@isaacs isaacs commented Sep 17, 2020

In the overwhelming majority of cases in the wild, a peer dependency
conflict that results in an ERESOLVE can be fixed by using the
--force flag.

However, this has other side effects (causing npm audit fix to install
semver-major fixes, blowing away file collisions, etc.) which might not
be desirable. Also, since it's opt-in, it means that users have to run
the install twice for something where we're pretty sure what the right
course of action is.

Let's just make that particular override the default, and reduce most
ERESOLVE errors from a crash to a warning.

(Also, a dumb fix for vuln, should probably pull that in regardless of where we land on this.)

cc: @MylesBorins

In the overwhelming majority of cases in the wild, a peer dependency
conflict that results in an ERESOLVE can be fixed by using the
`--force` flag.

However, this has other side effects (causing `npm audit fix` to install
semver-major fixes, blowing away file collisions, etc.) which might not
be desirable.  Also, since it's opt-in, it means that users have to run
the install twice for something where we're _pretty_ sure what the right
course of action is.

Let's just make that particular override the default, and reduce most
ERESOLVE errors from a crash to a warning.
@MylesBorins
Copy link

I floated this patch locally and attempted to reify two known failing modules radium and serialport. Both of those modules now reify without error 🎉

I'm a huge fan of this change, it will give the community some time to adjust and update repos.

@ljharb
Copy link
Contributor

ljharb commented Sep 17, 2020

@isaacs does this mean that invalid peer deps will no longer fail installs?

@ruyadorno
Copy link
Contributor

ruyadorno commented Sep 17, 2020

@ljharb it should no longer fail most of the time. It's not the same as --legacy-peer-deps though, this change is just making it so that the current behavior we had with --force becomes the default, which means: it may still fail in the case of an unsolvable peer dep conflict but the vast majority of cases it will simply resolve the tree assuming the top-most declaration for that peer dep is the one that counts.

It will WARN with that big pretty-formatted ERESOLVE stack explaining precisely what the conflict is, nudging users/lib authors/maintainers (the ecosystem in general) into fixing these borked peer deps down the install tree.

@ljharb
Copy link
Contributor

ljharb commented Sep 17, 2020

To be clear, when npm ls would return nonzero, I would want the install to always fail. Is that property still preserved?

@isaacs
Copy link
Contributor Author

isaacs commented Sep 17, 2020

Yes, we'll be setting strictPeerDeps: true explicitly in npm ls.

@isaacs isaacs closed this in fe0a5f6 Sep 17, 2020
isaacs added a commit to npm/cli that referenced this pull request Sep 17, 2020
@darcyclarke darcyclarke added the Agenda will be discussed at the Open RFC call label Sep 18, 2020
ruyadorno pushed a commit to npm/cli that referenced this pull request Sep 22, 2020
This is the CLI portion of npm/arborist#136

PR-URL: #1819
Credit: @isaacs
Close: #1819
Reviewed-by: @ruyadorno
@darcyclarke darcyclarke added this to the OSS - Sprint 15 milestone Sep 25, 2020
@darcyclarke darcyclarke removed the Agenda will be discussed at the Open RFC call label Oct 7, 2020
@wraithgar wraithgar deleted the isaacs/strict-peer-deps branch April 22, 2021 17:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants