Skip to content

Commit

Permalink
feat: add avoidStrict option to strictly avoid
Browse files Browse the repository at this point in the history
PR-URL: #30
Credit: @isaacs
Close: #30
Reviewed-by: @isaacs
  • Loading branch information
isaacs committed Apr 7, 2020
1 parent c64973d commit c268796
Show file tree
Hide file tree
Showing 3 changed files with 144 additions and 13 deletions.
19 changes: 19 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,25 @@ All options are optional.
there is no other option, so when using this for version selection ensure
that you check the result against the range to see if there was no
alternative available.
* `avoidStrict` Boolean, default `false`. If set to true, then
`pickManifest` will never return a version in the `avoid` range. If the
only available version in the `wanted` range is a version that should be
avoided, then it will return a version _outside_ the `wanted` range,
preferring to do so without making a SemVer-major jump, if possible. If
there are no versions outside the `avoid` range, then throw an
`ETARGET` error. It does this by calling pickManifest first with the
`wanted` range, then with a `^` affixed to the version returned by the
`wanted` range, and then with a `*` version range, and throwing if
nothing could be found to satisfy the avoidance request.

Return value is the manifest as it exists in the packument, possibly
decorated with the following boolean flags:

* `_shouldAvoid` The version is in the `avoid` range. Watch out!
* `_outsideDependencyRange` The version is outside the `wanted` range,
because `avoidStrict: true` was set.
* `_isSemVerMajor` The `_outsideDependencyRange` result is a SemVer-major
step up from the version returned by the `wanted` range.

### Algorithm

