v0.9.0: shinkwrap causes all dev dependencies to be installed

[boneskull]

When I run npm install markdownlint-cli2@0.8.1, I get markdownlint-cli2 and all its dependencies.

When I run npm install markdownlint-cli2@0.9.0, I get this and all of the dev dependencies.

I don't recommend using a shrinkwrap, but if you must, you will want to run npm prune --production before generating it.

[boneskull]

Note that this is especially problematic if, say, one is using Linux and the shrinkwrap wants fsevents to be installed (which fails brilliantly). Evidently you're on a Mac. 😉

[DavidAnson]

Thanks for the information. I initially fought this, but was persuaded that reproducibility is a good quality. If you care, you can see the relevant issue here: #186

I try to avoid problems like this by running CI on all supported versions of Node on all supported platforms. Unfortunately, I don't think I have a test that packs and then installs from that tarball which sounds like it would have been necessary to reproduce this error.

I will try something like what you recommend or perhaps just a production–only install before generating it. It would be nice npm handle this scenario better; the current behavior seems inconsistent and problematic to me for obvious reasons.

I expect to publish a patch release in a day or two. Sorry for the trouble!

[DavidAnson]

By the way, what happens when you run npm install --production ...? Does it still install the problematic dev dependencies?

[DavidAnson]

BTW, I DO have a relevant test on Ubuntu that passes, but it uses a custom command line that seems to opt out of the problem constraints: sudo npm install --global --no-package-lock --production markdownlint-cli2. That said, I would expect that using one or both of those flags during install should make this scenario work for you as well.

[thoroc]

--no-package-lock is not an option when using on the CI though. It'll prevent using or generating a package lock file.

[boneskull]

I try to avoid problems like this by running CI on all supported versions of Node on all supported platforms. Unfortunately, I don't think I have a test that packs and then installs from that tarball which sounds like it would have been necessary to reproduce this error.

You may be interested in midnight-smoker--which uses markdownlint-cli2 itself--where I discovered the problem. ;)

(Also related: boneskull/midnight-smoker#306)

[boneskull]

By the way, what happens when you run npm install --production ...? Does it still install the problematic dev dependencies?

For the initial install (npm install markdownlint-cli2 --production --save-dev), yes. But once I run npm install in my working copy, I get everything. And if I run npm install --production instead, then I don't get dev dependencies at all.

[boneskull]

My 2c: Shrinkwrap is a tradeoff of reproducibility vs user control. A shrinkwrap means that transitive dependencies cannot be deduped, hoisted, nor overridden by the consumer; this makes the consumer wholly dependent upon the shrinkwrapped dep to remediate any security vulnerabilities or critical bugs.

While it may be your preference, markdownlint-cli2 is not a program that needs to be shrinkwrapped.

package-lock.json is halfway between a shrinkwrap and no lockfile at all. It will give you visibility into your own development and CI environment and the opportunity to audit what's in there.

While an end user may not get the same transitive dependency tree, you can be reasonably assured your package works as expected by a regular cadence of CI checks, pinned dependencies (no semver ranges), automated dependency updates (I recommend Renovate), and (shameless plug) midnight-smoker.

[DavidAnson]

I will try midnight-smoker, it sounds like a great addition! In fact, I will make sure it detects this problem as part of the initial commit. :)

Regarding shrinkwrap, you can follow my thought process in that linked issue. I do not see value in package-lock, but it seemed to me the intent of shrinkwrap was reasonable for the task of knowing exactly what dependencies have been tested/validated. I will spend some time this evening experimenting and hope that I can find a configuration that doesn't cause problems.

[boneskull]

FWIW this is exactly how to reproduce the problem:

# updates package-lock.json, but does not install fsevents because it's optional
$ npm install markdownlint-cli2@0.9.0
# clean install using package-lock.json as source of truth
$ npm ci
npm ERR! code EBADPLATFORM
npm ERR! notsup Unsupported platform for fsevents@2.3.3: wanted {"os":"darwin"} (current: {"os":"linux"})
...

[DavidAnson]

I'll try to fix this, but I'm not unreasonable claiming it's an npm problem, am I?? It's responsible for everything here and it's the only tool involved!

[DavidAnson]

Okay... so... good news? This evening the CI Action for this project started failing on Ubuntu and Windows with the error/message you show above. This is AFTER I made a fairly trivial commit which DID involve freshening npm-shrinkwrap.json which DID get much bigger EVEN THOUGH I ran the same command to do so as a day ago: rm npm-shrinkwrap.json && npm install && npm shrinkwrap. Which is all a little spooky, but I'm glad I can reproduce the problem without jumping through hoops. (Especially since my idea to use a GitHub Codespace for Ubuntu access is failing right now because they're seemingly mounting the entire file system read-only...)

[DavidAnson]

@boneskull, I wonder if you can help me out with midnight-smoker...

I wanted to start by using it to demonstrate the problem you report. So I added it to the CI workflow as a single commit on the v0.9.0 tag which corresponds to the release you're using above (but feel free to verify that). You can see the change here: v0.9.0...midsmo

However, two things seem off:

1. I expect the test to fail on Ubuntu because of the problem you report - but it passes.
2. midnight-smoker itself seems to fail to run on Windows due to a script error.

Have I misconfigured anything? Are my expectations wrong?

[DavidAnson]

I meant to include a link to the failing CI run: https://github.com/DavidAnson/markdownlint-cli2/actions/runs/5959809845/job/16166105145

