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

Replace ua-parser-js with a reduced in-house version #3720

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
127 changes: 122 additions & 5 deletions lib/helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,132 @@
const fs = require('graceful-fs')
const path = require('path')
const _ = require('lodash')
const useragent = require('ua-parser-js')
const mm = require('minimatch')

const extractUAParts = (ua) => {
Copy link
Member

Choose a reason for hiding this comment

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

Can we add some unit tests for this? @davidje13

const result = {}
if (ua) {
// general format is: Thing1/Version.1 Thing 2/Version.2 (detailA; detailB)
const partPattern = /(\w[\w_ ]*)\/([^ ]+)(?: \(([^)]+)\) *| (?!\()|$)/g

for (const [, name, version, bracketed] of ua.matchAll(partPattern)) {
const details = (bracketed || '').split(/; */g).filter((x) => x)
const key = name.toLowerCase().replace(/ /g, '_')
if (!result[key]) { // use first occurrence, and prevent __proto__ changes, etc.
result[key] = {
version,
details,
hasDetail: (test) => details.some((v) => test.exec(v)),
getDetail: (test, mapper) => {
const found = details.map((v) => test.exec(v)).filter((x) => x)[0]
if (mapper) {
return found ? mapper(found) : null
}
return found || []
}
}
// lots of checks look at the details of the first entry, which is
// usually 'Mozilla', but not always (e.g. 'Opera'),
// so record the first entry separately:
if (!result.firstEntry) {
result.firstEntry = result[key]
}
}
}
}
if (!result.firstEntry) {
// ensure firstEntry is always set no matter what (null object pattern)
result.firstEntry = {
version: null,
details: [],
hasDetail: () => false,
getDetail: () => []
}
}
return result
}

const extractFromParts = (parts, checks) => checks
.map((check) => check(parts))
.filter((result) => result)[0] || []

const extractMacVersion = (m) => m[1].replace(/_/g, '.')

const WINDOWS_NT_VERSION_MAP = {
5.1: 'XP',
5.2: 'XP',
'6.0': 'Vista',
6.1: '7',
6.2: '8',
6.3: '8.1',
6.4: '10',
'10.0': '10'
}
const extractWindowsVersion = (m) => WINDOWS_NT_VERSION_MAP[m[1]]

const UA_BROWSERS = [
({ phantomjs }) => phantomjs && // also contains Safari
['PhantomJS', phantomjs.version],

({ headlesschrome }) => headlesschrome && // also contains Safari
['Chrome Headless', headlesschrome.version],

({ opera, version }) => opera &&
['Opera', version && version.version],

({ firefox }) => firefox &&
['Firefox', firefox.version],

({ edg }) => edg && // also contains Chrome, Safari
['Edge', edg.version],

({ chrome }) => chrome && // also contains Safari
['Chrome', chrome.version],

({ firstEntry, version }) => firstEntry.hasDetail(/^iphone/i) && // also contains Safari
['Mobile Safari', version && version.version],

({ firstEntry, version }) => firstEntry.hasDetail(/^android/i) && // also contains Safari
['Android Browser', version && version.version],

({ safari, version }) => safari &&
['Safari', version && version.version],

({ firstEntry }) => firstEntry.hasDetail(/^msie/i) &&
['IE', firstEntry.getDetail(/^msie ([\d.]+)/i)[1]]
]

const UA_SYSTEMS = [
({ firstEntry }) => firstEntry.hasDetail(/^android/i) &&
['Android', firstEntry.getDetail(/^android ([\d.]+)/i)[1]],

({ firstEntry }) => firstEntry.hasDetail(/^iphone/i) &&
['iOS', firstEntry.getDetail(/iphone os ([\d._]+)/i, extractMacVersion)],

({ ubuntu }) => ubuntu &&
['Ubuntu', ubuntu.version],

({ firstEntry }) => firstEntry.hasDetail(/^freebsd/i) &&
['FreeBSD', null],

({ firstEntry }) => firstEntry.hasDetail(/^linux/i) &&
['Linux', firstEntry.getDetail(/^linux (.+)/i)[1]],

({ firstEntry }) => firstEntry.hasDetail(/mac os/i) &&
['Mac OS', firstEntry.getDetail(/mac os(?: x)? ([\d._]+)/i, extractMacVersion)],

({ firstEntry }) => firstEntry.hasDetail(/^windows/i) &&
['Windows', firstEntry.getDetail(/windows nt ([\d.]+)/i, extractWindowsVersion)]
]

Comment on lines +57 to +123
Copy link
Member

Choose a reason for hiding this comment

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

Where did you get all this info? Probably from the implementation of ua-parser-js? Can we add a reference?

Copy link
Member

Choose a reason for hiding this comment

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

Also, can you please add comments to the functions?

Copy link
Author

Choose a reason for hiding this comment

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

source of this is actually the test user-agent strings which were already in this project. The code and regular expressions here have no relation to ua-parser-js's code.

Copy link
Author

Choose a reason for hiding this comment

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

As for documentation: if the team is happy with this approach, I can extract the code into its own file, add finer-grained tests, and document. Wanted to get a quick first pass submitted without all that so that I didn't waste too much time if the team was opposed to the whole idea.

Copy link
Member

Choose a reason for hiding this comment

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

I am happy with your approach. It is very elegant! :)
I'd like to hear from @devoto13 as well.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Approach overall LGTM!
I'll try to look more into details in the coming days.

exports.browserFullNameToShort = (fullName) => {
const ua = useragent(fullName)
if (!ua.browser.name && !ua.browser.version && !ua.os.name && !ua.os.version) {
return fullName
const parts = extractUAParts(fullName)
const [browserName, browserVersion] = extractFromParts(parts, UA_BROWSERS)
const [osName, osVersion] = extractFromParts(parts, UA_SYSTEMS)
if (browserName || osName) {
return `${browserName || 'unknown'} ${browserVersion || '0.0.0'} (${osName || 'unknown'} ${osVersion || '0.0.0'})`
}
return `${ua.browser.name} ${ua.browser.version || '0.0.0'} (${ua.os.name} ${ua.os.version || '0.0.0'})`
return fullName || 'unknown'
}
Comment on lines +8 to 132
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should move this logic to its own file. @devoto13 WDYT?


exports.isDefined = (value) => {
Expand Down
6 changes: 0 additions & 6 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,6 @@
"socket.io": "^4.2.0",
"source-map": "^0.6.1",
"tmp": "^0.2.1",
"ua-parser-js": "^0.7.30",
"yargs": "^16.1.1"
},
"devDependencies": {
Expand Down