Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Update @npmcli/arborist #97

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ericcornelissen
Copy link
Contributor

This is a new version of #90 following the engines update in #96. Now, the upgrade of @npmcli/arborist from v6 to v7 is no longer a breaking change for licensee. I had to recreate the PR because I had deleted my fork of this project.


Update the dependency @npmcli/arborist from v6 to v7 in order to transitively upgrade or remove deprecated dependencies. For example, the dependency @npmcli/move-file is deprecated and dropped by @npmcli/arborist v7. Similarly, the dependency glob is deprecated at v7 and v8 (both were previously in the dependency tree) and has been upgrade to v10.

The exact version I picked is the first v7 release that occurred after the release of the release of v6.5.0, which hopefully means any feature of v6.5.0 used by this project is also present in the selected minimum v7 release.

The effect can be seen from within this repository as follows, before (on main at d746a5b):

$ npm install --omit dev
npm warn deprecated inflight@1.0.6: This module is not supported, and leaks memory. Do not use it. Check out lru-cache if you want a good and tested way to coalesce async requests by a key value, which is much more comprehensive and powerful.
npm warn deprecated @npmcli/move-file@2.0.1: This functionality has been moved to @npmcli/fs
npm warn deprecated read-package-json@6.0.4: This package is no longer supported. Please use @npmcli/package-json instead.
npm warn deprecated npmlog@7.0.1: This package is no longer supported.
npm warn deprecated npmlog@6.0.2: This package is no longer supported.
npm warn deprecated rimraf@3.0.2: Rimraf versions prior to v4 are no longer supported
npm warn deprecated are-we-there-yet@4.0.2: This package is no longer supported.
npm warn deprecated are-we-there-yet@3.0.1: This package is no longer supported.
npm warn deprecated glob@8.1.0: Glob versions prior to v9 are no longer supported
npm warn deprecated glob@7.2.3: Glob versions prior to v9 are no longer supported
npm warn deprecated glob@7.2.3: Glob versions prior to v9 are no longer supported
npm warn deprecated gauge@5.0.2: This package is no longer supported.
npm warn deprecated gauge@4.0.4: This package is no longer supported.

added 268 packages, and audited 269 packages in 4s

[...]

after (this PR):

$ npm install --omit dev

added 187 packages, and audited 188 packages in 3s

[...]

Update the dependency `@npmcli/arborist` from v6 to v7 in order to
transitively upgrade or remove dependencies. For example, the dependency
`@npmcli/move-file` is deprecated and dropped by `@npmcli/arborist` v7.
Similarly, the dependency `glob` is deprecated at v7 and v8 (both were
previously in the dependency tree) and has been upgrade to v10.

The exact version I picked is the first v7 release that occurred after
the release of the release of v6.5.0, which hopefully means any feature
of v6.5.0 used by this project is also present in the selected minimum
v7 release.

The effect can be seen from within this repository as follows, before
(on `main`):

    $ npm install --omit dev
    npm warn deprecated inflight@1.0.6: This module is not supported, and leaks memory. Do not use it. Check out lru-cache if you want a good and tested way to coalesce async requests by a key value, which is much more comprehensive and powerful.
    npm warn deprecated @npmcli/move-file@2.0.1: This functionality has been moved to @npmcli/fs
    npm warn deprecated read-package-json@6.0.4: This package is no longer supported. Please use @npmcli/package-json instead.
    npm warn deprecated npmlog@7.0.1: This package is no longer supported.
    npm warn deprecated npmlog@6.0.2: This package is no longer supported.
    npm warn deprecated rimraf@3.0.2: Rimraf versions prior to v4 are no longer supported
    npm warn deprecated are-we-there-yet@4.0.2: This package is no longer supported.
    npm warn deprecated are-we-there-yet@3.0.1: This package is no longer supported.
    npm warn deprecated glob@8.1.0: Glob versions prior to v9 are no longer supported
    npm warn deprecated glob@7.2.3: Glob versions prior to v9 are no longer supported
    npm warn deprecated glob@7.2.3: Glob versions prior to v9 are no longer supported
    npm warn deprecated gauge@5.0.2: This package is no longer supported.
    npm warn deprecated gauge@4.0.4: This package is no longer supported.

    added 268 packages, and audited 269 packages in 4s

    [...]

after (this commit):

    $ npm install --omit dev

    added 187 packages, and audited 188 packages in 3s

    [...]
@kemitchell
Copy link
Member

There's no longer a separate changelog for Arborist. It's been moved into the npm CLI monorepo, in a workspace subdirectory.

wget https://registry.npmjs.org/@npmcli/arborist/-/arborist-6.5.0.tgz
unp arborist-6.5.0.tgz
mv package 6.5.0
wget https://registry.npmjs.org/@npmcli/arborist/-/arborist-7.2.1.tgz
unp arborist-7.2.1.tgz
mv package 7.2.1
diff -ru 6.5.0 7.2.1

Output:

  • one code change
  • a bunch of dep bumps
  • new engines range