[DavidAnson]

(I confirm the tag is correct via https://www.npmjs.com/package/markdownlint-cli2?activeTab=code: version is 0.9.0 in package.json (commit 6b75793) and eslint-plugin-n is version 16.0.1 (commit a73a23f).)

[theoludwig]

I feel like that using npm-shrinkwrap.json causes more harm than good.
While, I completely agree and I am in the team "reproducibility is a good quality", I think specifying versions without ^/~ specifiers to use exact specific version is enough for a package/library meant to be installed on npm.

Updating markdownlint-cli2 to v0.9.0 causes some issues:

• node_modules installation size increased by ~40mb
• Random fsevents errors appearing in CI (I'm using Linux too as CI but can't reproduce the issue locally weirdly, don't know why)
• Can't use anymore "extends": "markdownlint/style/prettier" in the config, because now, markdownlint is installed below markdownlint-cli2 and not at the "root" of node_modules, so to make it work, must install markdownlint which yet again increase node_modules unnecessarily as markdownlint is now installed twice.

For the moment, I prefer staying on v0.8.1, while all these issues are not solved.
Might be worth to revert this commit: 0df0bba
The advantages to use npm-shrinkwrap.json in those cases are not worth it, there is a reason that most packages on npm doesn't use this mechanism.
For application developement, it makes sense to use package-lock.json, but for library/cli published on npm, it is not worth it, IMO.

[DavidAnson]

I'm not ready to give up yet, but this experience certainly has NOT convinced me of how great it is. :)

Regarding the drawbacks you mention:

• Increased install size is hopefully temporary and should go away with a pruned shrink wrap.
• The fact that you were successfully referencing markdownlint without declaring it as a dependency was kind of an anti-pattern. Adding that reference to your project doesn't need to increase the install size; the fact that it does is an implementation decision by npm.

[DavidAnson]

FYI, I haven't been able to reproduce the problem outside midnight-smoker, either. The problem I'm having - which may be the same as number 1 in my post above - is that npm install of a local tarball seems to NOT install dev dependencies even when the same command WOULD do so for that same tarball as a published package. I haven't found a way to force matching behavior yet.

[DavidAnson]

This should be fixed in v0.9.2; I deleted npm-shrinkwrap.json entirely. I could not find a way to use it without cross-platform problems.

v0.9.0:

@DavidAnson ➜ /workspaces/markdownlint-cli2 (main) $ npm install markdownlint-cli2@0.9.0
npm WARN deprecated date-format@0.0.2: 0.x is no longer supported. Please upgrade to 4.x or higher.

added 442 packages in 4s

9 packages are looking for funding
  run `npm fund` for details
@DavidAnson ➜ /workspaces/markdownlint-cli2 (main) $ npm clean-install
npm ERR! code EBADPLATFORM
npm ERR! notsup Unsupported platform for fsevents@2.3.3: wanted {"os":"darwin"} (current: {"os":"linux"})
npm ERR! notsup Valid os:  darwin
npm ERR! notsup Actual os: linux

npm ERR! A complete log of this run can be found in: /home/codespace/.npm/_logs/2023-08-26T04_22_27_378Z-debug-0.log
@DavidAnson ➜ /workspaces/markdownlint-cli2 (main) $

v0.9.2:

@DavidAnson ➜ /workspaces/markdownlint-cli2 (main) $ npm install markdownlint-cli2@0.9.2

added 35 packages in 2s

7 packages are looking for funding
  run `npm fund` for details
@DavidAnson ➜ /workspaces/markdownlint-cli2 (main) $ npm clean-install

added 35 packages, and audited 36 packages in 1s

7 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities
@DavidAnson ➜ /workspaces/markdownlint-cli2 (main) $

[theoludwig]

Thank you! 😄

[boneskull]

@DavidAnson midnight-smoker doesn't yet check for these sorts of problems; if you want, keep an eye on boneskull/midnight-smoker#306

thanks for fixing this.

[DavidAnson]

FYI, I captured my notes here historical purposes and possible future reference: https://gist.github.com/DavidAnson/39b0eed160f7ce481c92e24a651b5d6f

[boneskull]

FWIW, I think shrinkwrap is appropriate if:

1. The package is an app (CLI apps count), not a library
2. The package is expected to be installed in a shallow manner (e.g., something that should be installed globally, or a service being deployed)

From the docs

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. It's strongly discouraged for library authors to publish this file, since that would prevent end users from having control over transitive dependency updates.

IMO this is too broad, since we can see that a package (e.g., markdownlint-cli2) installed as a dev dependency also prohibits consumers from having control over transitive dependency updates.

It kind of just depends who your users are.

You may want to look at ncc, which should provide reproducible bundles based on a package-lock.json. While it would still wrest control from consumers (since all production transitive dependencies would be in the bundle), it should avoid installation problems entirely.

[DavidAnson]

I don't think ncc is the right link, but I think I know of the thing you refer to. But it doesn't matter: I'm not trying to be weird or make users do weird things. I'm trying to use the platform as it was intended! Users should use all the commands they normally would and the right thing should happen. If I can make builds deterministic, I will try - but not at the cost of all this nonsense. npm could behave more reasonably here, but I can't tell if the fact that it doesn't is a bug on its part or a misunderstanding on my/our part. If you read the gist I wrote and link to above, the platform-specific behavior looks to me like a deal-breaker for basically every scenario (unless you want to gamble on never having a transitive dependency that's OS-specific or you don't care about being cross-platform).