airbnb/javascript

7.1 says function expressions are always unnamed, but this changed in ES6

Arnavion opened this issue Β· 44 comments

7.1 contains this statement:

Function declarations are named, so they're easier to identify in call stacks.

Anonymous function expressions assigned to a binding are also named, per ES6 semantics (1.e.iii). That is, the "bad" example const foo = function () { }; is the same as const foo = function foo() { };, which is equivalent to the "good" example function foo() { } in that respect.

Should the statement be qualified that it's only a distinction for ES5 and below ?

Function name inference is implemented in hardly any of the browsers we support. In addition, relying on function name inference hinders greppability, so even in a pure ES6 environment, we wouldn't recommend that.

(other subtler differences are that a function with an inferred name still doesn't have a lexical name binding in the function body, but that's not relevant here)

Do you want to clarify that in the document itself? Via footnote, etc.

Sure, that's a great idea for a footnote!

... frome Style Guide:

// bad
const foo = function () {
  // ...
};

// good
const foo = function bar() {
  // ...
};

So, you say, there should be 2 different names for one function?

For example,

// Is it worse
const sum = function(a, b) {
  return a + b;
};

// than this?
const my_sum = function sum(a, b) {
  return a + b;
};

I'm a begginer in JS/ES, can you tell, please, what are advantages of second method?

@tandav In your last example, the first function doesn't have a .name - which means in debugging, it's likely to show up as an anonymous function. In the latter example, it will show up with the name sum - and when you grep for it, all the uses in the file will use my_sum, so the actual function will be easier to locate.

@ljharb I tested it using babel and babel-preset-env:

const foo = () => {}

will be converted to

var foo = function foo () {}

So it does have an .name. Also since Node v6 const foo = () => {} does have foo as .name (without babel). So it is not anonymous anymore.

Yes, that's function name inference. Which can't always be relied on, and isn't supported in older engines.

It remains preferred to use only explicit names, and never rely on inference.

@ljharb -- Closely related to @tandav's question...

Style guide example:

const foo = function () {
  // ...
};

// good
const foo = function bar() {
  // ...
};

Is this specifically saying that not only should the function be named rather than an anonymous, but that name (here, bar) should be distinct from the assignment (here, foo)?

Or would this also be adequate:

// bad
const foo = function () {
  // ...
};

// good
const foo = function bar() {
  // ...
};

// also good, or a reason these namings should be distinct?
const foo = function foo() {
  // ...
};

The example seems to imply that they should be distinct. If so, what is the advantage of that?

@amberleyromo indeed, they should be distinct. This is to improve readability and greppability - it lets you use a short variable name, but a long, verbose, unique function name; also, when self-referencing the function, it makes it very clear that you're referring to the lexical name binding, not the containing variable.

10.6 has an example that says

// good
export default function foo() {}

Is this in conflict with 7.1?

@chengyin I don't see why; that function is named, and export default takes an expression, not a declaration.

My understanding is the function is still declared in the scope, and hoisted.

Example:

foo();

export default function foo() {
  console.log('bar');
}

This will output bar in the console.

Doesn't this cause the same issue as in "Why? Function declarations are hoisted, which means that it’s easy - too easy - to reference the function before it is defined in the file. This harms readability and maintainability"? I see this example similar to the first bad example in 7.1.

My understanding of the spec is export default treats function as "HoistableDeclaration", not an expression: https://tc39.github.io/ecma262/#prod-ExportDeclaration, http://www.ecma-international.org/ecma-262/6.0/#sec-exports

fair point - however, we have no-use-before-define to prevent that.

The alternative would be const foo = function bar() {}; export default foo which is fine, but needlessly verbose.

I have a readability suggestion: why not do this edit based off of the comment above (#794 (comment)):

const foo = function foo_more_descriptive_name() {
  // ...
};

I support VermillionAzure's suggestion, I came here to figure out why there was different names and that explains why concisely.

I think that using a short name as a variable binding, one which will be used elsewhere to call the function, will harm readability in places where the function is called. One purpose of having a descriptive function name is to be more clear about what's happening in a section of code that calls the function. To find a function declaration, just grep function <name>.

To satisfy the block-scoping requirement as well as providing a name to the function, I think it would be perfectly acceptable to name the binding and the function the same thing, as @amberleyromo suggested (grep <name> = will also work in this case).

const initializeDatabase = function initializeDatabase() { }
// note: the name 'initializeDatabase' does *not* get hoisted

It looks a little silly, but so does

const foo = function bar() { }

Also, as a note for the future: when function name inference becomes more commonplace, I believe the naming requirement should be removed altogether, as this whole thing is really a confusing, wonky-looking workaround.

Name inference will always be implicit, so it will forever be a bad idea to rely on it - even when it's commonplace. Explicit > implicit :-)

Hi all! Does anyone know if there is an eslint rule for this? I've been making most of my functions in the following form by habit and I'd love to get a warning for it:

const foo =  () => ("Something");

@dmk255 https://eslint.org/docs/rules/func-style

// bad
console.log(`foo =`, foo);

var foo = function () {
    // ...
};

console.log(`foo =`, foo);

function foo() {
    // ...
};


// not too good
console.log(`foo =`, foo);

let foo = function () {
    // ...
};

// good
console.log(`foo =`, foo);

let foo = () => {
    // ...
};


// perfect
console.log(`foo =`, foo);

const foo = () => {
    // ...
};

@dmk255, @xgqfrms-gildata

I think it have been addressed previously in this thread. If I were right, what this guideline suggested is to use the following pattern:

// preferred
const foo = function foo_more_descriptive_name () {
    // ...
};

// ok
const foo = () => {
    // ...
};

Reason: this pattern is explicit and beneficial for debugging.

However, I agree with some people that foo_more_descriptive_name is wonky and somehow leading to confuse. So I personally just prefer name foo_more_descriptive_name as foo and let foo itself be descriptive.

On the other hand, depending on my working environment, in practice, I prefer to use const foo = () => {} because I will only working on the environment that function name inference is available.

BTW, I'm a believer of less is more (if possible, equivalent, no-side effects) ;).

Is this a problem for the object method shorthand?

foo({
  bar: function bar() {
    // ...
  },
});

vs.

foo({
  bar() {
    // ...
  },
});

@teohhanhui I don't think it is; because in this case you're using "bar" as a property name, not as a local binding.

But the rationale from https://github.com/airbnb/javascript#functions--declarations is as such:

Don’t forget to explicitly name the expression, regardless of whether or not the name is inferred from the containing variable (which is often the case in modern browsers or when using compilers such as Babel). This eliminates any assumptions made about the Error's call stack.

So it does sound like it may still cause debuggability issues?

@teohhanhui concise object methods are explicitly named, and do not use name inference, so it won't cause any issues.

also, when self-referencing the function, it makes it very clear that you're referring to the lexical name binding, not the containing variable.

@ljharb When would this actually be a problem? I don't understand. Would it matter since the containing variable points to the function?

If it’s reassigned, sure - which is something the engine has to keep track of, and could impact optimizations.

@patgrasso made a good point I think. Can you maybe add the following?

// good
const descriptiveName = function descriptiveName() {
  // ...
};

It's hard enough to find a good name for a function, but we have to do it once more with a more verbose version? It may be good technically, but is it actually practical?

I have a readability suggestion: why not do this edit based off of the comment above (#794 (comment)):

const foo = function foo_more_descriptive_name() {
  // ...
};

I am interested in knowing then, how will this function be called? is it foo() or foo_more_descriptive_name()?

@rotimi-best due to rules of JS scoping, from outside the function, only foo can reference it; inside it’d be both.

@amberleyromo indeed, they should be distinct. This is to improve readability and greppability - it lets you use a short variable name, but a long, verbose, unique function name; also, when self-referencing the function, it makes it very clear that you're referring to the lexical name binding, not the containing variable.

This doesn't pass the common sense test. I understand that this is perhaps a good idea given that in the current point in time, you can't always assume that name inference will work. But is this really a sane way to address the issue going forward?
You may be enthusiastic about this convention, but I think the vast majority of people are not. It's already hard enough to come up with the right name for a function: it needs to be both concise and descriptive. It's a skill that needs to be learned over time, and not everyone does it well. And that's with one name. I don't think it's realistic to expect people to come up with two separate names. If people feel like they're supposed to make them different, what will happen is that they'll come up with the short name (the one they'll actually type in their code) and then for the descriptive one they'll append something like ''_func" to it.
There is no point to a standard if it's too much of a hassle to use properly. This is something that really will get in the way of the coding thought process. It's intrusive and counter-intuitive. And the solution to the fundamental problem has already been addressed in modern browsers. It makes sense that it would be fixed at some point, because it was never the intention of any javascript implementation to have different names for the same function. If doing something the intended way causes unintended problems, then it's a defect. I don't think it's a good idea to base a coding style on unintended defects that are already addressed or will be eventually.

@Borthralla yes, it is a sensible way going forward since there's intuitive places that names should be inferred that they forever will not be inferred, unrelated to supported target environments.

On the other hand, depending on my working environment, in practice, I prefer to use const foo = () => {} because I will only working on the environment that function name inference is available. @leoyli

We are closing in on 2021 and with IE11 support dropped from many library, as well as in our project, is function name inference an issue anymore? Which browsers are those, to be more specific?

Yes. Many sites and companies still support older browsers, older than IE 11, but also, it’s just a better and clearer practice to have explicit names in your code instead of relying on inference.

@ljharb You are so incredibly patient πŸ‘

@ljharb I really agree with all the argument that @Borthralla put in the table. Please remember:

There are only two hard things in Computer Science: cache invalidation and naming things.
-- Phil Karlton

Also I tested with Babel in my project (in the same way that mentioned @JPeer264) throwing Error, and the stack trace of the error are identical using arrow function vs named function statement.

At this point it should be just:

const foo = () => { ... }

babel just traspiler to:

var foo = function foo () { ... }

if Babel does that, then it may not be transpiling correctly, because non-arrow functions and arrow functions have actual runtime differences.

If you want a named function, use one.

zhesha commented

Rule 7.1 states that I should use "named function expressions" and check it with "eslint: func-style" but eslint can check only that I use "function expression" or "function declaration". It did not check that the function expression is "named". Am I wrong? So the "eslint: func-style" is not applied to this rule. And there are no tools to check the whole codebase on this rule?
Also, the rule suggests not to use "function declarations", but the style guide violates it multiple times. Is it worth having a rule that can't be followed even in the style guide?
Rule 3.2

function getKey(k) {
  return `a key named ${k}`;
}

Rule 5.1

// best
function getFullName({ firstName, lastName }) {
  return `${firstName} ${lastName}`;
}

Rule 5.3

// good
function processInput(input) {
  // then a miracle occurs
  return { left, right, top, bottom };
}

Rule 6.3

// good
function sayHi(name) {
  return `How are you, ${name}?`;
}

And more. In the style guide itself, there are tons of function expressions and function declarations and only 3 examples contain "named function expressions".

ljharb commented

@zhesha func-style ensures that it's an expression, and func-names ensures it has a name.

Yes, a style guide must by necessity sometimes violate itself in order to show clear examples.

zhesha commented

@ljharb Thank you for the clarification. But as rule 7.1 is checked by two eslint rules I think both of them should be in a style guide. I have created PR with this tiny change, do you think it's possible to apply it? #2779

Like some others here, I'm challenged by this rule, but appreciate the reasoning behind it. If I'm calling the function by its variable, then I want that variable to be expressive, for readability.

const foo = function fooLex()

How about that?

Excerpt from MDN JS guide:
"...Providing a name allows the function to refer to itself, and also makes it easier to identify the function in a debugger's stack traces:"

const factorial = function fac(n) {
  return n < 2 ? 1 : n * fac(n - 1);
};

console.log(factorial(3)); // 6

Interesting that the function's lexical name is the short name, while the variable is more descriptive.

@bubbavox i agree with you that the internal name should be more descriptive than the outer variable, but pragmatism may prefer the other approach at times :-)