Skip to content

Commit

Permalink
feat: add flag --omit-lockfile-registry-resolved (#4874)
Browse files Browse the repository at this point in the history
* feat(arborist): added flag to omit lockfile resolved

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

Co-authored-by: Caleb ツ Everett <calebev@amazon.com>
  • Loading branch information
fritzy and Caleb ツ Everett authored May 10, 2022
1 parent c024e90 commit bfb8bcc
Show file tree
Hide file tree
Showing 13 changed files with 216 additions and 7 deletions.
13 changes: 13 additions & 0 deletions docs/content/using-npm/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions lib/utils/config/definitions.js
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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

0 comments on commit bfb8bcc

Please sign in to comment.