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

Use the es plugin rather than our own modelling of JS versions? #224

Open
jdforrester opened this issue Mar 31, 2020 · 4 comments
Open

Use the es plugin rather than our own modelling of JS versions? #224

jdforrester opened this issue Mar 31, 2020 · 4 comments

Comments

@jdforrester
Copy link
Member

Credit to @catrope for finding it. https://www.npmjs.com/package/eslint-plugin-es

@edg2s
Copy link
Member

edg2s commented Mar 31, 2020

Here's my findings so far:

  • For all the static methods (Object.values, Math.cosh etc) it's an improvement. Each rule has it's own name so we don't have to worry about all the horrible hacking we are doing with merge.js
  • It currently doesn't have any prototype methods, due to concerns over false positives. I've filed a task to at least include these as optional rules: Create optional rules for prototype methods mysticatea/eslint-plugin-es#24
  • All the other rules seem unnecessary if you have ecmaVersion set in ESLint as they only catch on things that already cause parser errors, e.g. es/no-arrow-function will never fire because you'll get an unexpected token error first. If we use this plugin we'll have to manually disable all these rules.

@catrope
Copy link
Contributor

catrope commented Mar 31, 2020

  • All the other rules seem unnecessary if you have ecmaVersion set in ESLint as they only catch on things that already cause parser errors, e.g. es/no-arrow-function will never fire because you'll get an unexpected token error first. If we use this plugin we'll have to manually disable all these rules.

For my particular use case, this was a feature not a bug. eslint-plugin-vue crashes if ecmaVersion is set to 5, so I needed a way to prohibit ES6 features from being used while still setting ecmaVersion to 6.

I only did this for .vue files though, because I also found that it's much harder to disable these prohibitions again for code that is supposed to be ES6.

@edg2s
Copy link
Member

edg2s commented Mar 31, 2020

this was a feature

Well it's a hack for a bug in another plugin. Also that sounds like a pretty bad bug, is it reported?

@catrope
Copy link
Contributor

catrope commented Mar 31, 2020

Fair enough :) . I forgot to report the bug upstream yesterday, thanks for reminding me. vuejs/eslint-plugin-vue#1089

@jdforrester jdforrester changed the title Use the es plugin rather than our own modelling of JS versions Use the es plugin rather than our own modelling of JS versions? Apr 15, 2020
edg2s added a commit that referenced this issue Apr 27, 2020
Don't bother with syntax errors as these are caught
by the parser, but having individual rules for builtin
methods makes them easier to selectively disable.

Part of #224
edg2s added a commit that referenced this issue Apr 27, 2020
Don't bother with syntax errors as these are caught
by the parser, but having individual rules for builtin
methods makes them easier to selectively disable.

Also ensure Array.prototype.entries rule doesn't conflict
with Object.entries.

Part of #224
jdforrester pushed a commit that referenced this issue Apr 27, 2020
Don't bother with syntax errors as these are caught
by the parser, but having individual rules for builtin
methods makes them easier to selectively disable.

Also ensure Array.prototype.entries rule doesn't conflict
with Object.entries.

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

No branches or pull requests

3 participants