From 775cffe743586927eebe2718325537513aa3d006 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Caleb=20=E3=83=84=20Everett?= Date: Tue, 16 Nov 2021 14:30:30 -0800 Subject: [PATCH 1/2] feat(arborist): added flag to omit lockfile resolved --- .../arborist/lib/arborist/build-ideal-tree.js | 2 + .../arborist/lib/arborist/load-actual.js | 2 + .../arborist/lib/arborist/load-virtual.js | 1 + workspaces/arborist/lib/node.js | 12 +++ workspaces/arborist/lib/override-resolves.js | 11 +++ workspaces/arborist/lib/shrinkwrap.js | 31 +++++-- workspaces/arborist/test/node.js | 23 +++++ workspaces/arborist/test/shrinkwrap.js | 88 +++++++++++++++++++ 8 files changed, 163 insertions(+), 7 deletions(-) create mode 100644 workspaces/arborist/lib/override-resolves.js diff --git a/workspaces/arborist/lib/arborist/build-ideal-tree.js b/workspaces/arborist/lib/arborist/build-ideal-tree.js index 55eb8292335d0..e6c9fe68fab97 100644 --- 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 index 70b898141cc54..43351b69034af 100644 --- 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 index 097e5fb84298e..fb3f334747fc8 100644 --- 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 index 60301798b918d..d731e5f617908 100644 --- 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 new file mode 100644 index 0000000000000..794b2c335dc62 --- /dev/null +++ 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 index ab6c91935c78e..3305bac4914be 100644 --- 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,7 +894,10 @@ 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 () { @@ -894,7 +905,10 @@ class Shrinkwrap { 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 index 73e0818938f27..36856a4cc557e 100644 --- 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 index d47266d30e1c1..07178062e3a40 100644 --- 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 From e680892f2dc9567fd3741f89893810d08e7ff4aa Mon Sep 17 00:00:00 2001 From: Nathan Fritz Date: Mon, 9 May 2022 14:29:17 -0700 Subject: [PATCH 2/2] feat: add flag --omit-lockfile-registry-resolved --- docs/content/using-npm/config.md | 13 +++++++++++++ lib/utils/config/definitions.js | 12 ++++++++++++ tap-snapshots/test/lib/commands/config.js.test.cjs | 2 ++ .../test/lib/utils/config/definitions.js.test.cjs | 13 +++++++++++++ .../test/lib/utils/config/describe-all.js.test.cjs | 13 +++++++++++++ 5 files changed, 53 insertions(+) diff --git a/docs/content/using-npm/config.md b/docs/content/using-npm/config.md index 4dc2829825d60..3c06b4161f825 100644 --- a/docs/content/using-npm/config.md +++ b/docs/content/using-npm/config.md @@ -1177,6 +1177,19 @@ variable will be set to `'production'` for all lifecycle scripts. +#### `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. + + + + #### `otp` * Default: null diff --git a/lib/utils/config/definitions.js b/lib/utils/config/definitions.js index 6f1b1a7244a50..1c426736ead38 100644 --- 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 index c6f07dd326cb2..fe392344ed35a 100644 --- 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 index 42db6ce030b53..c31a6b7dc7e67 100644 --- 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 index d0ce3a26f764f..0ec2ca17a4ef1 100644 --- 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. +#### \`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. + + + + #### \`otp\` * Default: null