Expand Down
75 changes: 63 additions & 12 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,68 @@ const engineOk = (manifest, npmVersion, nodeVersion) => {
const isBefore = (verTimes, ver, time) =>
!verTimes || !verTimes[ver] || Date.parse(verTimes[ver]) <= time

const avoidSemverOpt = { includePrerelease: true, loose: true }
const shouldAvoid = (ver, avoid) =>
avoid && semver.satisfies(ver, avoid, avoidSemverOpt)

const decorateAvoid = (result, avoid) =>
result && shouldAvoid(result.version, avoid)
? { ...result, _shouldAvoid: true }
: result

const pickManifest = (packument, wanted, opts) => {
const {
defaultTag = 'latest',
before = null,
nodeVersion = process.version,
npmVersion = null,
includeStaged = false,
avoid = null
avoid = null,
avoidStrict = false
} = opts

const { name, time: verTimes } = packument
const versions = packument.versions || {}

if (avoidStrict) {
const looseOpts = {
...opts,
avoidStrict: false
}

const result = pickManifest(packument, wanted, looseOpts)
if (!result || !result._shouldAvoid) {
return result
}

const caret = pickManifest(packument, `^${result.version}`, looseOpts)
if (!caret || !caret._shouldAvoid) {
return {
...caret,
_outsideDependencyRange: true,
_isSemVerMajor: false
}
}

const star = pickManifest(packument, '*', looseOpts)
if (!star || !star._shouldAvoid) {
return {
...star,
_outsideDependencyRange: true,
_isSemVerMajor: true
}
}

throw Object.assign(new Error(`No avoidable versions for ${name}`), {
code: 'ETARGET',
name,
wanted,
avoid,
before,
versions: Object.keys(versions)
})
}

const staged = (includeStaged && packument.stagedVersions &&
packument.stagedVersions.versions) || {}
const restricted = (packument.policyRestrictions &&
Expand All @@ -50,7 +100,7 @@ const pickManifest = (packument, wanted, opts) => {
// we use that. Otherwise, we get the highest precedence version
// prior to the dist-tag.
if (isBefore(verTimes, ver, time)) {
return versions[ver] || staged[ver] || restricted[ver]
return decorateAvoid(versions[ver] || staged[ver] || restricted[ver], avoid)
} else {
return pickManifest(packument, `<=${ver}`, opts)
}
Expand All @@ -60,15 +110,19 @@ const pickManifest = (packument, wanted, opts) => {
if (wanted && type === 'version') {
const ver = semver.clean(wanted, { loose: true })
const mani = versions[ver] || staged[ver] || restricted[ver]
return isBefore(verTimes, ver, time) ? mani : null
return isBefore(verTimes, ver, time) ? decorateAvoid(mani, avoid) : null
}

// ok, sort based on our heuristics, and pick the best fit
const range = type === 'range' ? wanted : '*'

// if the range is *, then we prefer the 'latest' if available
// but skip this if it should be avoided, in that case we have
// to try a little harder.
const defaultVer = distTags[defaultTag]
if (defaultVer && (range === '*' || semver.satisfies(defaultVer, range, { loose: true }))) {
if (defaultVer &&
(range === '*' || semver.satisfies(defaultVer, range, { loose: true })) &&
!shouldAvoid(defaultVer, avoid)) {
const mani = versions[defaultVer]
if (mani && isBefore(verTimes, defaultVer, time)) {
return mani
Expand All @@ -82,27 +136,24 @@ const pickManifest = (packument, wanted, opts) => {
.filter(([ver, mani]) => isBefore(verTimes, ver, time))

if (!allEntries.length) {
throw Object.assign(new Error(`No valid versions available for ${name}`), {
throw Object.assign(new Error(`No versions available for ${name}`), {
code: 'ENOVERSIONS',
name,
type,
wanted,
before,
versions: Object.keys(versions)
})
}

const avoidSemverOpt = { includePrerelease: true, loose: true }
const shouldAvoid = ver =>
avoid && semver.satisfies(ver, avoid, avoidSemverOpt)

const sortSemverOpt = { loose: true }
const entries = allEntries.filter(([ver, mani]) =>
semver.satisfies(ver, range, { loose: true }))
.sort((a, b) => {
const [vera, mania] = a
const [verb, manib] = b
const notavoida = !shouldAvoid(vera)
const notavoidb = !shouldAvoid(verb)
const notavoida = !shouldAvoid(vera, avoid)
const notavoidb = !shouldAvoid(verb, avoid)
const notrestra = !restricted[a]
const notrestrb = !restricted[b]
const notstagea = !staged[a]
Expand All @@ -128,7 +179,7 @@ const pickManifest = (packument, wanted, opts) => {
semver.rcompare(vera, verb, sortSemverOpt)
})

return entries[0] && entries[0][1]
return decorateAvoid(entries[0] && entries[0][1], avoid)
}

module.exports = (packument, wanted, opts = {}) => {
Expand Down
63 changes: 62 additions & 1 deletion test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,10 @@ test('support selecting staged versions if allowed by options', t => {

test('support excluding avoided version ranges', t => {
const metadata = {
name: 'vulny',
'dist-tags': {
latest: '1.0.3'
},
versions: {
'1.0.0': { version: '1.0.0' },
'1.0.1': { version: '1.0.1' },
Expand All @@ -492,6 +496,63 @@ test('support excluding avoided version ranges', t => {
const cannotAvoid = pickManifest(metadata, '^1.0.0', {
avoid: '1.x'
})
t.equal(cannotAvoid.version, '1.0.3', 'could not avoid within semver range')
t.match(cannotAvoid, {
version: '1.0.3',
_shouldAvoid: true
}, 'could not avoid within SemVer range')
t.end()
})

test('support excluding avoided version ranges strictly', t => {
const metadata = {
name: 'vulny',
'dist-tags': {
latest: '1.0.3'
},
versions: {
'1.0.0': { version: '1.0.0' },
'1.0.1': { version: '1.0.1' },
'1.0.2': { version: '1.0.2' },
'1.0.3': { version: '1.0.3' },
'2.0.0': { version: '2.0.0' }
}
}
const manifest = pickManifest(metadata, '^1.0.2', {
avoid: '1.x >1.0.2',
avoidStrict: true
})
t.match(manifest, {
version: '1.0.2'
}, 'picked the right manifest using ^')

const breakRange = pickManifest(metadata, '1.0.2', {
avoid: '1.x <1.0.3',
avoidStrict: true
})
t.match(breakRange, {
version: '1.0.3',
_outsideDependencyRange: true,
_isSemVerMajor: false
}, 'broke dep range, but not SemVer major')

const majorBreak = pickManifest(metadata, '1.0.2', {
avoid: '1.x',
avoidStrict: true
})
t.match(majorBreak, {
version: '2.0.0',
_outsideDependencyRange: true,
_isSemVerMajor: true
}, 'broke dep range with SemVer-major change')

t.throws(() => pickManifest(metadata, '^1.0.0', {
avoid: '<3.0.0',
avoidStrict: true
}), {
code: 'ETARGET',
message: 'No avoidable versions for vulny',
avoid: '<3.0.0'
})

t.end()
})

2 comments on commit c268796

@isaacs
Copy link
Author

@isaacs isaacs commented on c268796 Apr 8, 2020

Choose a reason for hiding this comment

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

Commit message is wrong, this was actually reviewed by @darcyclarke

@darcyclarke
Copy link

Choose a reason for hiding this comment

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

@isaacs 🤷‍♂️ no harm no foul

Please sign in to comment.