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

fix: set lockfileVersion from file during reset #340

Closed
wants to merge 1 commit into from
Closed

fix: set lockfileVersion from file during reset #340

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
30 changes: 20 additions & 10 deletions lib/shrinkwrap.js
View file
Open in desktop
Expand Up @@ -238,21 +238,31 @@ class Shrinkwrap {
return swKeyOrder
}

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

return s[_maybeStat]().then(([sw, lock]) => {
s.filename = resolve(s.path,
(s.hiddenLockfile ? 'node_modules/.package-lock'
: s.shrinkwrapOnly || sw ? 'npm-shrinkwrap'
: 'package-lock') + '.json')
s.loadedFromDisk = !!(sw || lock)
s.type = basename(s.filename)
return s
})
const [sw, lock] = await s[_maybeStat]()

s.filename = resolve(s.path,
(s.hiddenLockfile ? 'node_modules/.package-lock'
: s.shrinkwrapOnly || sw ? 'npm-shrinkwrap'
: 'package-lock') + '.json')
s.loadedFromDisk = !!(sw || lock)
s.type = basename(s.filename)

try {
if (s.loadedFromDisk && !s.lockfileVersion) {
const json = parseJSON(await maybeReadFile(s.filename))
if (json.lockfileVersion > defaultLockfileVersion) {
s.lockfileVersion = json.lockfileVersion
}
}
} catch (e) {}

return s
}

static metaFromNode (node, path) {
Expand Down
28 changes: 28 additions & 0 deletions test/arborist/build-ideal-tree.js
View file
Open in desktop
Expand Up @@ -611,6 +611,34 @@ t.test('force a new mkdirp (but not semver major)', async t => {
t.equal(arb.idealTree.children.get('minimist').package.version, '1.2.5')
})

t.test('update v3 doesnt downgrade lockfile', async t => {
const fixt = t.testdir({
'package-lock.json': JSON.stringify({
name: 'empty-update-v3',
version: '1.0.0',
lockfileVersion: 3,
requires: true,
packages: {
'': {
name: 'empty-update-v3',
version: '1.0.0',
},
},
}),
'package.json': JSON.stringify({
name: 'empty-update-v3',
version: '1.0.0',
}),
})

const arb = newArb(fixt)
await arb.reify({ update: true })

const { lockfileVersion } = require(resolve(fixt, 'package-lock.json'))

t.strictSame(lockfileVersion, 3)
})

t.test('no fix available', async t => {
const path = resolve(fixtures, 'audit-mkdirp/mkdirp-unfixable')
const checkLogs = warningTracker()
Expand Down
83 changes: 59 additions & 24 deletions test/shrinkwrap.js
View file
Open in desktop
Expand Up @@ -79,6 +79,35 @@ t.test('starting out with a reset lockfile is an empty lockfile', t =>
t.equal(sw.filename, resolve(fixture, 'package-lock.json'))
}))

t.test('reset in a bad dir gets an empty lockfile with no lockfile version', async t => {
const nullLockDir = t.testdir({
'package-lock.json': JSON.stringify(null),
})

const [swMissingLock, swNullLock] = await Promise.all([
Shrinkwrap.reset({ path: 'path/which/does/not/exist' }),
Shrinkwrap.reset({ path: nullLockDir }),
])

t.strictSame(swMissingLock.data, {
lockfileVersion: 2,
requires: true,
dependencies: {},
packages: {},
})
t.equal(swMissingLock.lockfileVersion, null)
t.equal(swMissingLock.loadedFromDisk, false)

t.strictSame(swNullLock.data, {
lockfileVersion: 2,
requires: true,
dependencies: {},
packages: {},
})
t.equal(swNullLock.lockfileVersion, null)
t.equal(swNullLock.loadedFromDisk, true)
})

t.test('loading in bad dir gets empty lockfile', t =>
Shrinkwrap.load({ path: 'path/which/does/not/exist' }).then(sw => {
t.strictSame(sw.data, {
Expand Down Expand Up @@ -1508,37 +1537,43 @@ t.test('setting lockfileVersion from the file contents', async t => {
'package-lock.json': JSON.stringify({ lockfileVersion: 3 }),
},
})

const loadAndReset = (options) => Promise.all([
Shrinkwrap.load(options).then((s) => s.lockfileVersion),
Shrinkwrap.reset(options).then((s) => s.lockfileVersion),
])
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought it was worthwhile to reuse the existing tests for Shrinkwrap.load to ensure that all calls to Shrinkwrap.reset work as expected.

Currently there are some calls to Shrinkwrap.reset that leave lockfileVersion as null when the existing shrinkwrap lockfileVersion is less than or equal to the default version. I'm not sure if it's worth changing so that reset always sets lockfileVersion to 1 || 2 || 3 when reading from disk.


t.test('default setting', async t => {
const s1 = await Shrinkwrap.load({ path: `${path}/v1` })
t.equal(s1.lockfileVersion, 2, 'will upgrade old lockfile')
const s2 = await Shrinkwrap.load({ path: `${path}/v2` })
t.equal(s2.lockfileVersion, 2, 'will keep v2 as v2')
const s3 = await Shrinkwrap.load({ path: `${path}/v3` })
t.equal(s3.lockfileVersion, 3, 'will keep v3 as v3')
const s1 = await loadAndReset({ path: `${path}/v1` })
t.strictSame(s1, [2, null], 'will upgrade old lockfile')
const s2 = await loadAndReset({ path: `${path}/v2` })
t.strictSame(s2, [2, null], 'will keep v2 as v2')
const s3 = await loadAndReset({ path: `${path}/v3` })
t.strictSame(s3, [3, 3], 'load will keep v3 as v3')
})
t.test('v1', async t => {
const s1 = await Shrinkwrap.load({ path: `${path}/v1`, lockfileVersion: 1 })
t.equal(s1.lockfileVersion, 1, 'keep explicit v1 setting')
const s2 = await Shrinkwrap.load({ path: `${path}/v2`, lockfileVersion: 1 })
t.equal(s2.lockfileVersion, 1, 'downgrade explicitly')
const s3 = await Shrinkwrap.load({ path: `${path}/v3`, lockfileVersion: 1 })
t.equal(s3.lockfileVersion, 1, 'downgrade explicitly')
const s1 = await loadAndReset({ path: `${path}/v1`, lockfileVersion: 1 })
t.strictSame(s1, [1, 1], 'keep explicit v1 setting')
const s2 = await loadAndReset({ path: `${path}/v2`, lockfileVersion: 1 })
t.strictSame(s2, [1, 1], 'downgrade explicitly')
const s3 = await loadAndReset({ path: `${path}/v3`, lockfileVersion: 1 })
t.strictSame(s3, [1, 1], 'downgrade explicitly')
})
t.test('v2', async t => {
const s1 = await Shrinkwrap.load({ path: `${path}/v1`, lockfileVersion: 2 })
t.equal(s1.lockfileVersion, 2, 'upgrade explicitly')
const s2 = await Shrinkwrap.load({ path: `${path}/v2`, lockfileVersion: 2 })
t.equal(s2.lockfileVersion, 2, 'keep v2 setting')
const s3 = await Shrinkwrap.load({ path: `${path}/v3`, lockfileVersion: 2 })
t.equal(s3.lockfileVersion, 2, 'downgrade explicitly')
const s1 = await loadAndReset({ path: `${path}/v1`, lockfileVersion: 2 })
t.strictSame(s1, [2, 2], 'upgrade explicitly')
const s2 = await loadAndReset({ path: `${path}/v2`, lockfileVersion: 2 })
t.strictSame(s2, [2, 2], 'keep v2 setting')
const s3 = await loadAndReset({ path: `${path}/v3`, lockfileVersion: 2 })
t.strictSame(s3, [2, 2], 'downgrade explicitly')
})
t.test('v3', async t => {
const s1 = await Shrinkwrap.load({ path: `${path}/v1`, lockfileVersion: 3 })
t.equal(s1.lockfileVersion, 3, 'upgrade explicitly')
const s2 = await Shrinkwrap.load({ path: `${path}/v2`, lockfileVersion: 3 })
t.equal(s2.lockfileVersion, 3, 'upgrade explicitly')
const s3 = await Shrinkwrap.load({ path: `${path}/v3`, lockfileVersion: 3 })
t.equal(s3.lockfileVersion, 3, 'keep v3 setting')
const s1 = await loadAndReset({ path: `${path}/v1`, lockfileVersion: 3 })
t.strictSame(s1, [3, 3], 'upgrade explicitly')
const s2 = await loadAndReset({ path: `${path}/v2`, lockfileVersion: 3 })
t.strictSame(s2, [3, 3], 'upgrade explicitly')
const s3 = await loadAndReset({ path: `${path}/v3`, lockfileVersion: 3 })
t.strictSame(s3, [3, 3], 'keep v3 setting')
})

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