[Deps] update `@npmcli/arborist`

[ericcornelissen]

Update the dependency @npmcli/arborist from v6 to v7 in order to transitively upgrade dependencies such that @npmcli/move-file is removed from the (runtime) dependency tree (this follows the recent release of node-gyp v10.0.0). The motivation for this is that @npmcli/move-file has been deprecated because "This functionality has been moved to @npmcli/fs" for a while.

The breaking change from v6 to v7 is only that support for Node.js <=16.13 has been dropped. The implication is that that this change needs require a major version bump for licensee as well, since we currently support >=14.17. This new minimum Node.js version has been included here as well, based on the engines.node value for @npmcli/arborist. - See also #85.

The effect can be seen from within this repository as follows, before:

$ npm i --omit dev
npm WARN deprecated @npmcli/move-file@2.0.1: This functionality has been moved to @npmcli/fs

added 263 packages, and audited 264 packages in 2s

[...]

$ npm ls --omit dev @npmcli/move-file
licensee@10.0.0 /path/to/licensee
└─┬ @npmcli/arborist@6.5.0
  └─┬ @npmcli/run-script@6.0.2
    └─┬ node-gyp@9.4.1
      └─┬ make-fetch-happen@10.2.1
        └─┬ cacache@16.1.3
          └── @npmcli/move-file@2.0.1

after:

$ npm i --omit dev

added 213 packages, and audited 214 packages in 2s

[...]

$ npm ls --omit dev @npmcli/move-file
licensee@10.0.0 /path/to/licensee
└── (empty)

$ npm ls --omit dev cacache          
licensee@10.0.0 /path/to/licensee
└─┬ @npmcli/arborist@7.2.0
  ├─┬ @npmcli/metavuln-calculator@7.0.0
  │ └── cacache@18.0.0 deduped
  ├── cacache@18.0.0
  ├─┬ npm-registry-fetch@16.1.0
  │ └─┬ make-fetch-happen@13.0.0
  │   └── cacache@18.0.0 deduped
  └─┬ pacote@17.0.4
    └── cacache@18.0.0 deduped

[kemitchell]

Thanks for sending this surprise contribution!

Looking into our engines/versions situation here, I have wound up confused again We declare >= 14.17 in package.json, but looking at the most recent run of @ljharb's node-majors CI config, we only actually tested on 16, 18, 19, 20, and 21. Node.js 14 was an LTS release. I guess the CI config skips LTS releases after scheduled end-of-life.

I'm inclined to just remove engines from package.json, take this, and publish the new major version.

[ericcornelissen]

Looking into our engines/versions situation here, I have wound up confused again We declare >= 14.17 in package.json, but looking at the most recent run of [@ljharb's] node-majors CI config, we only actually tested on 16, 18, 19, 20, and 21. Node.js 14 was an LTS release. I guess the CI config skips LTS releases after scheduled end-of-life.

If I'm not mistaken, the reason it only tested on 16, 18, 19, 20, and 21 is because I updated the CI configuration as well. The CI for 4f3985c4b82aa7e7b2c9946e990b97626db8c196 did run tests against Node 14.

I'm inclined to just remove engines from package.json

I'll leave that up to you/the maintainers of this project, I think you make a good case for it in #91 though :+1: That being said, you'd still need to decide which version of Node.js to test against.

[ljharb]

The CI config doesn't look at engines at all, and removing engines is a nonstarter.

[kemitchell]

On testing, I suspect we may have dropped Node.js 14 support already, by not testing on it. I'm on vacation now, but I'll be happily surprised if I get around to testing on Node.js 14, everything installs, and the tests pass.

I'm not up-to-date on goings-on with move-file or fs. If there was a mistaken deprecation, licensee doesn't do anyone good falling for it.

At the same time, I'm generally in favor of following npm's modules closely, and would take deprecation warnings and the like from them seriously. But I'd expect them to be pretty good about "announcing" transition from deprecation to breaking removal via SemVer.

[ljharb]

@kemitchell we are testing on it. It's only this PR that drops tests for it. Here's an example from your "remove engines" PR: https://github.com/jslicense/licensee.js/actions/runs/6684697210/job/18162248337

node 14 still works just fine, and is tested.

[kemitchell]

@ljharb thanks so much. I was lost in the hall of mirrors here.

While I have you, do you happen to have a link for the @npmcli deprecation being a mistake? You've done enough already, but that would close this issue out for me. Still with thanks to @ericcornelissen, of course.

[ljharb]

I mean, they clearly don't think it's a mistake - but imo deprecating @npmcli/move-file without migrating its important dependents first (like arborist 6) is imo more disruptive than the alternative. Unfortunately, the npm team is wildly understaffed and doesn't have time to do many of the backports that ideally would be done.

[kemitchell]

I hear you. But unless npm itself drops Arborist, or Arborist veers off in a completely unexpected direction, I suspect we'll be riding along with the npm team and saying "thank you" for whatever work they can put out, as bumpy as the going may be. The view of node_modules we're interested in is largely the view as npm sees it.

If things become untenable, we can always go back to recursing the directory ourselves. node:fs has been pretty stable since LTS. And it's something for an increasingly old UNIX dog to do. :-P

For now I think we just wait and see what happens.

I can live with warnings. We've all seen a few.

[ljharb]

Certainly whenever there's a compelling reason to upgrade and do a major bump, we should - I just don't personally think "deprecation warnings" are compelling.

[ericcornelissen]

What's the point of this update? What benefit do we get? I only see a downside if we're just dropping node 14.

Just to clarify, the only motivation is indeed to resolve the deprecation warning.

Note that deprecation warnings matter precisely zero;

While I personally disagree with that exact statement (won't go into detail without a prompt), I can definitely live with the decision that resolving it is less important than compatibility with older Node.js versions.