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 3 commits
Commits
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
3 changes: 2 additions & 1 deletion bin/semver.js
Expand Up @@ -24,6 +24,7 @@ let rtl = false
let identifier

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

let reverse = false

Expand Down Expand Up @@ -88,7 +89,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
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
3 changes: 1 addition & 2 deletions functions/parse.js
Expand Up @@ -4,8 +4,6 @@ const SemVer = require('../classes/semver')

const parseOptions = require('../internal/parse-options')
const parse = (version, options) => {
options = parseOptions(options)

if (version instanceof SemVer) {
return version
}
Expand All @@ -18,6 +16,7 @@ const parse = (version, options) => {
return null
}

options = parseOptions(options)
const r = options.loose ? re[t.LOOSE] : re[t.FULL]
if (!r.test(version)) {
return null
Expand Down
24 changes: 14 additions & 10 deletions internal/parse-options.js
@@ -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
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