Skip to content

Commit

Permalink
fix: improve heuristics when dist-tags.latest is in range
Browse files Browse the repository at this point in the history
Previously, there were two problems.

First problem, even though we heuristically choose the version that best
suits the stated 'engines' restriction, we were skipping that check when
the 'defaultTag' (ie, 'latest', typically) version was a semver match.

Second problem, the heuristic was improperly being set for 'staged' and
'restricted' packages, resulting in failure to sort those versions
properly.

Only choose the defaultTag version if it both a semver match, _and_
passes the engines/staged/restricted heuristics, and apply those
heuristics properly in the sort that comes later, if the defaultTag
version is not used.

Related-to: npm/rfcs#405
  • Loading branch information
isaacs committed Jul 28, 2021
1 parent e79d08b commit 66b46ab
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 6 deletions.
16 changes: 11 additions & 5 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,15 @@ const pickManifest = (packument, wanted, opts) => {
const defaultVer = distTags[defaultTag]
if (defaultVer &&
(range === '*' || semver.satisfies(defaultVer, range, { loose: true })) &&
!restricted[defaultVer] &&
!shouldAvoid(defaultVer, avoid)) {
const mani = versions[defaultVer]
if (mani && isBefore(verTimes, defaultVer, time)) {
const ok = mani &&
isBefore(verTimes, defaultVer, time) &&
engineOk(mani, npmVersion, nodeVersion) &&
!mani.deprecated &&
!staged[defaultVer]
if (ok) {
return mani
}
}
Expand Down Expand Up @@ -155,10 +161,10 @@ const pickManifest = (packument, wanted, opts) => {
const [verb, manib] = b
const notavoida = !shouldAvoid(vera, avoid)
const notavoidb = !shouldAvoid(verb, avoid)
const notrestra = !restricted[a]
const notrestrb = !restricted[b]
const notstagea = !staged[a]
const notstageb = !staged[b]
const notrestra = !restricted[vera]
const notrestrb = !restricted[verb]
const notstagea = !staged[vera]
const notstageb = !staged[verb]
const notdepra = !mania.deprecated
const notdeprb = !manib.deprecated
const enginea = engineOk(mania, npmVersion, nodeVersion)
Expand Down
13 changes: 12 additions & 1 deletion test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,9 @@ test('ETARGET if range does not match anything', t => {

test('E403 if version is forbidden', t => {
const metadata = {
'dist-tags': {
latest: '2.1.0' // do not default the latest if restricted
},
policyRestrictions: {
versions: {
'2.1.0': { version: '2.1.0' }
Expand All @@ -141,6 +144,9 @@ test('E403 if version is forbidden', t => {
'2.0.5': { version: '2.0.5' }
}
}
t.equal(pickManifest(metadata, '2').version, '2.0.5')
t.equal(pickManifest(metadata, '').version, '2.0.5')
t.equal(pickManifest(metadata, '1 || 2').version, '2.0.5')
t.throws(() => {
pickManifest(metadata, '2.1.0')
}, { code: 'E403' }, 'got correct error on match failure')
Expand Down Expand Up @@ -419,13 +425,18 @@ test('accepts opts.before option to do date-based cutoffs', t => {

test('prefers versions that satisfy the engines requirement', t => {
const pack = {
'dist-tags': {
latest: '1.5.0' // do not default latest if engine mismatch
},
versions: {
'1.0.0': { version: '1.0.0', engines: { node: '>=4' } },
'1.1.0': { version: '1.1.0', engines: { node: '>=6' } },
'1.2.0': { version: '1.2.0', engines: { node: '>=8' } },
'1.3.0': { version: '1.3.0', engines: { node: '>=10' } },
'1.4.0': { version: '1.4.0', engines: { node: '>=12' } },
'1.5.0': { version: '1.5.0', engines: { node: '>=14' } }
'1.5.0': { version: '1.5.0', engines: { node: '>=14' } },
// not tagged as latest, won't be chosen by default
'1.5.1': { version: '1.5.0', engines: { node: '>=14' } }
}
}

Expand Down

0 comments on commit 66b46ab

Please sign in to comment.