Allowing no name for class property functions?

natehardy commented 5 years ago

At the moment, the plugin is not allowing class property functions. Is this intended? Would it be useful to have an option to allow this inside the no-name rule?

getify commented 5 years ago

Can you give me a code example?

getify commented 5 years ago

FWIW, I used this code example (hoping this is what you meant):

class Foo {
   fn = x => this.x = x;

   constructor() {
      console.log(this.fn.name);
   }
}

var f = new Foo();
// "fn"

IOW, class properties (of function expressions) should have name inference, so I just added that handling. No need for extra options, should just automatically work now. Released as v7.0.0.

Thanks for catching it. Technically class-properties are still just stage3 so not yet official in JS, but they're pretty common, and pretty certain to land, so... worth supporting. :)

natehardy commented 5 years ago

Sorry I didn't send an example. I Didn't get notified of the message. I updated the plugin and it is still throwing errors in my react classes. I thought it might be related to React so I pasted your code example from above into my code and also got an error for no name. The console is also not printing "fn". Is there something I'm missing about the name property on classes?

getify commented 5 years ago

I need to see a code example or I can't just guess at what code is causing you problems?

But if you took my above code and ran it with the current version of the plugin, and it complained about the name missing, then there's definitely something wrong/strange with your setup.

This is running that code in Chrome console:

As you can see, the name inference works fine.

Perhaps you're running some sort of setup that is converting the code to something not actually using class before it's handing the code to eslint?

natehardy commented 5 years ago

Thanks so much for responding. I know you're very busy so ignore if you don't have time but the following are the relevant pieces:

Using "@getify/eslint-plugin-proper-arrows": "7.1.0"

webpack config

Console output in chrome from calling the TestClass function:

Screenshot from intellij (could be a intellij anamoly) noname

If you have time, would love yo know your thoughts. Thanks, Nate

getify commented 5 years ago

Your code works fine in my eslint setup, using the "babel-eslint" parser -- which is necessary because the feature class-properties is stage3 (not official JS) and thus doesn't work in the standard eslint parser.

You'll notice there, I have the same arrow function expression used two different kinds of ways, once as a call-argument (no name inference possible) and once as a class property. My linter only gives me the name-inference linting error for the former, not the latter.

I only have two theories to explain why you're seeing this, but they're not great theories.

You think you're using the updated "proper-arrows" plugin, but somehow your setup is using the older/cached version. I don't know how to verify this, per se, but it would explain why you're still getting the error.

However, that doesn't explain why, in your setup, the name inference itself actually fails, and you get an empty printout for name. Chrome is doing the correct thing, which is that there should be a name inference there. So we know your setup is somehow doing less than the correct/accurate thing with the JS you're writing.
Something about your setup is using a different kind of parser/transpilation which is giving eslint a different version of your code than what's actually displayed on your screen. I didn't see anything obvious about that in your webpack config, but I'm not very knowledgable about webpack so I don't know what most of it means, TBH.

What I do know is, the transpiled version of that class property setting, via Babel (with their stage3 + spec setting, as well as the class properties transform plugin), looks basically like this:

 _defineProperty(_assertThisInitialized(_this), "myFunc", function (abc, def) {
   _newArrowCheck(this, _this2);
   return _this.whatever(abc, def);
 }.bind(this));

Notice the anonymous function on that first line? Babel appears to not be trying to do anything to preserve the name inferencing that is supposed to occur. They should be doing function myFunc(abc,def) { .. to preserve the name.

That's strange that they're not doing it, because they do handle the name inferencing for code like this:

var fn = () => { console.log(fn.name); };

Babel turns that code into:

var _fn = function fn() {
  _newArrowCheck(this, _this);
  console.log(_fn.name);
}.bind(void 0);

Notice there, they used function fn() { ...

However, in both cases, they're overriding the name inferencing anyway, by using .bind(..), which ends up making the name print out "bound .." instead. So it's all moot. That's pretty annoying, but I guess babel doesn't really care that much about spec-correct behavior.

Boiling all this evidence/obvservation down, I still don't have an explanation, because neither theory really fully explains what you're seeing.

natehardy commented 5 years ago

So much for spending the time to look into this. I know how busy you are. If I figure out what's going on, I'll let you know. As an aside, your JS workshops have really deepened my working knowledge and understanding of JS and I really appreciate all the clarity you bring to explaining how to become a better coder. Thanks!

natehardy commented 5 years ago

I deleted the node-modules directory and re-installed all the dependencies and the error is no longer appearing. It must have been some conflict with the previous version not being removed properly. Thanks again for all your help.

getify commented 5 years ago

Excellent, glad to hear that! :)

natehardy commented 5 years ago

Interesting side note - the console is still logging a blank name for the function name so it must be a transform that webpack is doing or a plugin I have in webpack. Thanks again.

getify / eslint-plugin-proper-arrows

Allowing no name for class property functions? #15