-
-
Notifications
You must be signed in to change notification settings - Fork 18
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
feat: allow all browserslist options via JS API #1489
feat: allow all browserslist options via JS API #1489
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!
src/useragentRegex/useragentRegex.ts
Outdated
const { | ||
browsers, | ||
env, | ||
path, | ||
...otherOptions | ||
} = options | ||
const finalOptions = { | ||
...defaultOptions, | ||
...otherOptions | ||
const regexpOptions: SemverCompareOptions = { | ||
...defaultOptions | ||
} | ||
const browserslistOptions: BrowserslistRequest = {} | ||
|
||
for (const optName of Object.keys(options) as (keyof UserAgentRegexOptions)[]) { | ||
if (optName in defaultOptions) { | ||
regexpOptions[optName] = options[optName] | ||
} else { | ||
browserslistOptions[optName] = options[optName] | ||
} | ||
} | ||
const browsersList = getBrowsersList({ | ||
browsers, | ||
env, | ||
path | ||
}) | ||
|
||
const browsersList = getBrowsersList(browserslistOptions) | ||
const mergedBrowsers = mergeBrowserVersions(browsersList) | ||
const sourceRegexes = getRegexesForBrowsers(mergedBrowsers, finalOptions) | ||
const versionedRegexes = applyVersionsToRegexes(sourceRegexes, finalOptions) | ||
const sourceRegexes = getRegexesForBrowsers(mergedBrowsers, regexpOptions) | ||
const versionedRegexes = applyVersionsToRegexes(sourceRegexes, regexpOptions) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make code more easer
export function getPreUserAgentRegexes(options: UserAgentRegexOptions = {}) {
const finalOptions = {
...defaultOptions,
...options
}
const browsersList = getBrowsersList(finalOptions)
const mergedBrowsers = mergeBrowserVersions(browsersList)
const sourceRegexes = getRegexesForBrowsers(mergedBrowsers, finalOptions)
const versionedRegexes = applyVersionsToRegexes(sourceRegexes, finalOptions)
return versionedRegexes
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I considered that as well. I didn't want to make the assumption that browserslist
wouldn't throw on unknown options. If you're okay with that, I'll make the change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x1AFc752808fe62BBC62Eaf30105e34b6582e05ba
Fixes #1484 by allowing the JS API to pass any non-regex option to
browserslist
. I implemented this in a way that is both backwards compatible and doesn't require knowledge of thebrowserslist
options.This doesn't implement for the CLI as the current
env
andpath
options aren't possible there either and doing so requires listing out all thebrowserslist
options.