Skip to content

Commit

Permalink
feat: omit resolved from registry dependencies
Browse files Browse the repository at this point in the history
Start npm/rfcs#486. This implements `$disable-write-resolves` without
creating an option. Next I'll figure out how to plumb a npm config
option thru to shrinkwrap.
  • Loading branch information
Caleb ツ Everett committed Dec 16, 2021
1 parent a703bbd commit 37ad149
Show file tree
Hide file tree
Showing 4 changed files with 126 additions and 6 deletions.
12 changes: 12 additions & 0 deletions lib/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,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
27 changes: 21 additions & 6 deletions lib/shrinkwrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,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 +299,9 @@ class Shrinkwrap {
})

const resolved = consistentResolve(node.resolved, node.path, path, true)
if (resolved) {
// hide resolved from registry dependencies.
if (resolved &&
!(options.omitLockfileRegistryResolved && node.isRegistryDependency)) {
meta.resolved = resolved
}

Expand Down Expand Up @@ -331,6 +333,7 @@ class Shrinkwrap {
hiddenLockfile = false,
log = procLog,
lockfileVersion,
omitLockfileRegistryResolved = false,
} = options

this.lockfileVersion = hiddenLockfile ? 3
Expand All @@ -349,6 +352,7 @@ class Shrinkwrap {
this.yarnLock = null
this.hiddenLockfile = hiddenLockfile
this.loadingError = null
this.resolvedOptions = { omitLockfileRegistryResolved }
// only load npm-shrinkwrap.json in dep trees, not package-lock
this.shrinkwrapOnly = shrinkwrapOnly
}
Expand Down Expand Up @@ -823,7 +827,7 @@ class Shrinkwrap {
resolved,
integrity,
hasShrinkwrap,
} = Shrinkwrap.metaFromNode(node, this.path)
} = Shrinkwrap.metaFromNode(node, this.path, this.resolvedOptions)
node.resolved = node.resolved || resolved || null
node.integrity = node.integrity || integrity || null
node.hasShrinkwrap = node.hasShrinkwrap || hasShrinkwrap || false
Expand Down Expand Up @@ -879,15 +883,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.resolvedOptions)
}

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.resolvedOptions)
this.data.packages = {}
if (Object.keys(root).length) {
this.data.packages[''] = root
Expand All @@ -898,7 +908,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.resolvedOptions)
}
} else if (this[_awaitingUpdate].size > 0) {
for (const loc of this[_awaitingUpdate].keys()) {
Expand Down Expand Up @@ -999,6 +1012,8 @@ class Shrinkwrap {
// git, file, dir, or remote, then the resolved value is necessary.
if (node.resolved &&
!node.isLink &&
!(node.isRegistryDependency &&
this.resolvedOptions.omitLockfileRegistryResolved) &&
rSpec.type !== 'git' &&
rSpec.type !== 'file' &&
rSpec.type !== 'directory' &&
Expand Down
23 changes: 23 additions & 0 deletions test/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -2842,3 +2842,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)
})
70 changes: 70 additions & 0 deletions test/shrinkwrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,76 @@ t.test('throws when attempting to access data before loading', t => {
t.end()
})

t.test('omitLockfileRegistryResolved', async t => {
const url = 'https://registry.npmjs.org/registry/-/registry-1.2.3.tgz'
const getData = async (omitLockfileRegistryResolved) => {
const dir = t.testdir()
const meta = await Shrinkwrap.load({
path: dir,
omitLockfileRegistryResolved})

const root = new Node({
pkg: {
name: 'root',
dependencies: {
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 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 }
}

await t.test('true', async t => {
const { data } = await getData(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('false', async t => {
const { data } = await getData(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(false)
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 37ad149

Please sign in to comment.