feat: add flag --omit-lockfile-registry-resolved (#4874)

fritzy · everett1992 · web-flow · commit bfb8bccbe837 · 2022-05-10T12:21:03.000-07:00
* feat(arborist): added flag to omit lockfile resolved

* feat: add flag --omit-lockfile-registry-resolved

Co-authored-by: Caleb ツ Everett &lt;calebev@amazon.com&gt;
diff --git a/docs/content/using-npm/config.md b/docs/content/using-npm/config.md
@@ -1178,6 +1178,19 @@ variable will be set to `'production'` for all lifecycle scripts.
 <!-- automatically generated, do not edit manually -->
 <!-- see lib/utils/config/definitions.js -->
 
+#### `omit-lockfile-registry-resolved`
+
+* Default: false
+* Type: Boolean
+
+This option causes npm to create lock files without a `resolved` key for
+registry dependencies. Subsequent installs will need to resolve tarball
+endpoints with the configured registry, likely resulting in a longer install
+time.
+
+<!-- automatically generated, do not edit manually -->
+<!-- see lib/utils/config/definitions.js -->
+
 #### `otp`
 
 * Default: null
diff --git a/lib/utils/config/definitions.js b/lib/utils/config/definitions.js
@@ -1385,6 +1385,18 @@ define('omit', {
   },
 })
 
