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

Throw errors on wrong feature and region names from Can I Use #714

Merged
merged 2 commits into from Jul 27, 2022
Merged

Throw errors on wrong feature and region names from Can I Use #714

merged 2 commits into from Jul 27, 2022

Conversation

sashachabin
Copy link
Contributor

@sashachabin sashachabin commented Jul 27, 2022

Problem

Recently I made mistakes in queries that use caniuse-lite data. It's regional usage statistics query >0% in [Region] and feature support query supports [feature-name] .

Browserslist crashes and doesn't output errors to the user. It seems that the Browserslist, as in other cases, should indicate user errors (especially if the request consists of several parts).

Other discussions about one of these problems: #684, PSPDFKit-labs/browserslist.dev#39

Bad requests examples

Supports

'supports replace-all' — example from Issue #684

Error message
➜  ~ npx browserslist 'supports replace-all'
/usr/local/lib/node_modules/browserslist/cli.js:96
      throw e
      ^

Error: Cannot find module 'caniuse-lite/data/features/replace-all.js'

Region

'>2% in __' — 2 wrong symbols

Error message
➜  ~ npx browserslist '>2% in __' 
/usr/local/lib/node_modules/browserslist/cli.js:96
      throw e
      ^

Error: Cannot find module 'caniuse-lite/data/regions/__.js'

'>2% in alt-__' — 2 wrong symbols after alt-

Error message
➜  ~ npx browserslist '>2% in alt-__'
/usr/local/lib/node_modules/browserslist/cli.js:96
      throw e
      ^

Error: Cannot find module 'caniuse-lite/data/regions/alt-__.js'

My suggestions

  • Wrap dynamic require('caniuse-lite/**/*.js') with try { ... } catch { throw new BrowserslistError('...') } with messages:
    • Unknown feature name ${name}
    • Unknown region name ${name} — for countries and continents
  • Add tests

Result

➜  ~ npx browserslist '>0% in LL'
browserslist: Unknown region name `LL`.

@ai
Copy link
Member

ai commented Jul 27, 2022

Looks very good

@ai ai merged commit aea0b74 into browserslist:main Jul 27, 2022
@ai
Copy link
Member

ai commented Jul 27, 2022

Thanks. Released in 4.21.3.

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.

None yet

2 participants