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

no-es6-methods a bit too strict #12

Open
refractalize opened this issue Feb 28, 2018 · 9 comments
Open

no-es6-methods a bit too strict #12

refractalize opened this issue Feb 28, 2018 · 9 comments

Comments

@refractalize
Copy link

an innocent file like this:

var x
x.values()

Will fail eslint with this error:

ES6 methods not allowed: values  es5/no-es6-methods

This is my .eslintrc file:

{
  "extends": [
    "plugin:es5/no-es2015"
  ]
}

In short, I think there's no easy way to tell if x (in the example above) is an array or something that really does have .values() - so I propose that we drop these rules... WDYT?

@bzmw
Copy link

bzmw commented Mar 6, 2018

I am having the same problem with .find

The following will fail even though we are using the jQuery API and not Array.find.

var jqueryObject = $('.example').find('.child-element');

https://api.jquery.com/find/

I ended up just setting "es5/no-es6-methods": "warn",

@BearAlliance
Copy link

BearAlliance commented Apr 12, 2018

I'm seeing the same thing for chained lodash methods

eg.

_.chain(
  .get(arr)
  .find('prop')
)

These are false positives.

@macklinu
Copy link
Contributor

This seems like a very difficult thing to detect with ESLint and seems like a type checker like Flow or TypeScript, since it would be hard to analyze the variable to scope to know if you're calling .find() on a jQuery element or an array without the help of a type system.

I tried to introduce this rule into a large codebase using jQuery and had to disable it. I'm wondering if there should be an option to ignore certain function names (like .find()), or maybe that's overkill? Thoughts?

@theprivileges
Copy link

I'm also experiencing the same issue, and ended up using the same solution that @bzmw recommended.

@derwaldgeist
Copy link

Same problem here with jQuery's find method on forms:

var form = jQuery('#myForm');
var val = form.find('#myfield').val();

@Amorymeltzer
Copy link

Same as above.

One thing I started doing, which is far from perfect, is running something like grep '\.find(' *.js|grep -v '\$', which at least let's me find potential cases of non-jquery .find after using the plugin. Most are likely to be variables (i.e., x.find(y) where previously var x = $(y)) but it's a quick sanity check.

@simonwiles
Copy link

While there's no simple and complete solution to this, in the case that brought me here, the ability to exempt specific methods would suffice, and would make a good addition to this plugin.

@Amorymeltzer
Copy link

Amorymeltzer commented Jun 12, 2020

#36 (and thus v1.5.0) made this much better, especially if naming jQuery variables with $.

Amorymeltzer added a commit to Amorymeltzer/twinkle that referenced this issue Jun 13, 2020
I stumbled on https://github.com/nkt/eslint-plugin-es5 about a year ago but it had too many false-positives when used with jQuery.  Most of that (nkt/eslint-plugin-es5#12) appears to have been resolved with v1.5.0, specifically nkt/eslint-plugin-es5#36.  This won't catch everything but I *think* it shouldn't have many false positives, as long as things like jQuery's `.find` are used on the same line as `$`, whether `jQuery` or as a variable name; we're inconsistent on that front, but it's not a bad idea to encourage it.
Amorymeltzer added a commit to Amorymeltzer/twinkle that referenced this issue Jun 13, 2020
I stumbled on https://github.com/nkt/eslint-plugin-es5 about a year ago but it had too many false-positives when used with jQuery.  Most of that (nkt/eslint-plugin-es5#12) appears to have been resolved with v1.5.0, specifically nkt/eslint-plugin-es5#36.  This won't catch everything but I *think* it shouldn't have many false positives, as long as things like jQuery's `.find` are used on the same line as `$`, whether `jQuery` or as a variable name; we're inconsistent on that front, but it's not a bad idea to encourage it.
Amorymeltzer added a commit to Amorymeltzer/twinkle that referenced this issue Jun 23, 2020
I stumbled on https://github.com/nkt/eslint-plugin-es5 about a year ago but it had too many false-positives when used with jQuery.  Most of that (nkt/eslint-plugin-es5#12) appears to have been resolved with v1.5.0, specifically nkt/eslint-plugin-es5#36.  This won't catch everything but I *think* it shouldn't have many false positives, as long as things like jQuery's `.find` are used on the same line as `$`, whether `jQuery` or as a variable name; we're inconsistent on that front, but it's not a bad idea to encourage it.
@skyway777
Copy link

skyway777 commented Jun 26, 2020

#39 adds an ability to skip methods that still don't work correctly. For example, in my project, there are a lot of jQuery variables with names without a $ sign and it could take a long time to fix and check thousands of lines (the previous PR doesn't fix it).
And, I've noticed that some es6 methods aren't banned.
I've added also the ability to add extra methods listed in MDN (Array methods, String methods ) to be highlighted by this plugin.

wiki-ST47 pushed a commit to wiki-ST47/twinkle that referenced this issue Sep 2, 2020
I stumbled on https://github.com/nkt/eslint-plugin-es5 about a year ago but it had too many false-positives when used with jQuery.  Most of that (nkt/eslint-plugin-es5#12) appears to have been resolved with v1.5.0, specifically nkt/eslint-plugin-es5#36.  This won't catch everything but I *think* it shouldn't have many false positives, as long as things like jQuery's `.find` are used on the same line as `$`, whether `jQuery` or as a variable name; we're inconsistent on that front, but it's not a bad idea to encourage it.
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

No branches or pull requests

9 participants