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

Don't override user specified depth in outdated #239

Closed
wants to merge 1 commit into from
Closed

Don't override user specified depth in outdated #239

wants to merge 1 commit into from

Conversation

G-Rath
Copy link
Contributor

@G-Rath G-Rath commented Aug 25, 2019

Restores ability to update packages using --depth as suggested by npm audit.

i.e npm update eslint-utils --depth 2.


I've restored the previous conditional check, which was against Infinity; however I've never worked in npm/cli before, so don't know if that value still holds water :)

How it is currently means the npm update <package> --depth <depth> commands suggested by npm audit won't work, as any depth value is clobbered to 0 if it's anything but, resulting in empty output.

This is the related community bug report.


Since this was included in v6.6.0 onwards, it effects all versions of Node from 11.10.0 onwards, as well as 10.16.0+

Restores ability to update packages using `--depth` as suggested by `npm audit`.

i.e `npm update eslint-utils --depth 2`.
@G-Rath G-Rath requested a review from a team as a code owner August 25, 2019 07:42
@shadowspawn
Copy link
Contributor

I came across another PR related to infinity regressions which casts a little light on the original use of the infinity magic value to have different default behaviours in different commands: npm/npm#11726

(And like @G-Rath, not sure if that value is still relevant!)

@isaacs isaacs self-assigned this Aug 26, 2019
@isaacs isaacs added the semver:patch semver patch level for changes label Aug 26, 2019
@isaacs
Copy link
Contributor

isaacs commented Aug 26, 2019

Thanks, this is definitely a bug.

@billatnpm
Copy link

https://docs.npmjs.com/misc/config#depth

For npm outdated, a setting of Infinity will be treated as 0 since that gives more useful information. To show the outdated status of all packages and dependents, use a large integer value, e.g., npm outdated --depth 9999

@claudiahdz claudiahdz closed this in 235ed1d Sep 3, 2019
@G-Rath G-Rath deleted the patch-1 branch September 3, 2019 22:31
jaredhirsch added a commit to mozilla/fxa that referenced this pull request Nov 14, 2019
* Add a lint:deps job to the top-level package.json, so lerna can run
  lint:deps in all packages in parallel.

* Also fix today's handlebars vulnerability, so that builds don't fail.

Some of the vulnerabilities are in transitive dependencies, yet the
suggested `npm update foo --depth N` command sometimes seems to do
nothing. There was a related bug in npm 6.6.0 - 6.11.2, fixed by
npm/cli#239, but perhaps that didn't fix all the
cases? As a workaround, I've added exceptions where npm wasn't able to
fixup vulnerabilities.

Fixes #2229.
jaredhirsch added a commit to mozilla/fxa that referenced this pull request Nov 14, 2019
* Add a lint:deps job to the top-level package.json, so lerna can run
  lint:deps in all packages in parallel.

* Also fix today's handlebars vulnerability, so that builds don't fail.

Some of the vulnerabilities are in transitive dependencies, yet the
suggested `npm update foo --depth N` command sometimes seems to do
nothing. There was a related bug in npm 6.6.0 - 6.11.2, fixed by
npm/cli#239, but perhaps that didn't fix all the
cases? (I was using npm 6.12.0.) As a workaround, I've added audit-filter
exceptions where `npm update` wasn't able to fix vulnerabilities.

Fixes #2229.
jaredhirsch added a commit to mozilla/fxa that referenced this pull request Nov 15, 2019
* Add a lint:deps job to the top-level package.json, so lerna can run
  lint:deps in all packages in parallel.

* Also fix today's handlebars vulnerability, so that builds don't fail.

Some of the vulnerabilities are in transitive dependencies, yet the
suggested `npm update foo --depth N` command sometimes seems to do
nothing. There was a related bug in npm 6.6.0 - 6.11.2, fixed by
npm/cli#239, but perhaps that didn't fix all the
cases? (I was using npm 6.12.0.) As a workaround, I've added audit-filter
exceptions where `npm update` wasn't able to fix vulnerabilities.

Fixes #2229.
@cloakedninjas
Copy link

cloakedninjas commented Nov 18, 2019

Seem to be experiencing this in npm 6.13.0

$ npm audit
...
found 14 vulnerabilities
....
# Run  npm update https-proxy-agent --depth 7  to resolve 9 vulnerabilities
....

npm update https-proxy-agent --depth 7
No output, a further npm audit reports the same 14 vulnerabilities

Update
Nevermind, blowing out node_modules and re-running solved the issue 🤷‍♂

jaredhirsch added a commit to mozilla/fxa that referenced this pull request Nov 18, 2019
* Add a lint:deps job to the top-level package.json, so lerna can run
  lint:deps in all packages in parallel.

* Also handle recent handlebars vulnerability, so that builds don't fail.

* Note, the lint:deps job is a no-op in fxa-amplitude-send, as I can't
  get it to build yet in the monorepo.

Some of the vulnerabilities are in transitive dependencies, yet the
suggested `npm update foo --depth N` command sometimes seems to do
nothing. There was a related bug in npm 6.6.0 - 6.11.2, fixed by
npm/cli#239, but perhaps that didn't fix all the
cases? (I was using npm 6.12.0.) As a workaround, I've added audit-filter
exceptions where `npm update` wasn't able to fix vulnerabilities.

Fixes #2229.
jaredhirsch added a commit to mozilla/fxa that referenced this pull request Nov 18, 2019
* Add a lint:deps job to the top-level package.json, so lerna can run
  lint:deps in all packages in parallel.

* Also handle recent handlebars vulnerability, so that builds don't fail.

* Note, the lint:deps job is a no-op in fxa-amplitude-send, as I can't
  get it to build yet in the monorepo.

Some of the vulnerabilities are in transitive dependencies, yet the
suggested `npm update foo --depth N` command sometimes seems to do
nothing. There was a related bug in npm 6.6.0 - 6.11.2, fixed by
npm/cli#239, but perhaps that didn't fix all the
cases? (I was using npm 6.12.0.) As a workaround, I've added audit-filter
exceptions where `npm update` wasn't able to fix vulnerabilities.

Fixes #2229.
jaredhirsch added a commit to mozilla/fxa that referenced this pull request Nov 18, 2019
* Add a lint:deps job to the top-level package.json, so lerna can run
  lint:deps in all packages in parallel.

* Also handle recent handlebars vulnerability, so that builds don't fail.

* Note, the lint:deps job is a no-op in fxa-amplitude-send, as I can't
  get it to build yet in the monorepo.

Some of the vulnerabilities are in transitive dependencies, yet the
suggested `npm update foo --depth N` command sometimes seems to do
nothing. There was a related bug in npm 6.6.0 - 6.11.2, fixed by
npm/cli#239, but perhaps that didn't fix all the
cases? (I was using npm 6.12.0.) As a workaround, I've added audit-filter
exceptions where `npm update` wasn't able to fix vulnerabilities.

Fixes #2229.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver:patch semver patch level for changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants