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

[BUG] another peerOptional ERESOLVE case #236

Closed
isaacs opened this issue Feb 17, 2021 · 1 comment · Fixed by #237
Closed

[BUG] another peerOptional ERESOLVE case #236

isaacs opened this issue Feb 17, 2021 · 1 comment · Fixed by #237
Assignees

Comments

@isaacs
Copy link
Contributor

isaacs commented Feb 17, 2021

Missed a case in #223

root -> (x, z@1)
x -> PEEROPTIONAL(y)
y -> PEER(z@2)

This is subtly similar to case A

case A, hidden to not be confusing
# case a
root -> (x, y@1)
x -> PEEROPTIONAL(z)
z -> PEER(y@2)

But, with the names spelled in such a way that the conflicted dep is not present in the peerSet tree until after the not-required peerOptional meta-peer is added. (They are sorted alphabetically for consistency.)

This code path wasn't being hit previously, and it's what prevents [email protected] from being installed alongside typescript@4. Ie,

x = [email protected]
y = tsickle
z@1 = typescript@4
z@2 = typescript@~3.9.5

What / Why

n/a

When

  • n/a

Where

  • n/a

How

Current Behavior

  • n/a

Steps to Reproduce

  • n/a

Expected Behavior

  • n/a

Who

  • n/a

References

  • n/a
@isaacs isaacs self-assigned this Feb 17, 2021
@isaacs
Copy link
Contributor Author

isaacs commented Feb 17, 2021

Ok, took a bit more work to get a reproduction case that actually is broken in the same way. Got to it with this:

root -> (x, z@1)
x -> PEEROPTIONAL(y) PEER(z@1) PEER(w@1)
w -> PEER(z@1)
y -> PEER(z@2)

PR incoming.

isaacs added a commit that referenced this issue Feb 17, 2021
case D

```
root -> (x, z@1)
x -> PEEROPTIONAL(y)
y -> PEER(z@2)
```

case E

```
root -> (x, z@1)
x -> PEEROPTIONAL(y) PEER(z@1)
y -> PEER(z@2)
```

case F

```
root -> (x, z@1)
x -> PEEROPTIONAL(y) PEER(z@1) PEER(w@1)
w -> PEER(z@1)
y -> PEER(z@2)
```

Case F properly reproduces the failure seen by installing
[email protected] alongside typescript@4.

Fix: #236

cc: @kyliau
isaacs added a commit that referenced this issue Feb 17, 2021
case D

```
root -> (x, z@1)
x -> PEEROPTIONAL(y)
y -> PEER(z@2)
```

case E

```
root -> (x, z@1)
x -> PEEROPTIONAL(y) PEER(z@1)
y -> PEER(z@2)
```

case F

```
root -> (x, z@1)
x -> PEEROPTIONAL(y) PEER(z@1) PEER(w@1)
w -> PEER(z@1)
y -> PEER(z@2)
```

Case F properly reproduces the failure seen by installing
[email protected] alongside typescript@4.

Fix: #236

cc: @kyliau
@isaacs isaacs closed this as completed in f25367d Feb 18, 2021
isaacs added a commit to npm/cli that referenced this issue Feb 18, 2021
* [#1875](#1875)
  [npm/arborist#230](npm/arborist#230) Set default
  advisory `severity`/`vulnerable_range` when missing from audit endpoint
  data ([@isaacs](https://github.com/isaacs))
* [npm/arborist#231](npm/arborist#231) skip
  optional deps with mismatched platform or engine
  ([@nlf](https://github.com/nlf))
* [#2251](#2251) Unpack shrinkwrapped deps
  not already unpacked ([@isaacs](https://github.com/isaacs),
  [@nlf](https://github.com/nlf))
* [#2714](#2714) Do not write package.json
  if nothing changed ([@isaacs](https://github.com/isaacs))
* [npm/rfcs#324](npm/rfcs#324) Prefer peer over
  prod dep, if both specified ([@isaacs](https://github.com/isaacs))
* [npm/arborist#236](npm/arborist#236) Fix
  additional peerOptional conflict cases
  ([@isaacs](https://github.com/isaacs))
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 a pull request may close this issue.

1 participant