Refactor all functions to use lambda (fat arrow) syntax

[lamchakchan]

Context

Refactor all anonymous function declarations to use () => {} syntax. Should help with readability of the overall code base. But let's not refactor any function declaration with name associations such as function namedFunction(). But if possible, replaced those with let namedFunction = () => {}

[WesTyler]

Anon functions as arrows, yes absolutely.

I'm not quite on board with named functions being arrows. But I'm not opposed on principle either. What's your thinking on that?

[lamchakchan]

We're agreeing on the named functions staying as such. But for anonymous functions assigned to variables such as.

existing let helloWorld = function () { return 'hello world' } new let helloWorld = () => { return 'hello world' }

[WesTyler]

Are you talking about using arrows in cases like this? https://github.com/xogroup/felicity/blob/master/lib/index.js#L12

Or like this? https://github.com/xogroup/felicity/blob/master/lib/index.js#L70

I'm not sure if there are any actual anonymous functions in the project that are not already using arrows.

And we are not using named function declarations anywhere per Hapi style guides function notLikeThis() {}

[lamchakchan]

Yes, that is a good example.

[WesTyler]

Gotcha. I'm totally okay with using arrows for the functions declared inside of other functions like this, but am not sure I like using arrows for main functions like this.

Is that a good enough compromise that you feel okay with this moving forward like that?

[devinivy]

In fact, in that latter case you can't use an arrow function since you need to access this.

[lamchakchan]

@WesTyler Why are you okay with one versus the other? Is it a stylistic opinion? Looking at the hapijs style guidelines and it looks to be completely acceptable. I would be more concerned with this line since it is returning a function to be instantiated and => syntax does not allow for that. But this might be a good choice to look at ES6 classes.

@devinivy this is accessible with => syntax. call and bind and apply though are not available so you can't set the this context directly. ES6 will lift this from the previous scope of the caller. Looks to work fine and I actually prefer it because it will allow for circumvention of using bind which is a performance hit. Check out my example here

[devinivy]

Right, this is always accessible, but it's not necessarily what you want. In the example above the function is a constructor, so you want this to refer to an instance of that constructor's class. Instead if you were to change it to an arrow function, this would refer to global, which wouldn't make for a very good constructor :P !

[lamchakchan]

@devinivy , I pointed out to @WesTyler that you can not use fat arrow syntax for newing up an instance. You will get JS errors.

[devinivy]

Right, and similar story for tacking methods onto a prototype

[WesTyler]

@lamchakchan It's mostly a stylistic opinion, mixed with guarding against arrow weirdness (like you guys already discussed). I know the Hapi style guide allows for using arrow functions just about everywhere, but just because you can doesn't always mean you should! I honestly would rather not use arrows anywhere except truly anonymous callbacks, but was trying to find some compromise to be a good team player, haha. :P

[WesTyler]

Your repl actually has a perfect example of why I am not a fan of using arrows everywhere you can:

var func1 = () => {
  (() => {
    console.log(this.foo);
  })()
};

that confuses the hell out of my eyes haha. The IIFE makes it more extreme, but still.

[lamchakchan]

With the sentiment exposed in this thread, I agree that the start usage of fat arrow should be narrow in scope and used within just the context of anonymous callbacks. Maybe when hapijs as a whole moves more towards ES6 adopted syntax, we can then revisit.