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

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

Merged
merged 2 commits into from May 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 13 additions & 0 deletions docs/content/using-npm/config.md
Expand Up @@ -1177,6 +1177,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
Expand Down
12 changes: 12 additions & 0 deletions lib/utils/config/definitions.js
Expand Up @@ -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'],
Expand Down
2 changes: 2 additions & 0 deletions tap-snapshots/test/lib/commands/config.js.test.cjs
Expand Up @@ -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,
Expand Down Expand Up @@ -257,6 +258,7 @@ noproxy = [""]
npm-version = "{NPM-VERSION}"
offline = false
omit = []
omit-lockfile-registry-resolved = false
only = null
optional = null
otp = null
Expand Down
13 changes: 13 additions & 0 deletions tap-snapshots/test/lib/utils/config/definitions.js.test.cjs
Expand Up @@ -97,6 +97,7 @@ Array [
"npm-version",
"offline",
"omit",
"omit-lockfile-registry-resolved",
"only",
"optional",
"otp",
Expand Down Expand Up @@ -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\`
Expand Down
13 changes: 13 additions & 0 deletions tap-snapshots/test/lib/utils/config/describe-all.js.test.cjs
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions workspaces/arborist/lib/arborist/build-ideal-tree.js
Expand Up @@ -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 }))

Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions workspaces/arborist/lib/arborist/load-actual.js
Expand Up @@ -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
Expand All @@ -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 })
Expand Down
1 change: 1 addition & 0 deletions workspaces/arborist/lib/arborist/load-virtual.js
Expand Up @@ -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')
Expand Down
12 changes: 12 additions & 0 deletions workspaces/arborist/lib/node.js
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions 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 }
31 changes: 24 additions & 7 deletions workspaces/arborist/lib/shrinkwrap.js
Expand Up @@ -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 => {
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -330,6 +336,7 @@ class Shrinkwrap {
shrinkwrapOnly = false,
hiddenLockfile = false,
lockfileVersion,
resolveOptions = {},
} = options

this.lockfileVersion = hiddenLockfile ? 3
Expand All @@ -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
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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()) {
Expand Down Expand Up @@ -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) {
Expand Down
23 changes: 23 additions & 0 deletions workspaces/arborist/test/node.js
Expand Up @@ -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)
})
88 changes: 88 additions & 0 deletions workspaces/arborist/test/shrinkwrap.js
Expand Up @@ -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
Expand Down