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

[BUG] npm query *:missing only return the first missing dependency #7316

Closed
2 tasks done
dmnsgn opened this issue Mar 26, 2024 · 1 comment · Fixed by #7320
Closed
2 tasks done

[BUG] npm query *:missing only return the first missing dependency #7316

dmnsgn opened this issue Mar 26, 2024 · 1 comment · Fixed by #7320
Assignees
Labels
Bug thing that needs fixing Priority 1 high priority issue Release 10.x

Comments

@dmnsgn
Copy link

dmnsgn commented Mar 26, 2024

Is there an existing issue for this?

  • I have searched the existing issues

This issue exists in the latest npm version

  • I am using the latest npm

Current Behavior

npm query *:missing only return one item when no dependency are installed (for package.json with 2 dependencies and 2 devDependencies).

No luck with any combination of selector (.dev,.prod) or pseudo-selector (:root, :scope).

Expected Behavior

I expect all "dependencies not found on disk" to be returned.

Steps To Reproduce

mkd my-package
npm init -y
npm i canvas-context canvas-tint-image
npm i -D glob
rm -r node_modules
npm query '*:missing'

// returns
[
  {
    "name": "canvas-context",
    "version": "^3.2.0",
    "_id": "canvas-context@^3.2.0",
    "pkgid": "canvas-context@^3.2.0",
    "path": null,
    "realpath": null,
    "resolved": null,
    "from": [],
    "to": [],
    "dev": true,
    "inBundle": false,
    "deduped": false,
    "overridden": false,
    "queryContext": {}
  }
]

Only the first missing package removed from node_modules is returned.

Environment

  • npm: 10.5.0
  • Node.js: v21.7.1
  • OS Name: MacOS
@dmnsgn dmnsgn added Bug thing that needs fixing Needs Triage needs review for next steps Release 10.x labels Mar 26, 2024
@milaninfy milaninfy added Priority 1 high priority issue and removed Needs Triage needs review for next steps labels Mar 26, 2024
@wraithgar wraithgar self-assigned this Mar 27, 2024
@wraithgar
Copy link
Member

Looks like missing nodes weren't fully fleshed out in arborist, and the query command wasn't nuanced enough to include them (they don't have a location so they all group as one).

diff --git a/lib/commands/query.js b/lib/commands/query.js
index 17a55a446..dfa1356eb 100644
--- a/lib/commands/query.js
+++ b/lib/commands/query.js
@@ -113,10 +113,12 @@ class Query extends BaseCommand {
   // builds a normalized inventory
   buildResponse (items) {
     for (const node of items) {
-      if (!this.#seen.has(node.target.location)) {
+      if (!node.target.location || !this.#seen.has(node.target.location)) {
         const item = new QuerySelectorItem(node)
         this.#response.push(item)
-        this.#seen.add(item.location)
+        if (node.target.location) {
+          this.#seen.add(item.location)
+        }
       }
     }
   }
diff --git a/workspaces/arborist/lib/query-selector-all.js b/workspaces/arborist/lib/query-selector-all.js
index ce49201ce..c8ec866f0 100644
--- a/workspaces/arborist/lib/query-selector-all.js
+++ b/workspaces/arborist/lib/query-selector-all.js
@@ -257,7 +257,12 @@ class Results {
       for (const edge of node.edgesOut.values()) {
         if (edge.missing) {
           const pkg = { name: edge.name, version: edge.spec }
-          res.push(new this.#targetNode.constructor({ pkg }))
+          const item = new this.#targetNode.constructor({ pkg })
+          item.queryContext = {
+            missing: true,
+          }
+          item.edgesIn = new Set([edge])
+          res.push(item)

This fixes it for me locally, will need tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Priority 1 high priority issue Release 10.x
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants