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(arborist) omit resolved from registry dependencies #4262

Closed
wants to merge 2 commits into from
Closed
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
2 changes: 2 additions & 0 deletions workspaces/arborist/lib/arborist/build-ideal-tree.js
Expand Up @@ -310,6 +310,7 @@ module.exports = cls => class IdealTreeBuilder extends cls {
? 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 @@ -354,6 +355,7 @@ module.exports = cls => class IdealTreeBuilder extends cls {
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 @@ -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
52 changes: 52 additions & 0 deletions workspaces/arborist/lib/override-resolves.js
@@ -0,0 +1,52 @@
const npa = require('npm-package-arg')

function overrideResolves (node, resolved, opts = {}) {
const {
omitLockfileRegistryResolved = false,
recordDefaultRegistry = false,
} = opts

const isRegistryDependency = node.isRegistryDependency

// omit the resolved url of registry dependencies. this makes installs slower
// because npm must resolve the url for each package version but it allows
// users to use a lockfile across registries that host tarballs at different
// paths.
if (isRegistryDependency && omitLockfileRegistryResolved) {
return undefined
}

// replace the configured registry with the default registry. the default
// registry is a magic value meaning the current registry, so recording
// resolved with the default registry allows users to switch to a
// different registry without removing their lockfile. The path portion
// of the resolved url is preserved so this trick only works when the
// different registries host tarballs at the same relative paths.
if (isRegistryDependency && recordDefaultRegistry) {
const scope = npa(node.packageName).scope
const registry = scope && opts[`${scope}:registry`]
? opts[`${scope}:registry`]
: opts.registry

// normalize registry url - strip trailing slash.
// TODO improve normalization for both the configured registry and resolved
// url. consider port, protocol, more path normalization.
const normalized = registry.endsWith('/')
? registry.slice(0, -1)
: registry

// only replace the host if the resolved url is for the configured
// registry. registries may host tarballs on another server. return
// undefined so npm will re-resolve the url from the current registry when
// it reads the lockfile.
if (resolved.startsWith(normalized)) {
return 'https://registry.npmjs.org' + resolved.slice(normalized.length)
} else {
return undefined
}
}

return resolved
}

module.exports = { overrideResolves }
26 changes: 19 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 @@ -300,7 +301,7 @@ class Shrinkwrap {

const resolved = consistentResolve(node.resolved, node.path, path, true)
if (resolved) {
meta.resolved = resolved
meta.resolved = overrideResolves(node, resolved, options)
}

if (node.extraneous) {
Expand Down Expand Up @@ -331,6 +332,7 @@ class Shrinkwrap {
hiddenLockfile = false,
log = procLog,
lockfileVersion,
resolveOptions = {},
} = options

this.lockfileVersion = hiddenLockfile ? 3
Expand All @@ -349,6 +351,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 @@ -827,7 +830,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 @@ -883,15 +886,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 @@ -902,7 +911,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 @@ -1010,7 +1022,7 @@ class Shrinkwrap {
spec.type !== 'git' &&
spec.type !== 'file' &&
spec.type !== 'remote') {
lock.resolved = node.resolved
lock.resolved = overrideResolves(node, node.resolved, this.resolveOptions)
}

if (node.integrity) {
Expand Down
23 changes: 23 additions & 0 deletions workspaces/arborist/test/node.js
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)
})
115 changes: 115 additions & 0 deletions workspaces/arborist/test/shrinkwrap.js
Expand Up @@ -231,6 +231,121 @@ 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',
'@scope/some-package': '^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-package', 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)
})

await t.test('recordDefaultRegistry: true', async t => {
const { data } = await getData({
recordDefaultRegistry: true,
registry: 'https://private.registry.org/deadbeef',
'@scope:registry': 'https://someother.registry.org',
})

// unscoped packages that resolve to their configured registry should be
// record the default registry
t.strictSame(data.packages['node_modules/registry'].resolved,
'https://registry.npmjs.org/registry/-/registry-1.2.3.tgz')
t.strictSame(data.dependencies.registry.resolved,
'https://registry.npmjs.org/registry/-/registry-1.2.3.tgz')

// scoped packages that resolve to their configured registry should be
// record the default registry
t.strictSame(data.packages['node_modules/@scope/some-package'].resolved,
'https://registry.npmjs.org/registry/-/registry-1.2.3.tgz')
t.strictSame(data.dependencies['@scope/some-package'].resolved,
'https://registry.npmjs.org/registry/-/registry-1.2.3.tgz')

// packages with resolved urls that don't match the configured registry
// should record undefined so npm resolves their url again.
t.strictSame(data.packages['node_modules/some-other-registry'].resolved, undefined)
t.strictSame(data.dependencies['some-other-registry'].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