Skip to content
This repository was archived by the owner on Jan 20, 2022. It is now read-only.

Commit 38cee94

Browse files
lukekarrysisaacs
lukekarrys
authored and
isaacs
committed
fix: set lockfileVersion from file during reset
Fix: npm/cli#3920 When loading the initial tree while updating all, a shrinkwrap file loaded from disk with an existing `lockfileVersion` was not having the `lockfileVersion` preserved when saving the file back to disk. With this patch, an existing `lockfileVersion` is preserved if it is greater than the `defaultLockfileVersion`. Co-authored-by: @isaacs PR-URL: #340 Credit: @lukekarrys Close: #340 Reviewed-by: @isaacs
1 parent d310bd3 commit 38cee94

3 files changed

Lines changed: 107 additions & 34 deletions

File tree

‎lib/shrinkwrap.js‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -238,21 +238,31 @@ class Shrinkwrap {
238238
return swKeyOrder
239239
}
240240

241-
static reset (options) {
241+
static async reset (options) {
242242
// still need to know if it was loaded from the disk, but don't
243243
// bother reading it if we're gonna just throw it away.
244244
const s = new Shrinkwrap(options)
245245
s.reset()
246246

247-
return s[_maybeStat]().then(([sw, lock]) => {
248-
s.filename = resolve(s.path,
249-
(s.hiddenLockfile ? 'node_modules/.package-lock'
250-
: s.shrinkwrapOnly || sw ? 'npm-shrinkwrap'
251-
: 'package-lock') + '.json')
252-
s.loadedFromDisk = !!(sw || lock)
253-
s.type = basename(s.filename)
254-
return s
255-
})
247+
const [sw, lock] = await s[_maybeStat]()
248+
249+
s.filename = resolve(s.path,
250+
(s.hiddenLockfile ? 'node_modules/.package-lock'
251+
: s.shrinkwrapOnly || sw ? 'npm-shrinkwrap'
252+
: 'package-lock') + '.json')
253+
s.loadedFromDisk = !!(sw || lock)
254+
s.type = basename(s.filename)
255+
256+
try {
257+
if (s.loadedFromDisk && !s.lockfileVersion) {
258+
const json = parseJSON(await maybeReadFile(s.filename))
259+
if (json.lockfileVersion > defaultLockfileVersion) {
260+
s.lockfileVersion = json.lockfileVersion
261+
}
262+
}
263+
} catch (e) {}
264+
265+
return s
256266
}
257267

258268
static metaFromNode (node, path) {

‎test/arborist/build-ideal-tree.js‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -638,6 +638,34 @@ t.test('empty update should not trigger old lockfile', async t => {
638638
t.strictSame(checkLogs(), [])
639639
})
640640

641+
t.test('update v3 doesnt downgrade lockfile', async t => {
642+
const fixt = t.testdir({
643+
'package-lock.json': JSON.stringify({
644+
name: 'empty-update-v3',
645+
version: '1.0.0',
646+
lockfileVersion: 3,
647+
requires: true,
648+
packages: {
649+
'': {
650+
name: 'empty-update-v3',
651+
version: '1.0.0',
652+
},
653+
},
654+
}),
655+
'package.json': JSON.stringify({
656+
name: 'empty-update-v3',
657+
version: '1.0.0',
658+
}),
659+
})
660+
661+
const arb = newArb(fixt)
662+
await arb.reify({ update: true })
663+
664+
const { lockfileVersion } = require(resolve(fixt, 'package-lock.json'))
665+
666+
t.strictSame(lockfileVersion, 3)
667+
})
668+
641669
t.test('no fix available', async t => {
642670
const path = resolve(fixtures, 'audit-mkdirp/mkdirp-unfixable')
643671
const checkLogs = warningTracker()

‎test/shrinkwrap.js‎

Lines changed: 59 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,35 @@ t.test('starting out with a reset lockfile is an empty lockfile', t =>
8080
t.equal(sw.filename, resolve(fixture, 'package-lock.json'))
8181
}))
8282

83+
t.test('reset in a bad dir gets an empty lockfile with no lockfile version', async t => {
84+
const nullLockDir = t.testdir({
85+
'package-lock.json': JSON.stringify(null),
86+
})
87+
88+
const [swMissingLock, swNullLock] = await Promise.all([
89+
Shrinkwrap.reset({ path: 'path/which/does/not/exist' }),
90+
Shrinkwrap.reset({ path: nullLockDir }),
91+
])
92+
93+
t.strictSame(swMissingLock.data, {
94+
lockfileVersion: 2,
95+
requires: true,
96+
dependencies: {},
97+
packages: {},
98+
})
99+
t.equal(swMissingLock.lockfileVersion, null)
100+
t.equal(swMissingLock.loadedFromDisk, false)
101+
102+
t.strictSame(swNullLock.data, {
103+
lockfileVersion: 2,
104+
requires: true,
105+
dependencies: {},
106+
packages: {},
107+
})
108+
t.equal(swNullLock.lockfileVersion, null)
109+
t.equal(swNullLock.loadedFromDisk, true)
110+
})
111+
83112
t.test('loading in bad dir gets empty lockfile', t =>
84113
Shrinkwrap.load({ path: 'path/which/does/not/exist' }).then(sw => {
85114
t.strictSame(sw.data, {
@@ -1509,37 +1538,43 @@ t.test('setting lockfileVersion from the file contents', async t => {
15091538
'package-lock.json': JSON.stringify({ lockfileVersion: 3 }),
15101539
},
15111540
})
1541+
1542+
const loadAndReset = (options) => Promise.all([
1543+
Shrinkwrap.load(options).then((s) => s.lockfileVersion),
1544+
Shrinkwrap.reset(options).then((s) => s.lockfileVersion),
1545+
])
1546+
15121547
t.test('default setting', async t => {
1513-
const s1 = await Shrinkwrap.load({ path: `${path}/v1` })
1514-
t.equal(s1.lockfileVersion, 2, 'will upgrade old lockfile')
1515-
const s2 = await Shrinkwrap.load({ path: `${path}/v2` })
1516-
t.equal(s2.lockfileVersion, 2, 'will keep v2 as v2')
1517-
const s3 = await Shrinkwrap.load({ path: `${path}/v3` })
1518-
t.equal(s3.lockfileVersion, 3, 'will keep v3 as v3')
1548+
const s1 = await loadAndReset({ path: `${path}/v1` })
1549+
t.strictSame(s1, [2, null], 'will upgrade old lockfile')
1550+
const s2 = await loadAndReset({ path: `${path}/v2` })
1551+
t.strictSame(s2, [2, null], 'will keep v2 as v2')
1552+
const s3 = await loadAndReset({ path: `${path}/v3` })
1553+
t.strictSame(s3, [3, 3], 'load will keep v3 as v3')
15191554
})
15201555
t.test('v1', async t => {
1521-
const s1 = await Shrinkwrap.load({ path: `${path}/v1`, lockfileVersion: 1 })
1522-
t.equal(s1.lockfileVersion, 1, 'keep explicit v1 setting')
1523-
const s2 = await Shrinkwrap.load({ path: `${path}/v2`, lockfileVersion: 1 })
1524-
t.equal(s2.lockfileVersion, 1, 'downgrade explicitly')
1525-
const s3 = await Shrinkwrap.load({ path: `${path}/v3`, lockfileVersion: 1 })
1526-
t.equal(s3.lockfileVersion, 1, 'downgrade explicitly')
1556+
const s1 = await loadAndReset({ path: `${path}/v1`, lockfileVersion: 1 })
1557+
t.strictSame(s1, [1, 1], 'keep explicit v1 setting')
1558+
const s2 = await loadAndReset({ path: `${path}/v2`, lockfileVersion: 1 })
1559+
t.strictSame(s2, [1, 1], 'downgrade explicitly')
1560+
const s3 = await loadAndReset({ path: `${path}/v3`, lockfileVersion: 1 })
1561+
t.strictSame(s3, [1, 1], 'downgrade explicitly')
15271562
})
15281563
t.test('v2', async t => {
1529-
const s1 = await Shrinkwrap.load({ path: `${path}/v1`, lockfileVersion: 2 })
1530-
t.equal(s1.lockfileVersion, 2, 'upgrade explicitly')
1531-
const s2 = await Shrinkwrap.load({ path: `${path}/v2`, lockfileVersion: 2 })
1532-
t.equal(s2.lockfileVersion, 2, 'keep v2 setting')
1533-
const s3 = await Shrinkwrap.load({ path: `${path}/v3`, lockfileVersion: 2 })
1534-
t.equal(s3.lockfileVersion, 2, 'downgrade explicitly')
1564+
const s1 = await loadAndReset({ path: `${path}/v1`, lockfileVersion: 2 })
1565+
t.strictSame(s1, [2, 2], 'upgrade explicitly')
1566+
const s2 = await loadAndReset({ path: `${path}/v2`, lockfileVersion: 2 })
1567+
t.strictSame(s2, [2, 2], 'keep v2 setting')
1568+
const s3 = await loadAndReset({ path: `${path}/v3`, lockfileVersion: 2 })
1569+
t.strictSame(s3, [2, 2], 'downgrade explicitly')
15351570
})
15361571
t.test('v3', async t => {
1537-
const s1 = await Shrinkwrap.load({ path: `${path}/v1`, lockfileVersion: 3 })
1538-
t.equal(s1.lockfileVersion, 3, 'upgrade explicitly')
1539-
const s2 = await Shrinkwrap.load({ path: `${path}/v2`, lockfileVersion: 3 })
1540-
t.equal(s2.lockfileVersion, 3, 'upgrade explicitly')
1541-
const s3 = await Shrinkwrap.load({ path: `${path}/v3`, lockfileVersion: 3 })
1542-
t.equal(s3.lockfileVersion, 3, 'keep v3 setting')
1572+
const s1 = await loadAndReset({ path: `${path}/v1`, lockfileVersion: 3 })
1573+
t.strictSame(s1, [3, 3], 'upgrade explicitly')
1574+
const s2 = await loadAndReset({ path: `${path}/v2`, lockfileVersion: 3 })
1575+
t.strictSame(s2, [3, 3], 'upgrade explicitly')
1576+
const s3 = await loadAndReset({ path: `${path}/v3`, lockfileVersion: 3 })
1577+
t.strictSame(s3, [3, 3], 'keep v3 setting')
15431578
})
15441579

15451580
t.equal(Shrinkwrap.defaultLockfileVersion, 2, 'default is 2')

0 commit comments

Comments
 (0)