Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sinon 9.0.0 installs duplicate dependencies #2224

Closed
asapach opened this issue Feb 19, 2020 · 6 comments
Closed

Sinon 9.0.0 installs duplicate dependencies #2224

asapach opened this issue Feb 19, 2020 · 6 comments

Comments

@asapach
Copy link

asapach commented Feb 19, 2020

Describe the bug
sinon@9.0.0 depends on @sinonjs/formatio@5.0.0 directly, but also on @sinonjs/formatio@4.0.1 transitively via nise. Similar for @sinonjs/samsam@5.0.1 and @sinonjs/samsam@4.2.2

To Reproduce
Steps to reproduce the behavior:

  1. npm i -D sinon
  2. npm ls
  3. See output and notice that not all packages have been deduped.
`-- sinon@9.0.0
  +-- @sinonjs/commons@1.7.1
  | `-- type-detect@4.0.8
  +-- @sinonjs/fake-timers@6.0.0
  | `-- @sinonjs/commons@1.7.1 deduped
  +-- @sinonjs/formatio@5.0.0    <--here
  | +-- @sinonjs/commons@1.7.1 deduped
  | `-- @sinonjs/samsam@4.2.2    <--here
  |   +-- @sinonjs/commons@1.7.1 deduped
  |   +-- lodash.get@4.4.2 deduped
  |   `-- type-detect@4.0.8 deduped
  +-- @sinonjs/samsam@5.0.1    <--here
  | +-- @sinonjs/commons@1.7.1 deduped
  | +-- lodash.get@4.4.2
  | `-- type-detect@4.0.8 deduped
  +-- diff@4.0.2
  +-- nise@4.0.1
  | +-- @sinonjs/commons@1.7.1 deduped
  | +-- @sinonjs/fake-timers@6.0.0 deduped
  | +-- @sinonjs/formatio@4.0.1    <--here
  | | +-- @sinonjs/commons@1.7.1 deduped
  | | `-- @sinonjs/samsam@4.2.2    <--here
  | |   +-- @sinonjs/commons@1.7.1 deduped
  | |   +-- lodash.get@4.4.2 deduped
  | |   `-- type-detect@4.0.8 deduped
  | +-- @sinonjs/text-encoding@0.7.1
  | +-- just-extend@4.0.2
  | `-- path-to-regexp@1.8.0
  |   `-- isarray@0.0.1
  `-- supports-color@7.1.0
    `-- has-flag@4.0.0

Expected behavior
There should be no duplicate dependencies.

  • Library version: 9.0.0
@fatso83
Copy link
Contributor

fatso83 commented Feb 20, 2020

Does this cause issues for you? Although we do try to keep these versions in sync, there is strictly speaking no need to. One downside is that browser builds might be a tad bit larger, but it's not a library that is included for production builds anyway, so ... 🤷‍♂

@asapach
Copy link
Author

asapach commented Feb 20, 2020

It does not present any immediate issues, other than the inconvenience of wasted resources, but in the future it could produce inconsistent results since those package versions will eventually deviate.

mroderick added a commit to mroderick/samsam that referenced this issue Feb 20, 2020
This will remove some duplication in Sinon, see sinonjs/sinon#2224
mroderick added a commit to sinonjs/samsam that referenced this issue Feb 20, 2020
This will remove some duplication in Sinon, see sinonjs/sinon#2224
mroderick added a commit to sinonjs/formatio that referenced this issue Feb 20, 2020
This helps dedupe dependencies in Sinon. See sinonjs/sinon#2224
mroderick added a commit to mroderick/nise that referenced this issue Feb 20, 2020
This helps dedupe dependencies when installing Sinon. See sinonjs/sinon#2224
mroderick added a commit to mroderick/nise that referenced this issue Feb 20, 2020
This will help dedupe dependencies. See sinonjs/sinon#2224
mroderick added a commit to sinonjs/nise that referenced this issue Feb 20, 2020
This helps dedupe dependencies when installing Sinon. See sinonjs/sinon#2224
mroderick added a commit to sinonjs/nise that referenced this issue Feb 20, 2020
This will help dedupe dependencies. See sinonjs/sinon#2224
@mroderick
Copy link
Member

In the PRs listed above, I've updated the libraries that Sinon depends on, so they can be deduped properly.

When installing Sinon, I now see this:

$ npm ls
scratch@1.0.0 /Users/morgan/code/scratch
└─┬ sinon@9.0.0
  ├─┬ @sinonjs/commons@1.7.1
  │ └── type-detect@4.0.8
  ├─┬ @sinonjs/fake-timers@6.0.0
  │ └── @sinonjs/commons@1.7.1 deduped
  ├─┬ @sinonjs/formatio@5.0.1
  │ ├── @sinonjs/commons@1.7.1 deduped
  │ └── @sinonjs/samsam@5.0.2 deduped
  ├─┬ @sinonjs/samsam@5.0.2
  │ ├── @sinonjs/commons@1.7.1 deduped
  │ ├── @sinonjs/formatio@5.0.1 deduped
  │ ├── lodash.get@4.4.2
  │ └── type-detect@4.0.8 deduped
  ├── diff@4.0.2
  ├─┬ nise@4.0.2
  │ ├── @sinonjs/commons@1.7.1 deduped
  │ ├── @sinonjs/fake-timers@6.0.0 deduped
  │ ├── @sinonjs/formatio@5.0.1 deduped
  │ ├── @sinonjs/text-encoding@0.7.1
  │ ├── just-extend@4.0.2
  │ └─┬ path-to-regexp@1.8.0
  │   └── isarray@0.0.1
  └─┬ supports-color@7.1.0
    └── has-flag@4.0.0

@asapach
Copy link
Author

asapach commented Feb 20, 2020

@mroderick, thanks for addressing this. I've left a comment in the nise repo

@asapach asapach closed this as completed Feb 20, 2020
@fatso83
Copy link
Contributor

fatso83 commented Feb 20, 2020

it could produce inconsistent results

I am not convinced of this. Each package depends on a separate version, and runs tests against that version, so no surprises should happen. And it has never occurred AFAIK this far in the history of Sinon. So yeah, wasted resourced (2kb) is the only thing that could present a minor nuisance AFAIK.

@mantoni
Copy link
Member

mantoni commented Feb 21, 2020

There is a potential issue with matchers. The isMatcher detection is based on the matcher prototype and that's a different instance for different versions. So if there is a case where isMatcher is called on one version of samsam with a matcher created by another version, the check would fail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants