Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: faster parse options #535

Merged
merged 4 commits into from
Apr 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion bin/semver.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ let identifier
let identifierBase

const semver = require('../')
const parseOptions = require('../internal/parse-options')

let reverse = false

Expand Down Expand Up @@ -93,7 +94,7 @@ const main = () => {
}
}

options = { loose: loose, includePrerelease: includePrerelease, rtl: rtl }
options = parseOptions({ loose, includePrerelease, rtl })
wraithgar marked this conversation as resolved.
Show resolved Hide resolved

versions = versions.map((v) => {
return coerce ? (semver.coerce(v, options) || { version: v }).version : v
Expand Down
7 changes: 0 additions & 7 deletions classes/comparator.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,6 @@ class Comparator {
throw new TypeError('a Comparator is required')
}

if (!options || typeof options !== 'object') {
options = {
loose: !!options,
includePrerelease: false,
}
}

if (this.operator === '') {
if (this.value === '') {
return true
Expand Down
24 changes: 14 additions & 10 deletions internal/parse-options.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
// parse out just the options we care about so we always get a consistent
// obj with keys in a consistent order.
const opts = ['includePrerelease', 'loose', 'rtl']
const parseOptions = options =>
!options ? {}
: typeof options !== 'object' ? { loose: true }
: opts.filter(k => options[k]).reduce((o, k) => {
o[k] = true
return o
}, {})
// parse out just the options we care about
const looseOption = Object.freeze({ loose: true })
const emptyOpts = Object.freeze({ })
const parseOptions = options => {
if (!options) {
return emptyOpts
}

if (typeof options !== 'object') {
return looseOption
}

return options
}
module.exports = parseOptions
20 changes: 16 additions & 4 deletions test/internal/parse-options.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,24 @@ t.test('truthy non-objects always loose mode, for backwards comp', t => {
t.end()
})

t.test('objects only include truthy flags we know about, set to true', t => {
t.strictSame(parseOptions(/asdf/), {})
t.strictSame(parseOptions(new Error('hello')), {})
t.strictSame(parseOptions({ loose: true, a: 1, rtl: false }), { loose: true })
t.test('any object passed is returned', t => {
t.strictSame(parseOptions(/asdf/), /asdf/)
t.strictSame(parseOptions(new Error('hello')), new Error('hello'))
t.strictSame(parseOptions({ loose: true, a: 1, rtl: false }), { loose: true, a: 1, rtl: false })
t.strictSame(parseOptions({ loose: 1, rtl: 2, includePrerelease: 10 }), {
loose: 1,
rtl: 2,
includePrerelease: 10,
})
t.strictSame(parseOptions({ loose: true }), { loose: true })
t.strictSame(parseOptions({ rtl: true }), { rtl: true })
t.strictSame(parseOptions({ includePrerelease: true }), { includePrerelease: true })
t.strictSame(parseOptions({ loose: true, rtl: true }), { loose: true, rtl: true })
t.strictSame(parseOptions({ loose: true, includePrerelease: true }), {
loose: true,
includePrerelease: true,
})
t.strictSame(parseOptions({ rtl: true, includePrerelease: true }), {
rtl: true,
includePrerelease: true,
})
Expand Down