eslint-plugin-unicorn
eslint-plugin-unicorn copied to clipboard
sindresorhus
`func-style` like rule
See: http://eslint.org/docs/rules/func-style
My personal preference is to prefer declaration style, as it allows hoisting (and just looks better to me). The only issue is when you want to reassign:
var func = function () {
}
if (someCondition) {
func = wrap(func);
}
ESLint's func-style rule does not accommodate the above exception when preferring declarations, so it is impractical to enforce.
Note:
You can get around ELint's rule by doing this:
var func; // don't initialize variable here, do it on the next line
func = function () {
}
But that feels impractical.
Do we feel it's worth enforcing func-style in XO? Should we use the builtin plugin to do so, or make our own that is a bit more permissive.
When writing in ES5, I prefer the declaration style, when writing with ES2015, I think I preferred (has been a while now) writing expression style for short functions (const double = (x) => x * 2;), and declaration style for longer ones. With the expression one, you have the benefit that you know that the function won't be re-declared elsewhere if you've assigned it to a const variable (and I don't like hoisting btw).
So I'm rather against enforcing it.
Arrow functions would be excluded (that's an option even in ESLints plugin).
With arrow function excluded... then maybe yeah. We'll still have to mix decorated stuff like this anyway:
var fn = _.curry(function (...) { ... });
But yeah, why not.
The eslint rule (in declaration mode, with arrow functions excluded), only flags a FunctionExpression if it's immediate parent is a VariableDeclarator, so you only run afoul doing var fn = function () {}.
The ESLint behavior makes sense to me, but it doesn't make an exception for variables that are later reassigned. That is a rare occurrence, but if you are already in the habit of using function declarations, then var fn = function () {} is a telegraphs the potential reassignment to the reader.
Jumping back to arrow functions for a second:
I think they should only be allowed for simple functions with implicit returns
// OK
var square = x => x * x;
// NOT OK
var complexFunc = (x, y) => {
if (x) {
return y * 2;
} else {
return y * 3;
}
}
// I would prefer
function complexFunc(x, y) {
if (x) {
return y * 2;
} else {
return y * 3;
}
}
Exceptions for arrow funcs should be made if they use any of this, arguments, super or new.target.
I think they should only be allowed for simple functions with implicit returns
Why? I prefer arrow functions for everything as they're lexically scoped (no dumb implicit this) and you can use const with them to ensure they're never overridden.
Well, if you don't need the lexical binding there's no advantage. Actually there may be a performance hit if arrow functions are transpiled (to accommodate the binding, etc) - though that might not even true - not sure if Babel is smart enough to avoid binding if it isn't needed.
Still, I am not dogmatic about it.
This came up because I noticed we're mixing styles across the AVA codebase, and was wondering if there's a good set of rules we can use to describe our preferred style (whatever that is).
Yes, I like to figure out a good default we can enforce for this too. I don't feel super strongly either way, just slightly prefer arrow functions. Also nested function declarations looks messy and having everything as variables looks cleaner and more consistent.
// @vdemedes @novemberborn @SamVerschueren
if you don't need the lexical binding there's no advantage.
Still the advantage of const and no hoisting. I consider hoisting a misfeature. I prefer explicitness.
Actually there may be a performance hit if arrow functions are transpiled
Node.js 4 supports arrow functions though. So I don't see it as a big problem going forward.
I tend to use a mix of syntaxes. E.g. in package-hash, because I'm using export function sync I like declaration style for the other functions in the file.
In other files where there are few declarations I'm more likely to use const with an arrow function, but I'm far from consistent about this.
If I'd have to choose I'd prefer const (and in rare occasions let) with arrow functions. Never var. But not for exports… I'm not sure there's a single consistent answer here.
I agree that ES2015 exports should use a normal function.
This:
export function foo() {}
Looks better than this:
export const foo = () => {}
Related to this (old-ish) issue, I've recently implemented and submitted related PR to eslint, not realizing it had no chance of being accepted: eslint#16159 .
I too prefer the declaration style, with the exception where reassignment is desired, where fat arrow functions look better to me. The current func-style rule in eslint does not allow the exception, my PR does. Shall I repurpose the PR for plugin-unicorn?
