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 publish npm-shrinkwrap.json #91

Open
balgillo opened this issue Apr 27, 2021 · 8 comments
Open

Don't publish npm-shrinkwrap.json #91

balgillo opened this issue Apr 27, 2021 · 8 comments

Comments

@balgillo
Copy link

Describe the bug
This project publishes its npm-shrinkwrap.json. That's discouraged:

It's strongly discouraged for library authors to publish this file, since that would prevent end users from having control over transitive dependency updates.

The practical impact is that electronegativity's dev dependencies are ending up in our package-lock.json (marked as either optional or extraneous, maybe depending on npm version). This is even though we are declaring electronegativity as a devDependency so its dev dependencies should be ignored. That may be caused by an npm issue. But if electronegativity didn't publish its npm-shrinkwrap.json, that bug wouldn't matter.

There are three dev dependencies that are particularly problematic because they or their dependencies have security advisories against them:

To Reproduce
Steps to reproduce the behavior:

  1. npm init in new directory
  2. In package.json add:
  "devDependencies": {
    "@doyensec/electronegativity": "^1.9.0"
  }
  1. npm install --include=dev
  2. Search in package-lock.json for "base", "chokidar", "snapdragon-node" to see optional or extraneous dependencies.

Expected behavior
Expect there to be no npm-shrinkwrap.json published with electronegativity, so its package.json is used to declare its dependencies. Only the runtime dependencies of electronegativity (and their trees of runtime dependencies) should end up in our package-lock.json.

Platform (please complete the following information):

  • OS: Windows
  • Electronegativity version: 1.9.0
@ikkisoft
Copy link
Contributor

I am under the impression that npm-shrinkwrap.json is NPM's version of yarn.lock, isn't it?

If so, this is our best way to enforce the installation of particular dependencies and avoid all "works on my machine" issues
See: https://classic.yarnpkg.com/blog/2016/11/24/lockfiles-for-all/

@balgillo
Copy link
Author

balgillo commented May 4, 2021

@ikkisoft I think it's good to have this file in your repository, but not to publish it with your library. Unlike yarn.lock, npm-shrinkwrap.json does affect your consumers, via package-lock.json (see https://docs.npmjs.com/cli/v6/commands/npm-shrinkwrap#description)

@tessro
Copy link

tessro commented May 10, 2021

We've been tracing an issue where npm seems to sporadically rewrite our package-lock.json. It seems to only happen to dependencies of Electronegativity. I suspect it's related to the use of shrinkwrap, since it isn't happening to any other libraries in our repo. Linking in case it turns out to be related: npm/cli#3225

@tessro
Copy link

tessro commented May 12, 2021

This has started causing a new problem for us as well, since npm-shrinkwrap.json includes things like fsevents v1.2.12, which causes install to fail on Windows. fsevents is a dependency of chokidar which is in shrinkwrap but not even used by electronegativity from what I can tell.

Unfortunately I think we are going to have to back out electronegativity from our testing pipeline until we can run it without pulling all these dependencies. Please consider dropping this file from the published package. 🙏🏼

@tessro
Copy link

tessro commented May 12, 2021

@ikkisoft this bit from that Yarn blog post is the key:

When you install dependencies in your application or library, only your own yarn.lock file is respected.

As @balgillo mentions, this is different from NPM, which will respect the shrinkwrap file during install.

@phosphore
Copy link
Contributor

Hello and thanks for reporting this!

Electronegativity was designed to be a command-line tool for auditors, which only later was adapted to be optionally used as a library. The documentation on NPM you cited also states:

The recommended use-case for npm-shrinkwrap.json is applications deployed through the publishing process on the registry: for example, daemons and command-line tools intended as global installs or devDependencies.

I'm convinced that the majority of our users are using it as a standalone, global install, but I'm willing to remove the shrinkwrap file for the time being and check with you if you're still experiencing problems (see v1.9.1). I reserve the right to restore it in case of reproducibility issues related to dependency discrepancies.

@tessro
Copy link

tessro commented May 24, 2021

Understood!

Have you considered separating out the library from the CLI and publishing them as separate artifacts? That might solve the problem of publishing npm-shrinkwrap.json while letting people consume the library without conflicts.

@bvandenbon
Copy link

I haven't used electronegativity. But isn't this a cli tool ?
And according to their documentation, it should be installed globally.

That means, you could just leave it out your package.json
Then it shouldn't impact the dependencies of your specific project at all.

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

No branches or pull requests

5 participants