diff -ur 6.5.0/lib/query-selector-all.js 7.2.1/lib/query-selector-all.js
--- 6.5.0/lib/query-selector-all.js	1985-10-26 01:15:00.000000000 -0700
+++ 7.2.1/lib/query-selector-all.js	1985-10-26 01:15:00.000000000 -0700
@@ -695,7 +695,11 @@
 // the compare nodes array
 const hasParent = (node, compareNodes) => {
   // All it takes is one so we loop and return on the first hit
-  for (const compareNode of compareNodes) {
+  for (let compareNode of compareNodes) {
+    if (compareNode.isLink) {
+      compareNode = compareNode.target
+    }
+
     // follows logical parent for link anscestors
     if (node.isTop && (node.resolveParent === compareNode)) {
       return true
diff -ur 6.5.0/package.json 7.2.1/package.json
--- 6.5.0/package.json	1985-10-26 01:15:00.000000000 -0700
+++ 7.2.1/package.json	1985-10-26 01:15:00.000000000 -0700
@@ -1,39 +1,39 @@
 {
   "name": "@npmcli/arborist",
-  "version": "6.5.0",
+  "version": "7.2.1",
   "description": "Manage node_modules trees",
   "dependencies": {
     "@isaacs/string-locale-compare": "^1.1.0",
     "@npmcli/fs": "^3.1.0",
     "@npmcli/installed-package-contents": "^2.0.2",
     "@npmcli/map-workspaces": "^3.0.2",
-    "@npmcli/metavuln-calculator": "^5.0.0",
+    "@npmcli/metavuln-calculator": "^7.0.0",
     "@npmcli/name-from-folder": "^2.0.0",
     "@npmcli/node-gyp": "^3.0.0",
-    "@npmcli/package-json": "^4.0.0",
-    "@npmcli/query": "^3.0.0",
-    "@npmcli/run-script": "^6.0.0",
+    "@npmcli/package-json": "^5.0.0",
+    "@npmcli/query": "^3.0.1",
+    "@npmcli/run-script": "^7.0.2",
     "bin-links": "^4.0.1",
-    "cacache": "^17.0.4",
+    "cacache": "^18.0.0",
     "common-ancestor-path": "^1.0.1",
-    "hosted-git-info": "^6.1.1",
+    "hosted-git-info": "^7.0.1",
     "json-parse-even-better-errors": "^3.0.0",
     "json-stringify-nice": "^1.1.4",
     "minimatch": "^9.0.0",
     "nopt": "^7.0.0",
     "npm-install-checks": "^6.2.0",
-    "npm-package-arg": "^10.1.0",
-    "npm-pick-manifest": "^8.0.1",
-    "npm-registry-fetch": "^14.0.3",
+    "npm-package-arg": "^11.0.1",
+    "npm-pick-manifest": "^9.0.0",
+    "npm-registry-fetch": "^16.0.0",
     "npmlog": "^7.0.1",
-    "pacote": "^15.0.8",
+    "pacote": "^17.0.4",
     "parse-conflict-json": "^3.0.0",
     "proc-log": "^3.0.0",
     "promise-all-reject-late": "^1.0.0",
     "promise-call-limit": "^1.0.2",
     "read-package-json-fast": "^3.0.2",
     "semver": "^7.3.7",
-    "ssri": "^10.0.1",
+    "ssri": "^10.0.5",
     "treeverse": "^3.0.0",
     "walk-up-path": "^3.0.1"
   },
@@ -42,8 +42,8 @@
     "@npmcli/template-oss": "4.19.0",
     "benchmark": "^2.1.4",
     "minify-registry-metadata": "^3.0.0",
-    "nock": "^13.3.0",
-    "tap": "^16.3.4",
+    "nock": "^13.3.3",
+    "tap": "^16.3.8",
     "tar-stream": "^3.0.0",
     "tcompare": "^5.0.6"
   },
@@ -79,7 +79,6 @@
     "test-env": [
       "LC_ALL=sk"
     ],
-    "color": 1,
     "timeout": "360",
     "nyc-arg": [
       "--exclude",
@@ -87,7 +86,7 @@
     ]
   },
   "engines": {
-    "node": "^14.17.0 || ^16.13.0 || >=18.0.0"
+    "node": "^16.14.0 || >=18.0.0"
   },
   "templateOSS": {
     "//@npmcli/template-oss": "This file is partially managed by @npmcli/template-oss. Edits may be overwritten.",

@ljharb any reason not to update engines for Licensee to >= 18.12, matching Arborist, and release as SemVer-major? Get on the Arborist train and ride it.

@ljharb
Copy link
Member

ljharb commented Sep 8, 2024

That's fine, but to minimize future majors, it's probably best to do something like ^20.9 || >= 22.8 so the next major of arborist keeps working?

@ericcornelissen
Copy link
Contributor Author

I'm sorry but I'm slightly confused. The engines values are:

  • Licensee at v11.1.0: ^18.12 || ^20.9 || >= 22.7 (src)
  • Arborist at v7.2.1: ^16.14.0 || >=18.0.0 (src)

meaning that Arborist's is a superset of Licensee (because >=18.0.0 is larger than ^18.12 || ^20.9 || >= 22.7), meaning any version supported by Licensee is fine for Arborist. Therefor, why would we want "to update engines for Licensee to >= 18.12"? If you mean the following change for Licensee:

-    "node": "^18.12 || ^20.9 || >= 22.7"
+    "node": ">=18.12"

that wouldn't be a breaking change - supporting more Node.js versions won't break any existing users.

@ericcornelissen
Copy link
Contributor Author

ericcornelissen commented Sep 24, 2024

ping @kemitchell - seems @ljharb is okay with the changes as is

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

Successfully merging this pull request may close these issues.

3 participants