+define('omit-lockfile-registry-resolved', {
+  default: false,
+  type: Boolean,
+  description: `
+    This option causes npm to create lock files without a \`resolved\` key for
+    registry dependencies. Subsequent installs will need to resolve tarball
+    endpoints with the configured registry, likely resulting in a longer install
+    time.
+  `,
+  flatten,
+})
+
 define('only', {
   default: null,
   type: [null, 'prod', 'production'],
diff --git a/tap-snapshots/test/lib/commands/config.js.test.cjs b/tap-snapshots/test/lib/commands/config.js.test.cjs
@@ -103,6 +103,7 @@ exports[`test/lib/commands/config.js TAP config list --json > output matches sna
   "npm-version": "{NPM-VERSION}",
   "offline": false,
   "omit": [],
+  "omit-lockfile-registry-resolved": false,
   "only": null,
   "optional": null,
   "otp": null,
@@ -257,6 +258,7 @@ noproxy = [""]
 npm-version = "{NPM-VERSION}" 
 offline = false 
 omit = [] 
+omit-lockfile-registry-resolved = false 
 only = null 
 optional = null 
 otp = null 
diff --git a/tap-snapshots/test/lib/utils/config/definitions.js.test.cjs b/tap-snapshots/test/lib/utils/config/definitions.js.test.cjs
@@ -97,6 +97,7 @@ Array [
   "npm-version",
   "offline",
   "omit",
+  "omit-lockfile-registry-resolved",
   "only",
   "optional",
   "otp",
@@ -1239,6 +1240,18 @@ If the resulting omit list includes \`'dev'\`, then the \`NODE_ENV\` environment
 variable will be set to \`'production'\` for all lifecycle scripts.
 `
 
+exports[`test/lib/utils/config/definitions.js TAP > config description for omit-lockfile-registry-resolved 1`] = `
+#### \`omit-lockfile-registry-resolved\`
+
+* Default: false
+* Type: Boolean
+
+This option causes npm to create lock files without a \`resolved\` key for
+registry dependencies. Subsequent installs will need to resolve tarball
+endpoints with the configured registry, likely resulting in a longer install
+time.
+`
+
 exports[`test/lib/utils/config/definitions.js TAP > config description for only 1`] = `
 #### \`only\`
 
diff --git a/tap-snapshots/test/lib/utils/config/describe-all.js.test.cjs b/tap-snapshots/test/lib/utils/config/describe-all.js.test.cjs
@@ -1051,6 +1051,19 @@ variable will be set to \`'production'\` for all lifecycle scripts.
 <!-- automatically generated, do not edit manually -->
 <!-- see lib/utils/config/definitions.js -->
 
+#### \`omit-lockfile-registry-resolved\`
+
+* Default: false
+* Type: Boolean
+
+This option causes npm to create lock files without a \`resolved\` key for
+registry dependencies. Subsequent installs will need to resolve tarball
+endpoints with the configured registry, likely resulting in a longer install
+time.
+
+<!-- automatically generated, do not edit manually -->
+<!-- see lib/utils/config/definitions.js -->
+
 #### \`otp\`
 
 * Default: null
diff --git a/workspaces/arborist/lib/arborist/build-ideal-tree.js b/workspaces/arborist/lib/arborist/build-ideal-tree.js
@@ -329,6 +329,7 @@ Try using the package name instead, e.g:
         ? Shrinkwrap.reset({
           path: this.path,
           lockfileVersion: this.options.lockfileVersion,
+          resolveOptions: this.options,
         }).then(meta => Object.assign(root, { meta }))
         : this.loadVirtual({ root }))
 
@@ -388,6 +389,7 @@ Try using the package name instead, e.g:
     const meta = new Shrinkwrap({
       path: this.path,
       lockfileVersion: this.options.lockfileVersion,
+      resolveOptions: this.options,
     })
     meta.reset()
     root.meta = meta
diff --git a/workspaces/arborist/lib/arborist/load-actual.js b/workspaces/arborist/lib/arborist/load-actual.js
@@ -147,6 +147,7 @@ module.exports = cls => class ActualLoader extends cls {
     const meta = await Shrinkwrap.load({
       path: this[_actualTree].path,
       hiddenLockfile: true,
+      resolveOptions: this.options,
     })
     if (meta.loadedFromDisk) {
       this[_actualTree].meta = meta
@@ -155,6 +156,7 @@ module.exports = cls => class ActualLoader extends cls {
       const meta = await Shrinkwrap.load({
         path: this[_actualTree].path,
         lockfileVersion: this.options.lockfileVersion,
+        resolveOptions: this.options,
       })
       this[_actualTree].meta = meta
       return this[_loadActualActually]({ root, ignoreMissing })
diff --git a/workspaces/arborist/lib/arborist/load-virtual.js b/workspaces/arborist/lib/arborist/load-virtual.js
@@ -57,6 +57,7 @@ module.exports = cls => class VirtualLoader extends cls {
     const s = await Shrinkwrap.load({
       path: this.path,
       lockfileVersion: this.options.lockfileVersion,
+      resolveOptions: this.options,
     })
     if (!s.loadedFromDisk && !options.root) {
       const er = new Error('loadVirtual requires existing shrinkwrap file')
diff --git a/workspaces/arborist/lib/node.js b/workspaces/arborist/lib/node.js
@@ -524,6 +524,18 @@ class Node {
     return this === this.root || this === this.root.target
   }
 
+  get isRegistryDependency () {
+    if (this.edgesIn.size === 0) {
+      return false
+    }
+    for (const edge of this.edgesIn) {
+      if (!npa(edge.spec).registry) {
+        return false
+      }
+    }
+    return true
+  }
+
   * ancestry () {
     for (let anc = this; anc; anc = anc.resolveParent) {
       yield anc
diff --git a/workspaces/arborist/lib/override-resolves.js b/workspaces/arborist/lib/override-resolves.js
@@ -0,0 +1,11 @@
+function overrideResolves (resolved, opts = {}) {
+  const { omitLockfileRegistryResolved = false } = opts
+
+  if (omitLockfileRegistryResolved) {
+    return undefined
+  }
+
+  return resolved
+}
+
+module.exports = { overrideResolves }
diff --git a/workspaces/arborist/lib/shrinkwrap.js b/workspaces/arborist/lib/shrinkwrap.js
@@ -95,6 +95,7 @@ const specFromResolved = resolved => {
 const relpath = require('./relpath.js')
 
 const consistentResolve = require('./consistent-resolve.js')
+const { overrideResolves } = require('./override-resolves.js')
 
 const maybeReadFile = file => {
   return readFile(file, 'utf8').then(d => d, er => {
@@ -265,7 +266,7 @@ class Shrinkwrap {
     return s
   }
 
-  static metaFromNode (node, path) {
+  static metaFromNode (node, path, options = {}) {
     if (node.isLink) {
       return {
         resolved: relpath(path, node.realpath),
@@ -299,7 +300,12 @@ class Shrinkwrap {
     })
 
     const resolved = consistentResolve(node.resolved, node.path, path, true)
-    if (resolved) {
+    // hide resolved from registry dependencies.
+    if (!resolved) {
+      // no-op
+    } else if (node.isRegistryDependency) {
+      meta.resolved = overrideResolves(resolved, options)
+    } else {
       meta.resolved = resolved
     }
 
@@ -330,6 +336,7 @@ class Shrinkwrap {
       shrinkwrapOnly = false,
       hiddenLockfile = false,
       lockfileVersion,
+      resolveOptions = {},
     } = options
 
     this.lockfileVersion = hiddenLockfile ? 3
@@ -347,6 +354,7 @@ class Shrinkwrap {
     this.yarnLock = null
     this.hiddenLockfile = hiddenLockfile
     this.loadingError = null
+    this.resolveOptions = resolveOptions
     // only load npm-shrinkwrap.json in dep trees, not package-lock
     this.shrinkwrapOnly = shrinkwrapOnly
   }
@@ -830,7 +838,7 @@ class Shrinkwrap {
           resolved,
           integrity,
           hasShrinkwrap,
-        } = Shrinkwrap.metaFromNode(node, this.path)
+        } = Shrinkwrap.metaFromNode(node, this.path, this.resolveOptions)
         node.resolved = node.resolved || resolved || null
         node.integrity = node.integrity || integrity || null
         node.hasShrinkwrap = node.hasShrinkwrap || hasShrinkwrap || false
@@ -886,15 +894,21 @@ class Shrinkwrap {
   [_updateWaitingNode] (loc) {
     const node = this[_awaitingUpdate].get(loc)
     this[_awaitingUpdate].delete(loc)
-    this.data.packages[loc] = Shrinkwrap.metaFromNode(node, this.path)
+    this.data.packages[loc] = Shrinkwrap.metaFromNode(
+      node,
+      this.path,
+      this.resolveOptions)
   }
 
   commit () {
     if (this.tree) {
       if (this.yarnLock) {
         this.yarnLock.fromTree(this.tree)
       }
-      const root = Shrinkwrap.metaFromNode(this.tree.target, this.path)
+      const root = Shrinkwrap.metaFromNode(
+        this.tree.target,
+        this.path,
+        this.resolveOptions)
       this.data.packages = {}
       if (Object.keys(root).length) {
         this.data.packages[''] = root
@@ -905,7 +919,10 @@ class Shrinkwrap {
           continue
         }
         const loc = relpath(this.path, node.path)
-        this.data.packages[loc] = Shrinkwrap.metaFromNode(node, this.path)
+        this.data.packages[loc] = Shrinkwrap.metaFromNode(
+          node,
+          this.path,
+          this.resolveOptions)
       }
     } else if (this[_awaitingUpdate].size > 0) {
       for (const loc of this[_awaitingUpdate].keys()) {
@@ -1013,7 +1030,7 @@ class Shrinkwrap {
         spec.type !== 'git' &&
         spec.type !== 'file' &&
         spec.type !== 'remote') {
-      lock.resolved = node.resolved
+      lock.resolved = overrideResolves(node.resolved, this.resolveOptions)
     }
 
     if (node.integrity) {
diff --git a/workspaces/arborist/test/node.js b/workspaces/arborist/test/node.js
@@ -2908,3 +2908,26 @@ t.test('overrides', (t) => {
 
   t.end()
 })
+
+t.test('node with no edges in is not a registry dep', async t => {
+  const node = new Node({ path: '/foo' })
+  t.equal(node.isRegistryDependency, false)
+})
+
+t.test('node with non registry edge in is not a registry dep', async t => {
+  const root = new Node({ path: '/some/path', pkg: { dependencies: { registry: '', tar: '' } } })
+  const node = new Node({ pkg: { name: 'node', version: '1.0.0' }, parent: root })
+
+  new Node({ pkg: { name: 'registry', dependencies: { node: '^1.0.0' } }, parent: root })
+  new Node({ pkg: { name: 'tar', dependencies: { node: 'file:node' } }, parent: root })
+
+  t.equal(node.isRegistryDependency, false)
+})
+
+t.test('node with only registry edges in a registry dep', async t => {
+  const root = new Node({ path: '/some/path', pkg: { dependencies: { registry: '', tar: '' } } })
+  const node = new Node({ pkg: { name: 'node', version: '1.0.0' }, parent: root })
+  new Node({ pkg: { name: 'registry', dependencies: { node: '^1.0.0' } }, parent: root })
+
+  t.equal(node.isRegistryDependency, true)
+})
diff --git a/workspaces/arborist/test/shrinkwrap.js b/workspaces/arborist/test/shrinkwrap.js
@@ -231,6 +231,94 @@ t.test('throws when attempting to access data before loading', t => {
   t.end()
 })
 
+t.only('resolveOptions', async t => {
+  const url = 'https://private.registry.org/deadbeef/registry/-/registry-1.2.3.tgz'
+  const someOtherRegistry = 'https://someother.registry.org/registry/-/registry-1.2.3.tgz'
+  const getData = async (resolveOptions) => {
+    const dir = t.testdir()
+    const meta = await Shrinkwrap.load({
+      path: dir,
+      resolveOptions,
+    })
+
+    const root = new Node({
+      pkg: {
+        name: 'root',
+        dependencies: {
+          registry: '^1.0.0',
+          'some-other-registry': '^1.0.0',
+          '@scoped/some-other-registry': '^1.0.0',
+          tar: url,
+        },
+      },
+      path: dir,
+      realpath: dir,
+      meta,
+    })
+
+    const registry = new Node({
+      pkg: { name: 'registry', version: '1.2.3' },
+      resolved: url,
+      integrity: 'sha512-registry',
+      parent: root,
+    })
+
+    const otherRegistry = new Node({
+      pkg: { name: 'some-other-registry', version: '1.2.3' },
+      resolved: someOtherRegistry,
+      integrity: 'sha512-registry',
+      parent: root,
+    })
+
+    const scopedOtherRegistry = new Node({
+      pkg: { name: '@scope/some-other-registry', version: '1.2.3' },
+      resolved: someOtherRegistry,
+      integrity: 'sha512-registry',
+      parent: root,
+    })
+
+    const tar = new Node({
+      pkg: { name: 'tar', version: '1.2.3' },
+      resolved: url,
+      integrity: 'sha512-registry',
+      parent: root,
+    })
+
+    calcDepFlags(root)
+    meta.add(root)
+    return { data: meta.commit(), registry, tar, root, otherRegistry, scopedOtherRegistry }
+  }
+
+  await t.test('omitLockfileRegistryResolved', async t => {
+    const { data } = await getData({ omitLockfileRegistryResolved: true })
+    // registry dependencies in v2 packages and v1 dependencies should
+    // have resolved stripped.
+    t.strictSame(data.packages['node_modules/registry'].resolved, undefined)
+    t.strictSame(data.dependencies.registry.resolved, undefined)
+
+    // tar should have resolved because it is not a registry dep.
+    t.strictSame(data.packages['node_modules/tar'].resolved, url)
+    // v1 url dependencies never have resolved.
+    t.strictSame(data.dependencies.tar.resolved, undefined)
+  })
+
+  await t.test('omitLockfileRegistryResolved: false', async t => {
+    const { data } = await getData({ omitLockfileRegistryResolved: false })
+    t.strictSame(data.packages['node_modules/registry'].resolved, url)
+    t.strictSame(data.dependencies.registry.resolved, url)
+
+    t.strictSame(data.packages['node_modules/tar'].resolved, url)
+    // v1 url dependencies never have resolved.
+    t.strictSame(data.dependencies.tar.resolved, undefined)
+  })
+
+  t.test('metaFromNode default', async t => {
+    // test to cover options default.
+    const { registry } = await getData(undefined)
+    t.strictSame(Shrinkwrap.metaFromNode(registry, '').resolved, url)
+  })
+})
+
 t.test('construct metadata from node and package data', t => {
   const meta = new Shrinkwrap({ path: '/home/user/projects/root' })
   // fake load
