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

feat(eslint): add compat eslint plugin #364

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ismay
Copy link
Contributor

@ismay ismay commented Mar 10, 2021

This adds https://www.npmjs.com/package/eslint-plugin-compat. This plugin checks the js APIs you're using against your browserslist. It'll warn you when you're using js APIs that aren't supported by your target audience.

As long as we're sticking to stable browser features, and not polyfilling with babel to features that haven't shipped with browsers yet, this is a useful plugin to have (and even if we do it can be useful, just means that we need to define the polyfills we ship).

Copy link
Contributor

@HendrikThePendric HendrikThePendric left a comment

Choose a reason for hiding this comment

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

Seems like another good addition 👍

@ismay
Copy link
Contributor Author

ismay commented Mar 10, 2021

I've changed the approach here slightly. The recommended preset of this plugin sets all violations to errors, and since our browserslist includes opera mini, most our codebases will fail linting as that browser doesn't support promises:

Screenshot 2021-03-10 at 11 45 49

We could remove opera mini from our browserslist if we don't support it, and otherwise we should ship a promise polyfill. Until then, I'll leave this rule on warn. Once we've decided what we want to do about the above, we could move the rule to error (and potentially configure the plugin so it knows we're shipping a promise polyfill).

Copy link
Member

@amcgee amcgee left a comment

Choose a reason for hiding this comment

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

Nice to have confidence in browser compatibility!

We should probably exclude Opera Mini, otherwise we will definitely need the polyfill

@stale
Copy link

stale bot commented Mar 2, 2022

Hi!

Due to a lack of activity on this issue over time (180 days) it seems to be stale. If still relevant, please provide information that moves it forward, e.g. additional information, a pull request with suggested changes, or a reason to keep it open.

Any activity will keep it open, otherwise it will be closed automatically in 30 days. Thanks! 🤖

@stale stale bot added the stale label Mar 2, 2022
@stale
Copy link

stale bot commented Apr 16, 2022

Automatically closing due to lack of activity. 🤖

@stale stale bot closed this Apr 16, 2022
@ismay ismay reopened this Oct 11, 2022
@ismay ismay removed the request for review from varl October 11, 2022 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants