Skip to content

Commit 7aaa344

Browse files
author
Caleb ツ Everett
committed
feat: omit resolved from registry dependencies
Implement `$disable-write-resolves` described in npm/rfcs#486. I named the option `omitLockfileRegistryResolved` but that can be changed later. Put simply, this option causes npm to create lock files without a `resolved` key for registry dependencies forcing npm to use the current configured registry and resolve package tarball urls on install. This fixes install errors when users change registries and the recorded resolved url is incorrect. This option causes slower installs because npm must fetch each packages manifest to find the tarball url, but it's the most comprehensive solution to this problem. Options like recording always the default registry, or recording a special 'current registry' sigil will break if registries host tarballs at different paths. For example `${REGISTRY}/npm/-/npm-8.3.0.tgz` only works if all registries host tarballs at `npm/-/npm-8.3.0.tgz`.
1 parent f37f7d2 commit 7aaa344

File tree

8 files changed

+163
-7
lines changed

8 files changed

+163
-7
lines changed

workspaces/arborist/lib/arborist/build-ideal-tree.js

+2
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,7 @@ Try using the package name instead, e.g:
327327
? Shrinkwrap.reset({
328328
path: this.path,
329329
lockfileVersion: this.options.lockfileVersion,
330+
resolveOptions: this.options,
330331
}).then(meta => Object.assign(root, { meta }))
331332
: this.loadVirtual({ root }))
332333

@@ -386,6 +387,7 @@ Try using the package name instead, e.g:
386387
const meta = new Shrinkwrap({
387388
path: this.path,
388389
lockfileVersion: this.options.lockfileVersion,
390+
resolveOptions: this.options,
389391
})
390392
meta.reset()
391393
root.meta = meta

workspaces/arborist/lib/arborist/load-actual.js

