lirantal/is-website-vulnerable

installing extraneous dependencies

travi opened this issue · 20 comments

travi commented

i noticed some peer dependency warnings about semantic-release and got curious why my non-package project had semantic-release installed at all. it looks like your dev dependencies are getting installed for some reason.

Expected Behavior

installing is-website-vulnerable should only install it and its production dependencies

Current Behavior

even though semantic-release and its plugins are only listed as dev-dependencies, at least some of the dependencies are getting installed in my project:

$ npm ls @semantic-release/github
matt.travi.org@ /path/to/my/project/matt.travi.org
└─┬ is-website-vulnerable@1.9.3
  └── @semantic-release/github@5.5.5  extraneous

Possible Solution

i havent used shrinkwrap much, but that is my best guess as to why this is happening. is it possible to only shrinkwrap prod dependencies?

Steps to Reproduce (for bugs)

  1. npm install is-website-vulnerable --save-dev
  2. npm ls @semantic-release/github (or another dev-dependency)

Context

i'm seeing peer-dependency warnings for dev-dependencies of this project that shouldnt have an impact on my project. i normally also enforce that npm ls exits with zero, which this would make fail. this particular does not fully enforce that yet, so this issue slipped through unnoticed originally

Your Environment

lirantal commented

ahh, good catch, it is due to the shrinkwrap. will take a look at fixing this, thanks!

lirantal commented

@travi which npm cli version are you running?

travi commented

I'm just using the version installed with the version of node mentioned above. I'm afk atm, but can confirm in a bit

travi commented

Regardless of whether dev dependencies should be installed or not, the warnings i see after they do get installed in my project highlight that your peer dependencies aren't compatible ;)

It looks like you're using the beta of semantic-release, but latest of the plugins. You should be using the pre-releases of those too. npm ls should help identify a compatible combination

travi commented

So if those were resolved, it would at least prevent warnings on my side. While i agree that it's odd that shrinkwrap installs the dev dependencies, i don't necessarily mind if it doesn't cause errors

lirantal commented

yep, I see those now too.
let's figure out first the shrinkwrap issue as I'd need to update that with any change regardless.

  • 👍1
lirantal commented

@travi can you try with the latest stable Node.js, or npm version 6.12.1 ?
I don't get peer dependency issues with it

lirantal commented

Nevermind, I do see it. The npm CLI is weird as it skipped those in the first install but running another install after everything is installed they will show up 🤷‍♂

github-actions commented

🎉 This issue has been resolved in version 1.9.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

lirantal commented

@travi can you check now if you're still getting peer deps issues?

travi commented

so, unfortunately, it doesn't seem to be available yet:

$ npm dist-tags ls is-website-vulnerable
latest: 1.9.3

i had a package publish earlier today that is still not available either. i sent a note to support earlier today, but the status page doesnt note any issues.

travi commented

at least i wasnt crazy: https://status.npmjs.org/incidents/lg9535dl2ff3

i'll keep an eye on the incident and test once it gets resolved

  • 👍1
travi commented

now that i could install the new version, i was able to confirm that the peer-dependency issues look good, but npm ls does still complain about the extraneous dev-dependencies, and therefore exits with 1 instead of 0

  • 👍1
lirantal commented

Yep, will see about fixing that.

travi commented

been a while since i've looked into this, so trying to remember where this landed. did we end up figuring out a way to sort out the shrinkwrap issues, or are we stuck in the state where devDependencies are getting wrapped until npm v7 is available?

travi commented

in case it helps us with context, figured it could help keeping this thread tied to this issue

lirantal commented

Indeed still need to work on this. Let me get to review this again.

github-actions commented

🎉 This issue has been resolved in version 1.14.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

lirantal commented

@travi is-website-vulnerable@1.14.1 fixes the issue now thanks to lockfile-prune

travi commented

Great work! Looks like that did it. Thanks a lot!

  • 👍1