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

fix: drop npm dependency #444

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

antongolub
Copy link
Contributor

@antongolub antongolub commented Dec 22, 2021

closes #434
relates #270

alternative #445

#434 (comment)

Suggestion: remove npm dependency. I still believe that the plugin always invokes global npm, so this dependency is completely useless.

Have a look:

const result = execa(

npm is called via execa. It is just a wrapper for child_process.exec by default. cp uses $PATH to find util ref, and it knows absolutely nothing about node_modules/.bin/npm.

git clone ... && npm i
npm -v
6.14.13

node -e "console.log(require('execa').sync('npm', ['-v']).stdout)"
6.14.13

node -e "console.log(require('./node_modules/npm/package.json').version)"
7.24.2

if we want execa to call the plugins's own npm version, we should pass preferlocal option.

node -e "console.log(require('execa').sync('npm', ['-v'], {preferLocal: true}).stdout)"
7.24.2

@the-spyke
Copy link

Please merge this. If there's a necessity to have a specific npm, it could be done through the engines property.

@gr2m gr2m changed the title fix: drop useless npm dependency fix: drop npm dependency Jan 17, 2022
@antongolub
Copy link
Contributor Author

Closed in favor of #445

@antongolub antongolub closed this Jan 18, 2022
@antongolub antongolub reopened this Jan 18, 2022
@antongolub
Copy link
Contributor Author

antongolub commented Jan 18, 2022

I've recognised that some package may bring its own npm version if needed. Reopened.

@the-spyke
Copy link

#445 Does not replace this. I don't want to see npm and it's tree in the lock file. If you don't want to drop it please make it a peerDependency

@schmidtk
Copy link

schmidtk commented Jul 7, 2022

This dependency causes an unfortunate side effect in projects using npm workspaces, because the npm dependency from this package gets hoisted to the root node_modules. Scripts using npm run will then use the locally installed version of npm from this package instead of the global install.

I described the issue in further detail here:
npm/rfcs#287 (comment)

Is the dependency truly needed in this package, or can it be either removed or made a peer dependency? Thanks for you time.

@travi
Copy link
Member

travi commented Jul 7, 2022

can it be either removed or made a peer dependency?

our recommendation is to execute semantic-release using npx so that semantic-release is only installed at execution time and not defined in your package.json or your lockfile. this will limit the impact of npm being installed as a dependency as well

This dependency causes an unfortunate side effect in projects using npm workspaces

this project does not officially support use in monorepos, so impacts to workspaces are officially out of scope. what additional tooling are you using in your project to enable use within a monorepo?

@the-spyke
Copy link

our recommendation is to execute semantic-release using npx so that semantic-release is only installed at execution time and not defined in your package.json or your lockfile.

I don't want to use npx so everything is stable and hash-verified without hacking around npm caches on CI.

@travi
Copy link
Member

travi commented Jul 7, 2022

I don't want to use npx so everything is stable and hash-verified without hacking around npm caches on CI.

you are welcome to make that choice, but that means living with the current trade-offs. we are not ready to make a decision on this change, but are considering the impacts. in the meantime, those are the available options.

@schmidtk
Copy link

schmidtk commented Jul 7, 2022

our recommendation is to execute semantic-release using npx so that semantic-release is only installed at execution time and not defined in your package.json or your lockfile. this will limit the impact of npm being installed as a dependency as well

I'll look into the global install route to avoid bloating our package lock, though we prefer the local installation for the same reasons @the-spyke identified.

this project does not officially support use in monorepos, so impacts to workspaces are officially out of scope. what additional tooling are you using in your project to enable use within a monorepo?

We wrote a simple script that gathers workspace projects with semantic-release configured and runs npm run semantic-release --workspace=<project> on each of them.

@jcnix
Copy link

jcnix commented Feb 14, 2024

I'm going to bump this because there is a CVE right now affecting npm so anyone who uses this package which pulls in npm and its tree get a High severity vulnerability tagged to them. npm bundles their dependencies so their is currently no solution other than waiting for an npm update, you cannot override the offending package via npm audit fix

GHSA-78xj-cgh5-2h22

@gr2m
Copy link
Member

gr2m commented Feb 14, 2024

regarding the CVE, see our recommendation above:
#444 (comment)

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

Successfully merging this pull request may close these issues.

npm audit - moderate vulnerabilities in deep json-schema dependency
6 participants