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

Please choose a package manager #171

Closed
bfmiv opened this issue Apr 16, 2019 · 3 comments
Closed

Please choose a package manager #171

bfmiv opened this issue Apr 16, 2019 · 3 comments
Labels

Comments

@bfmiv
Copy link

bfmiv commented Apr 16, 2019

Lockfiles are currently (and will likely always be) out of sync. Please let me know which package manager you prefer to use going forward and I'd be happy to open a PR with the necessary changes.

Versions of relevant software used

  • node@10.15.3
  • npm@6.9.0
  • yarn@1.15.2
  • ts-protoc-gen@0.9.1-pre (master)

What happened

  • npm install results in changes to package-lock.json and reports several known vulnerabilities in dependency packages

    added 465 packages from 1179 contributors and audited 2818 packages in 6.933s
    found 5 vulnerabilities (4 moderate, 1 high)
    
    === npm audit security report ===
    # Run npm install --save-dev @bazel/karma@0.27.12 to resolve 2 vulnerabilities
    ┌───────────────┬──────────────────────────────────────────────────────────────┐
    │ Moderate      │ Prototype Pollution                                          │
    ├───────────────┼──────────────────────────────────────────────────────────────┤
    │ Package       │ lodash                                                       │
    ├───────────────┼──────────────────────────────────────────────────────────────┤
    │ Dependency of │ @bazel/karma [dev]                                           │
    ├───────────────┼──────────────────────────────────────────────────────────────┤
    │ Path          │ @bazel/karma > karma > log4js > streamroller > lodash        │
    ├───────────────┼──────────────────────────────────────────────────────────────┤
    │ More info     │ https://npmjs.com/advisories/782                             │
    └───────────────┴──────────────────────────────────────────────────────────────┘
    

    ┌───────────────┬──────────────────────────────────────────────────────────────┐ │ Moderate │ Prototype Pollution │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Package │ lodash │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Dependency of │ @bazel/karma [dev] │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Path │ @bazel/karma > karma-sauce-launcher > sauce-connect-launcher │ │ │ > lodash │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ More info │ https://npmjs.com/advisories/782 │ └───────────────┴──────────────────────────────────────────────────────────────┘

    # Run npm install --save-dev lodash@4.17.11 to resolve 1 vulnerability ┌───────────────┬──────────────────────────────────────────────────────────────┐ │ Moderate │ Prototype Pollution │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Package │ lodash │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Dependency of │ lodash [dev] │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Path │ lodash │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ More info │ https://npmjs.com/advisories/782 │ └───────────────┴──────────────────────────────────────────────────────────────┘

    # Run npm update js-yaml --depth 2 to resolve 2 vulnerabilities ┌───────────────┬──────────────────────────────────────────────────────────────┐ │ Moderate │ Denial of Service │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Package │ js-yaml │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Dependency of │ tslint [dev] │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Path │ tslint > js-yaml │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ More info │ https://npmjs.com/advisories/788 │ └───────────────┴──────────────────────────────────────────────────────────────┘

    ┌───────────────┬──────────────────────────────────────────────────────────────┐ │ High │ Code Injection │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Package │ js-yaml │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Dependency of │ tslint [dev] │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Path │ tslint > js-yaml │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ More info │ https://npmjs.com/advisories/813 │ └───────────────┴──────────────────────────────────────────────────────────────┘
  • yarn install prints

    warning package-lock.json found. Your project contains lock files generated by
    tools other than Yarn. It is advised not to mix package managers in order to
    avoid resolution inconsistencies caused by unsynchronized lock files. To clear
    this warning, remove package-lock.json.
    

What you expected to happen

  • A single source of record for dependency resolutions
@jonny-improbable
Copy link
Contributor

Thanks for raising @bfmiv; this issue is caused by the fact that our Bazel build rules rely on yarn, where's the day-to-day development uses npm; I can certainly see how this causes a problem.

My own personal preference is to stick with npm, however removing yarn would break the Bazel build. @Dig-Doug - do you know if the bazel tooling now supports npm? If so, could we consider switching to use it and remove the yarn config?

If this is not possible (or time consuming); then I can switch to yarn - but it would be good to check first.

@Dig-Doug
Copy link
Contributor

Here's the issue with npm. I tried reproducing it today and it still exists.

The bazel team also has a few other reasons for preferring yarn listed here.

@jonny-improbable
Copy link
Contributor

Yarn configuration has been removed from this repo which means this issue is now resolved.

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

No branches or pull requests

3 participants