+2
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ module.exports = cls => class ActualLoader extends cls {
147147
const meta = await Shrinkwrap.load({
148148
path: this[_actualTree].path,
149149
hiddenLockfile: true,
150+
resolveOptions: this.options,
150151
})
151152
if (meta.loadedFromDisk) {
152153
this[_actualTree].meta = meta
@@ -155,6 +156,7 @@ module.exports = cls => class ActualLoader extends cls {
155156
const meta = await Shrinkwrap.load({
156157
path: this[_actualTree].path,
157158
lockfileVersion: this.options.lockfileVersion,
159+
resolveOptions: this.options,
158160
})
159161
this[_actualTree].meta = meta
160162
return this[_loadActualActually]({ root, ignoreMissing })

workspaces/arborist/lib/arborist/load-virtual.js

+1
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ module.exports = cls => class VirtualLoader extends cls {
5757
const s = await Shrinkwrap.load({
5858
path: this.path,
5959
lockfileVersion: this.options.lockfileVersion,
60+
resolveOptions: this.options,
6061
})
6162
if (!s.loadedFromDisk && !options.root) {
6263
const er = new Error('loadVirtual requires existing shrinkwrap file')

workspaces/arborist/lib/node.js

+12
Original file line numberDiff line numberDiff line change
@@ -522,6 +522,18 @@ class Node {
522522
return this === this.root || this === this.root.target
523523
}
524524

525+
get isRegistryDependency () {
526+
if (this.edgesIn.size === 0) {
527+
return false
528+
}
529+
for (const edge of this.edgesIn) {
530+
if (!npa(edge.spec).registry) {
531+
return false
532+
}
533+
}
534+
return true
535+
}
536+
525537
* ancestry () {
526538
for (let anc = this; anc; anc = anc.resolveParent) {
527539
yield anc
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
function overrideResolves (resolved, opts = {}) {
2+
const { omitLockfileRegistryResolved = false } = opts
3+
4+
if (omitLockfileRegistryResolved) {
5+
return undefined
6+
}
7+
8+
return resolved
9+
}
10+
11+
module.exports = { overrideResolves }

workspaces/arborist/lib/shrinkwrap.js

+24-7
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ const specFromResolved = resolved => {
9595
const relpath = require('./relpath.js')
9696

9797
const consistentResolve = require('./consistent-resolve.js')
98+
const { overrideResolves } = require('./override-resolves.js')
9899

99100
const maybeReadFile = file => {
100101
return readFile(file, 'utf8').then(d => d, er => {
@@ -265,7 +266,7 @@ class Shrinkwrap {
265266
return s
266267
}
267268

268-
static metaFromNode (node, path) {
269+
static metaFromNode (node, path, options = {}) {
269270
if (node.isLink) {
270271
return {
271272
resolved: relpath(path, node.realpath),
@@ -299,7 +300,12 @@ class Shrinkwrap {
299300
})
300301

301302
const resolved = consistentResolve(node.resolved, node.path, path, true)
302-
if (resolved) {
303+
// hide resolved from registry dependencies.
304+
if (!resolved) {
305+
// no-op
306+
} else if (node.isRegistryDependency) {
307+
meta.resolved = overrideResolves(resolved, options)
308+
} else {
303309
meta.resolved = resolved
304310
}
305311

@@ -330,6 +336,7 @@ class Shrinkwrap {
330336
shrinkwrapOnly = false,
331337
hiddenLockfile = false,
332338
lockfileVersion,
339+
resolveOptions = {},
333340
} = options
334341

335342
this.lockfileVersion = hiddenLockfile ? 3
@@ -347,6 +354,7 @@ class Shrinkwrap {
347354
this.yarnLock = null
348355
this.hiddenLockfile = hiddenLockfile
349356
this.loadingError = null
357+
this.resolveOptions = resolveOptions
350358
// only load npm-shrinkwrap.json in dep trees, not package-lock
351359
this.shrinkwrapOnly = shrinkwrapOnly
352360
}
@@ -830,7 +838,7 @@ class Shrinkwrap {
830838
resolved,
831839
integrity,
832840
hasShrinkwrap,
833-
} = Shrinkwrap.metaFromNode(node, this.path)
841+
} = Shrinkwrap.metaFromNode(node, this.path, this.resolveOptions)
834842
node.resolved = node.resolved || resolved || null
835843
node.integrity = node.integrity || integrity || null
836844
node.hasShrinkwrap = node.hasShrinkwrap || hasShrinkwrap || false
@@ -886,15 +894,21 @@ class Shrinkwrap {
886894
[_updateWaitingNode] (loc) {
887895
const node = this[_awaitingUpdate].get(loc)
888896
this[_awaitingUpdate].delete(loc)
889-
this.data.packages[loc] = Shrinkwrap.metaFromNode(node, this.path)
897+
this.data.packages[loc] = Shrinkwrap.metaFromNode(
898+
node,
899+
this.path,
900+
this.resolveOptions)
890901
}
891902

892903
commit () {
893904
if (this.tree) {
894905
if (this.yarnLock) {
895906
this.yarnLock.fromTree(this.tree)
896907
}
897-
const root = Shrinkwrap.metaFromNode(this.tree.target, this.path)
908+
const root = Shrinkwrap.metaFromNode(
909+
this.tree.target,
910+
this.path,
911+
this.resolveOptions)
898912
this.data.packages = {}
899913
if (Object.keys(root).length) {
900914
this.data.packages[''] = root
@@ -905,7 +919,10 @@ class Shrinkwrap {
905919
continue
906920
}
907921
const loc = relpath(this.path, node.path)
908-
this.data.packages[loc] = Shrinkwrap.metaFromNode(node, this.path)
922+
this.data.packages[loc] = Shrinkwrap.metaFromNode(
923+
node,
924+
this.path,
925+
this.resolveOptions)
909926
}
910927
} else if (this[_awaitingUpdate].size > 0) {
911928
for (const loc of this[_awaitingUpdate].keys()) {
@@ -1013,7 +1030,7 @@ class Shrinkwrap {
10131030
spec.type !== 'git' &&
10141031
spec.type !== 'file' &&
10151032
spec.type !== 'remote') {
1016-
lock.resolved = node.resolved
1033+
lock.resolved = overrideResolves(node.resolved, this.resolveOptions)
10171034
}
10181035

10191036
if (node.integrity) {

workspaces/arborist/test/node.js

+23
Original file line numberDiff line numberDiff line change
@@ -2842,3 +2842,26 @@ t.test('overrides', (t) => {
28422842

28432843
t.end()
28442844
})
2845+
2846+
t.test('node with no edges in is not a registry dep', async t => {
2847+
const node = new Node({ path: '/foo' })
2848+
t.equal(node.isRegistryDependency, false)
2849+
})
2850+
2851+
t.test('node with non registry edge in is not a registry dep', async t => {
2852+
const root = new Node({ path: '/some/path', pkg: { dependencies: { registry: '', tar: '' } } })
2853+
const node = new Node({ pkg: { name: 'node', version: '1.0.0' }, parent: root })
2854+
2855+
new Node({ pkg: { name: 'registry', dependencies: { node: '^1.0.0' } }, parent: root })
2856+
new Node({ pkg: { name: 'tar', dependencies: { node: 'file:node' } }, parent: root })
2857+
2858+
t.equal(node.isRegistryDependency, false)
2859+
})
2860+
2861+
t.test('node with only registry edges in a registry dep', async t => {
2862+
const root = new Node({ path: '/some/path', pkg: { dependencies: { registry: '', tar: '' } } })
2863+
const node = new Node({ pkg: { name: 'node', version: '1.0.0' }, parent: root })
2864+
new Node({ pkg: { name: 'registry', dependencies: { node: '^1.0.0' } }, parent: root })
2865+
2866+
t.equal(node.isRegistryDependency, true)
2867+
})

workspaces/arborist/test/shrinkwrap.js

+88
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,94 @@ t.test('throws when attempting to access data before loading', t => {
231231
t.end()
232232
})
233233

234+
t.only('resolveOptions', async t => {
235+
const url = 'https://private.registry.org/deadbeef/registry/-/registry-1.2.3.tgz'
236+
const someOtherRegistry = 'https://someother.registry.org/registry/-/registry-1.2.3.tgz'
237+
const getData = async (resolveOptions) => {
238+
const dir = t.testdir()
239+
const meta = await Shrinkwrap.load({
240+
path: dir,
241+
resolveOptions,
242+
})
243+
244+
const root = new Node({
245+
pkg: {
246+
name: 'root',
247+
dependencies: {
248+
registry: '^1.0.0',
249+
'some-other-registry': '^1.0.0',
250+
'@scoped/some-other-registry': '^1.0.0',
251+
tar: url,
252+
},
253+
},
254+
path: dir,
255+
realpath: dir,
256+
meta,
257+
})
258+
259+
const registry = new Node({
260+
pkg: { name: 'registry', version: '1.2.3' },
261+
resolved: url,
262+
integrity: 'sha512-registry',
263+
parent: root,
264+
})
265+
266+
const otherRegistry = new Node({
267+
pkg: { name: 'some-other-registry', version: '1.2.3' },
268+
resolved: someOtherRegistry,
269+
integrity: 'sha512-registry',
270+
parent: root,
271+
})
272+
273+
const scopedOtherRegistry = new Node({
274+
pkg: { name: '@scope/some-other-registry', version: '1.2.3' },
275+
resolved: someOtherRegistry,
276+
integrity: 'sha512-registry',
277+
parent: root,
278+
})
279+
280+
const tar = new Node({
281+
pkg: { name: 'tar', version: '1.2.3' },
282+
resolved: url,
283+
integrity: 'sha512-registry',
284+
parent: root,
285+
})
286+
287+
calcDepFlags(root)
288+
meta.add(root)
289+
return { data: meta.commit(), registry, tar, root, otherRegistry, scopedOtherRegistry }
290+
}
291+
292+
await t.test('omitLockfileRegistryResolved', async t => {
293+
const { data } = await getData({ omitLockfileRegistryResolved: true })
294+
// registry dependencies in v2 packages and v1 dependencies should
295+
// have resolved stripped.
296+
t.strictSame(data.packages['node_modules/registry'].resolved, undefined)
297+
t.strictSame(data.dependencies.registry.resolved, undefined)
298+
299+
// tar should have resolved because it is not a registry dep.
300+
t.strictSame(data.packages['node_modules/tar'].resolved, url)
301+
// v1 url dependencies never have resolved.
302+
t.strictSame(data.dependencies.tar.resolved, undefined)
303+
})
304+
305+
await t.test('omitLockfileRegistryResolved: false', async t => {
306+
const { data } = await getData({ omitLockfileRegistryResolved: false })
307+
t.strictSame(data.packages['node_modules/registry'].resolved, url)
308+
t.strictSame(data.dependencies.registry.resolved, url)
309+
310+
t.strictSame(data.packages['node_modules/tar'].resolved, url)
311+
// v1 url dependencies never have resolved.
312+
t.strictSame(data.dependencies.tar.resolved, undefined)
313+
})
314+
315+
t.test('metaFromNode default', async t => {
316+
// test to cover options default.
317+
const { registry } = await getData(undefined)
318+
t.strictSame(Shrinkwrap.metaFromNode(registry, '').resolved, url)
319+
})
320+
})
321+
234322
t.test('construct metadata from node and package data', t => {
235323
const meta = new Shrinkwrap({ path: '/home/user/projects/root' })
236324
// fake load

0 commit comments

Comments
 (0)