buildNpmPackage: add support for npm-shrinkwrap.json

[aduh95]

According to https://docs.npmjs.com/cli/v11/configuring-npm/npm-shrinkwrap-json, it's that file that is used in priority over package-lock.json.

Things done

• Built on platform:
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• Tested, as applicable:
  • NixOS tests in nixos/tests.
  • Package tests at passthru.tests.
  • Tests in lib/tests or pkgs/test for functions and "core" functionality.
• Ran nixpkgs-review on this PR. See nixpkgs-review usage.
• Tested basic functionality of all binary files, usually in ./result/bin/.
• Nixpkgs Release Notes
  • Package update: when the change is major or breaking.
• NixOS Release Notes
  • Module addition: when adding a new NixOS module.
  • Module update: when the change is significant.
• Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

---

Add a 👍 reaction to pull requests you find important.

[sarahec]

Move this to staging, please (the threshold is 5000+ rebuilds). See https://github.com/NixOS/nixpkgs/blob/master/CONTRIBUTING.md#rebasing-between-branches-ie-from-master-to-staging

[raboof]

I don't feel qualified to judge whether this change makes sense globally for npm-config-hook.sh, but I can confirm that it seems to work for x2t.

[sarahec]

@winterqt I'll defer to your judgement.

[aduh95]

In case someone needs convincing, you can confirm using the following script that npm does not read the package-lock.json when a npm-shrinkwrap.json is available:

rm -rf repro
(
    set -e
    mkdir repro
    cd repro
    echo '{}' > package.json
    echo 'invalid json' > package-lock.json
    ! npm ci
    echo '{}' > npm-shrinkwrap.json
    npm ci

    echo
    echo "=========="
    echo
    for file in *; do
        echo "=== Contents of $file ==="
        cat "$file"
        echo
    done
)
rm -rf repro

There is also this test case in npm repo: https://github.com/npm/cli/blob/d3896147c61b06d6d39a55bbb609f878548e0107/workspaces/arborist/test/shrinkwrap.js#L1612-L1620

[doronbehar]

This approach would feel a bit safer to reviewers, and I won't mind merge it and be responsible for it.

[thunze]

I'm not sure this made sense as a global change. From what I can see, this broke most packages it touched because fetchNpmDeps also fails if package-lock.json doesn't exist. (When testing, make sure to also rebuild <package>.npmDeps as it's still cached in most cases.)

Hydra already caught one instance of this issue: https://hydra.nixos.org/build/312774851/nixlog/5

And I'm also not sure it's great that we now have to sync the (fairly rare) npm-shrinkwrap.json edge case across all npm tooling in nixpkgs.

I see at least two possible courses of action here to fix the breakages:

1. Revert this change and maybe document how to manually deal with npm-shrinkwrap.json.
2. Keep it, handle the edge case wherever it needs to be handled (maybe other fetchers, package managers, etc. should handle this, too?) and probably also document where necessary that npm-shrinkwrap.json is handled implicity.

I don't know enough about the npm ecosystem in nixpkgs to take care of all of 2., so I'd like to hear your opinion on this first. :)

Relevant to ZHF: #457852

[aduh95]

I've opened #460084 to address this.

[doronbehar]

Damn I'm surprised we missed it, and that more then a month later we got notified this is buggy.

[doronbehar]

If this PR was completely buggy, I wonder how come balena-cli hasn't became broken. @thunze Do you have an idea?

[thunze]

nix-build -A balena-cli.npmDeps --check fails for me.

[doronbehar]

Oh I see. We see this failure only now when this PR has reached master.