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(arborist): dont skip adding advisories to audit based on name/range #4735

Merged
merged 1 commit into from
Apr 13, 2022

Conversation

lukekarrys
Copy link
Contributor

When generating an audit report, a cache of seen advisories is kept to
avoid doing any repeat fanout work on its nodes. Previously this cache
was also preventing audits from being added to the report. This has been
fixed so the cache is only used to prevent extra work, but all valid
advisories are added to the output.

Fixes #4681

When generating an audit report, a cache of seen advisories is kept to
avoid doing any repeat fanout work on its nodes. Previously this cache
was also preventing audits from being added to the report. This has been
fixed so the cache is only used to prevent extra work, but all valid
advisories are added to the output.

Fixes #4681
@lukekarrys lukekarrys requested a review from a team as a code owner April 12, 2022 01:01
@npm-robot
Copy link
Contributor

found 1 benchmarks with statistically significant performance regressions

  • app-large: clean
timing results
app-large clean lock-only cache-only cache-only
peer-deps
modules-only no-lock no-cache no-modules no-clean no-clean
audit
npm@8 48.249 ±1.28 29.869 ±0.03 29.106 ±16.42 19.863 ±0.44 2.988 ±0.03 2.987 ±0.02 2.535 ±0.03 11.426 ±0.01 2.348 ±0.00 3.441 ±0.05
#4735 57.501 ±1.94 29.739 ±0.15 30.595 ±18.82 20.027 ±1.09 2.997 ±0.01 2.966 ±0.01 2.417 ±0.00 11.378 ±0.03 2.421 ±0.01 3.584 ±0.16
app-medium clean lock-only cache-only cache-only
peer-deps
modules-only no-lock no-cache no-modules no-clean no-clean
audit
npm@8 37.426 ±0.44 23.036 ±0.02 12.990 ±0.03 13.734 ±0.40 2.737 ±0.04 2.765 ±0.03 2.441 ±0.04 8.427 ±0.03 2.267 ±0.02 3.049 ±0.04
#4735 37.317 ±0.53 23.198 ±0.00 13.171 ±0.05 14.005 ±0.55 2.765 ±0.05 2.763 ±0.06 2.468 ±0.02 8.568 ±0.15 2.283 ±0.01 3.124 ±0.13

Copy link
Contributor

@nlf nlf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@fritzy fritzy merged commit aa4a4da into latest Apr 13, 2022
@fritzy fritzy deleted the lk/missing-advisories branch April 13, 2022 21:34
@lukekarrys lukekarrys mentioned this pull request Apr 14, 2022
@Ockejanssen
Copy link

Would it be possible to bring this fix also to npm 7?

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.

[BUG] npm audit doesn't show github Dependabot alerts
5 participants