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

New matchesUA return signature #34

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jaydenseric
Copy link

@jaydenseric jaydenseric commented Dec 10, 2018

matchesUA() now returns an object with matches, a boolean indicating that the browserslist config matched the UA, and userAgent, the resolved user agent family and version. For unrecognizable user agents the userAgent is null.

This is a breaking change.

This allows consumers to tell apart if the user agent didn't match the browserslist config because:

  1. The UA is recognized, but doesn't match the browserslist config.
  2. The UA was unrecognizable.

A use case is showing a browser outdated warning to users. At the moment, such a warning displays for Facebook Messenger on Android users, as the in-app browser has an "Orca" user agent that doesn't correspond to a browserslist family. In reality, their browser is not out of date, just unrecognized. Ideally, you could display browser out of date warnings only for users with recognized user agents that are known to be out of date.

It now returns an object with `matches`, indicating that the browserslist config matches the UA, and `userAgent`, the resolved user agent family and version. For unrecognizable user agents the `userAgent` is `null`.
@jaydenseric
Copy link
Author

This API change is needed to fix issues with a live site, so if it takes much more than a day or so to review/merge I'll consider publishing a new package.

Unfortunately this package is not setup properly for installation via git, so I can't install my PR in the meantime 😞

@jaydenseric
Copy link
Author

I've also updated the package scripts to allow this package to be installable via Git, e.g.

{
  "dependencies": {
    "browserslist-useragent": "jaydenseric/browserslist-useragent#handle-unrecognizable-ua"
  }
}

@jaydenseric
Copy link
Author

jaydenseric commented Jan 14, 2019

@pastelsky your thoughts? I've been using this PR as a Git dependency for over a month now in several projects, including https://enamor.app and https://whimsy.art. Looking forward to a review/merge.

@jaydenseric
Copy link
Author

@pastelsky it's been around 10 months — should I make my own package?

@pastelsky
Copy link
Collaborator

Hi @jaydenseric, sorry I've been so unresponsive. The Ocra case you mentioned previously has already been solved. I'm interested in knowing more use cases where useragent recognition would fail?

@jaydenseric
Copy link
Author

@pastelsky There are many, many exotic user agents in the wild. For example, Zeit Now takes screenshots of a deployment (using a decent version of Chromium) for display on the deployment dash. Because browserslist-useragent doesn't recognize its UA, the ugly browser unsupported warning is present in the screenshot:

Screen Shot 2019-10-21 at 7 58 39 pm

I've come across many other examples, which I can't recall off the top of my head. Frankly it is scary that users could try to access your app with a new UA that hasn't been added to browserslist-useragent yet (a process that takes some time) and get scared off by a browser unsupported warning.

This PR has re-caught my attention because I've had trouble using my fork as a git dependency with Now v2.

@pastelsky
Copy link
Collaborator

pastelsky commented Apr 25, 2020

@jaydenseric browserslist-useragent is not a substitute for feature detection. Using this library to show unsupported browser warnings is a bad idea.

This library was built with the purpose of being able to ship less code/polyfills to users on modern browsers while serving them similar functionality.

That said, I'll consider introducing this change for the next major version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants