Do not rely on underscore fields in package.json

This also uncovered a bug where the yarn registry was being used when the npm registry should have been used. This bug is also fixed in this commit, and was hidden by the fact that our package.json files on disk for yarn tests were placed there by npm v6, which put the proper resolved values in _resolved in the package.json file iteself.
npm · Feb 22, 2021 · d51647e · d51647e
1 parent c102725
commit d51647e
Show file tree

Hide file tree

Showing 6 changed files with 1,175 additions and 1,671 deletions.
diff --git a/lib/shrinkwrap.js b/lib/shrinkwrap.js
@@ -59,6 +59,10 @@ const swKeyOrder = [
   'dependencies',
 ]
 
+// used to rewrite from yarn registry to npm registry
+const yarnRegRe = /^https?:\/\/registry.yarnpkg.com\//
+const npmRegRe = /^https?:\/\/registry.npmjs.org\//
+
 // sometimes resolved: is weird or broken, or something npa can't handle
 const specFromResolved = resolved => {
   try {
@@ -291,8 +295,6 @@ class Shrinkwrap {
     if (fromYarn && fromYarn.version) {
       // if it's the yarn or npm default registry, use the version as
       // our effective spec.  if it's any other kind of thing, use that.
-      const yarnRegRe = /^https?:\/\/registry.yarnpkg.com\//
-      const npmRegRe = /^https?:\/\/registry.npmjs.org\//
       const {resolved, version, integrity} = fromYarn
       const isYarnReg = spec.registry && yarnRegRe.test(resolved)
       const isnpmReg = spec.registry && !isYarnReg && npmRegRe.test(resolved)
@@ -733,6 +735,7 @@ class Shrinkwrap {
       : !/file:/.test(node.resolved) ? node.resolved
       : consistentResolve(node.resolved, node.path, this.path, true)
 
+    const spec = npa(`${node.name}@${edge.spec}`)
     const entry = this.yarnLock.entries.get(`${node.name}@${edge.spec}`)
 
     if (!entry ||
@@ -741,6 +744,9 @@ class Shrinkwrap {
         mismatch(pathFixed, entry.resolved))
       return
 
+    if (entry.resolved && yarnRegRe.test(entry.resolved) && spec.registry)
+      entry.resolved = entry.resolved.replace(yarnRegRe, 'https://registry.npmjs.org/')
+
     node.integrity = node.integrity || entry.integrity || null
     node.resolved = node.resolved ||
       consistentResolve(entry.resolved, this.path, node.path) || null

diff --git a/tap-snapshots/test-arborist-build-ideal-tree.js-TAP.test.js b/tap-snapshots/test-arborist-build-ideal-tree.js-TAP.test.js
@@ -136,7 +136,6 @@ ArboristNode {
       "location": "node_modules/once",
       "name": "once",
       "path": "{CWD}/test/fixtures/outdated-no-lockfile/node_modules/once",
-      "resolved": "https://registry.npmjs.org/once/-/once-1.3.3.tgz",
       "version": "1.3.3",
     },
     "wrappy" => ArboristNode {
@@ -151,7 +150,6 @@ ArboristNode {
       "location": "node_modules/wrappy",
       "name": "wrappy",
       "path": "{CWD}/test/fixtures/outdated-no-lockfile/node_modules/wrappy",
-      "resolved": "https://registry.npmjs.org/wrappy/-/wrappy-1.0.1.tgz",
       "version": "1.0.1",
     },
   },
@@ -6066,7 +6064,6 @@ ArboristNode {
           "location": "node_modules/@isaacs/testing-bundledeps/node_modules/@isaacs/testing-bundledeps-a",
           "name": "@isaacs/testing-bundledeps-a",
           "path": "{CWD}/test/fixtures/testing-bundledeps-empty/node_modules/@isaacs/testing-bundledeps/node_modules/@isaacs/testing-bundledeps-a",
-          "resolved": "https://registry.npmjs.org/@isaacs/testing-bundledeps-a/-/testing-bundledeps-a-1.0.0.tgz",
           "version": "1.0.0",
         },
         "@isaacs/testing-bundledeps-b" => ArboristNode {
@@ -6082,7 +6079,6 @@ ArboristNode {
           "location": "node_modules/@isaacs/testing-bundledeps/node_modules/@isaacs/testing-bundledeps-b",
           "name": "@isaacs/testing-bundledeps-b",
           "path": "{CWD}/test/fixtures/testing-bundledeps-empty/node_modules/@isaacs/testing-bundledeps/node_modules/@isaacs/testing-bundledeps-b",
-          "resolved": "https://registry.npmjs.org/@isaacs/testing-bundledeps-b/-/testing-bundledeps-b-1.0.0.tgz",
           "version": "1.0.0",
         },
       },
@@ -6196,7 +6192,6 @@ ArboristNode {
           "location": "node_modules/@isaacs/testing-bundledeps/node_modules/@isaacs/testing-bundledeps-a",
           "name": "@isaacs/testing-bundledeps-a",
           "path": "{CWD}/test/fixtures/testing-bundledeps-empty/node_modules/@isaacs/testing-bundledeps/node_modules/@isaacs/testing-bundledeps-a",
-          "resolved": "https://registry.npmjs.org/@isaacs/testing-bundledeps-a/-/testing-bundledeps-a-1.0.0.tgz",
           "version": "1.0.0",
         },
         "@isaacs/testing-bundledeps-b" => ArboristNode {
@@ -6212,7 +6207,6 @@ ArboristNode {
           "location": "node_modules/@isaacs/testing-bundledeps/node_modules/@isaacs/testing-bundledeps-b",
           "name": "@isaacs/testing-bundledeps-b",
           "path": "{CWD}/test/fixtures/testing-bundledeps-empty/node_modules/@isaacs/testing-bundledeps/node_modules/@isaacs/testing-bundledeps-b",
-          "resolved": "https://registry.npmjs.org/@isaacs/testing-bundledeps-b/-/testing-bundledeps-b-1.0.0.tgz",
           "version": "1.0.0",
         },
       },
@@ -6507,7 +6501,6 @@ ArboristNode {
           "location": "node_modules/@isaacs/testing-bundledeps/node_modules/@isaacs/testing-bundledeps-a",
           "name": "@isaacs/testing-bundledeps-a",
           "path": "{CWD}/test/fixtures/testing-bundledeps/node_modules/@isaacs/testing-bundledeps/node_modules/@isaacs/testing-bundledeps-a",
-          "resolved": "https://registry.npmjs.org/@isaacs/testing-bundledeps-a/-/testing-bundledeps-a-1.0.0.tgz",
           "version": "1.0.0",
         },
         "@isaacs/testing-bundledeps-b" => ArboristNode {
@@ -6523,7 +6516,6 @@ ArboristNode {
           "location": "node_modules/@isaacs/testing-bundledeps/node_modules/@isaacs/testing-bundledeps-b",
           "name": "@isaacs/testing-bundledeps-b",
           "path": "{CWD}/test/fixtures/testing-bundledeps/node_modules/@isaacs/testing-bundledeps/node_modules/@isaacs/testing-bundledeps-b",
-          "resolved": "https://registry.npmjs.org/@isaacs/testing-bundledeps-b/-/testing-bundledeps-b-1.0.0.tgz",
           "version": "1.0.0",
         },
       },
@@ -6552,7 +6544,6 @@ ArboristNode {
       "location": "node_modules/@isaacs/testing-bundledeps",
       "name": "@isaacs/testing-bundledeps",
       "path": "{CWD}/test/fixtures/testing-bundledeps/node_modules/@isaacs/testing-bundledeps",
-      "resolved": "https://registry.npmjs.org/@isaacs/testing-bundledeps/-/testing-bundledeps-1.0.0.tgz",
       "version": "1.0.0",
     },
     "@isaacs/testing-bundledeps-b" => ArboristNode {
@@ -6568,7 +6559,6 @@ ArboristNode {
       "location": "node_modules/@isaacs/testing-bundledeps-b",
       "name": "@isaacs/testing-bundledeps-b",
       "path": "{CWD}/test/fixtures/testing-bundledeps/node_modules/@isaacs/testing-bundledeps-b",
-      "resolved": "https://registry.npmjs.org/@isaacs/testing-bundledeps-b/-/testing-bundledeps-b-1.0.0.tgz",
       "version": "1.0.0",
     },
     "@isaacs/testing-bundledeps-c" => ArboristNode {
@@ -6592,7 +6582,6 @@ ArboristNode {
       "location": "node_modules/@isaacs/testing-bundledeps-c",
       "name": "@isaacs/testing-bundledeps-c",
       "path": "{CWD}/test/fixtures/testing-bundledeps/node_modules/@isaacs/testing-bundledeps-c",
-      "resolved": "https://registry.npmjs.org/@isaacs/testing-bundledeps-c/-/testing-bundledeps-c-1.0.0.tgz",
       "version": "1.0.0",
     },
   },
@@ -6637,7 +6626,6 @@ ArboristNode {
           "location": "node_modules/@isaacs/testing-bundledeps/node_modules/@isaacs/testing-bundledeps-a",
           "name": "@isaacs/testing-bundledeps-a",
           "path": "{CWD}/test/fixtures/testing-bundledeps/node_modules/@isaacs/testing-bundledeps/node_modules/@isaacs/testing-bundledeps-a",
-          "resolved": "https://registry.npmjs.org/@isaacs/testing-bundledeps-a/-/testing-bundledeps-a-1.0.0.tgz",
           "version": "1.0.0",
         },
         "@isaacs/testing-bundledeps-b" => ArboristNode {
@@ -6653,7 +6641,6 @@ ArboristNode {
           "location": "node_modules/@isaacs/testing-bundledeps/node_modules/@isaacs/testing-bundledeps-b",
           "name": "@isaacs/testing-bundledeps-b",
           "path": "{CWD}/test/fixtures/testing-bundledeps/node_modules/@isaacs/testing-bundledeps/node_modules/@isaacs/testing-bundledeps-b",
-          "resolved": "https://registry.npmjs.org/@isaacs/testing-bundledeps-b/-/testing-bundledeps-b-1.0.0.tgz",
           "version": "1.0.0",
         },
       },
@@ -6682,7 +6669,6 @@ ArboristNode {
       "location": "node_modules/@isaacs/testing-bundledeps",
       "name": "@isaacs/testing-bundledeps",
       "path": "{CWD}/test/fixtures/testing-bundledeps/node_modules/@isaacs/testing-bundledeps",
-      "resolved": "https://registry.npmjs.org/@isaacs/testing-bundledeps/-/testing-bundledeps-1.0.0.tgz",
       "version": "1.0.0",
     },
     "@isaacs/testing-bundledeps-b" => ArboristNode {
@@ -6697,7 +6683,6 @@ ArboristNode {
       "location": "node_modules/@isaacs/testing-bundledeps-b",
       "name": "@isaacs/testing-bundledeps-b",
       "path": "{CWD}/test/fixtures/testing-bundledeps/node_modules/@isaacs/testing-bundledeps-b",
-      "resolved": "https://registry.npmjs.org/@isaacs/testing-bundledeps-b/-/testing-bundledeps-b-1.0.0.tgz",
       "version": "1.0.0",
     },
     "@isaacs/testing-bundledeps-c" => ArboristNode {
@@ -6720,7 +6705,6 @@ ArboristNode {
       "location": "node_modules/@isaacs/testing-bundledeps-c",
       "name": "@isaacs/testing-bundledeps-c",
       "path": "{CWD}/test/fixtures/testing-bundledeps/node_modules/@isaacs/testing-bundledeps-c",
-      "resolved": "https://registry.npmjs.org/@isaacs/testing-bundledeps-c/-/testing-bundledeps-c-1.0.0.tgz",
       "version": "1.0.0",
     },
   },

diff --git a/tap-snapshots/test-arborist-load-actual.js-TAP.test.js b/tap-snapshots/test-arborist-load-actual.js-TAP.test.js
@@ -293,7 +293,6 @@ ArboristNode {
           "name": "express",
           "packageName": "abbrev",
           "path": "root/node_modules/foo/node_modules/express",
-          "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz",
           "version": "1.1.1",
         },
       },
@@ -880,7 +879,6 @@ ArboristLink {
             "name": "express",
             "packageName": "abbrev",
             "path": "root/node_modules/foo/node_modules/express",
-            "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz",
             "version": "1.1.1",
           },
         },
@@ -2662,7 +2660,6 @@ ArboristLink {
             "name": "express",
             "packageName": "abbrev",
             "path": "root/node_modules/foo/node_modules/express",
-            "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz",
             "version": "1.1.1",
           },
         },
@@ -4222,7 +4219,6 @@ ArboristNode {
           "name": "express",
           "packageName": "abbrev",
           "path": "root/node_modules/foo/node_modules/express",
-          "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz",
           "version": "1.1.1",
         },
       },
@@ -5859,7 +5855,6 @@ ArboristNode {
           "name": "express",
           "packageName": "abbrev",
           "path": "root/node_modules/foo/node_modules/express",
-          "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz",
           "version": "1.1.1",
         },
       },