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

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

Closed
Arnavion opened this Issue Mar 21, 2016 · 30 comments

Comments

Projects
None yet
@Arnavion
Contributor

Arnavion commented Mar 21, 2016

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 ?

@ljharb

This comment has been minimized.

Member

ljharb commented Mar 21, 2016

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)

@ljharb ljharb closed this Mar 21, 2016

@Arnavion

This comment has been minimized.

Contributor

Arnavion commented Mar 21, 2016

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

@ljharb

This comment has been minimized.

Member

ljharb commented Mar 21, 2016

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

@tandav

This comment has been minimized.

tandav commented Feb 26, 2017

... 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?

@ljharb

This comment has been minimized.

Member

ljharb commented Feb 26, 2017

@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.

@JPeer264

This comment has been minimized.

Contributor

JPeer264 commented Mar 11, 2017

@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.

@ljharb

This comment has been minimized.

Member

ljharb commented Mar 11, 2017

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.

@amberleyromo

This comment has been minimized.

amberleyromo commented Mar 22, 2017

@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?

@ljharb

This comment has been minimized.

Member

ljharb commented Mar 22, 2017

@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.

@chengyin

This comment has been minimized.

chengyin commented Jul 8, 2017

10.6 has an example that says

// good
export default function foo() {}

Is this in conflict with 7.1?

@ljharb

This comment has been minimized.

Member

ljharb commented Jul 9, 2017

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

@chengyin

This comment has been minimized.

chengyin commented Jul 9, 2017

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.

@chengyin

This comment has been minimized.

chengyin commented Jul 9, 2017

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

@ljharb

This comment has been minimized.

Member

ljharb commented Jul 9, 2017

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.

@VermillionAzure

This comment has been minimized.

VermillionAzure commented Aug 29, 2017

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() {
  // ...
};
@mtfurlan

This comment has been minimized.

mtfurlan commented Sep 8, 2017

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

jaylaw81 added a commit to appirio-digital/ads-best-practices that referenced this issue Sep 19, 2017

@patgrasso

This comment has been minimized.

patgrasso commented Sep 20, 2017

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.

@ljharb

This comment has been minimized.

Member

ljharb commented Sep 20, 2017

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 :-)

@dan-kez

This comment has been minimized.

dan-kez commented Mar 12, 2018

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");
@xyzdata

This comment has been minimized.

xyzdata commented Mar 31, 2018

@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 = () => {
    // ...
};
@leoyli

This comment has been minimized.

leoyli commented Apr 12, 2018

@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) ;).

LoneRifle added a commit to datagovsg/beeline-frontend that referenced this issue Apr 16, 2018

LoneRifle added a commit to datagovsg/beeline-frontend that referenced this issue Apr 16, 2018

@teohhanhui

This comment has been minimized.

teohhanhui commented Jun 14, 2018

Is this a problem for the object method shorthand?

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

vs.

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

This comment has been minimized.

Member

ljharb commented Jun 14, 2018

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

@teohhanhui

This comment has been minimized.

teohhanhui commented Jun 14, 2018

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?

@ljharb

This comment has been minimized.

Member

ljharb commented Jun 14, 2018

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

@Edwardzyc

This comment has been minimized.

Edwardzyc commented Aug 25, 2018

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?

@ljharb

This comment has been minimized.

Member

ljharb commented Aug 25, 2018

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

@michaelKaefer

This comment has been minimized.

Contributor

michaelKaefer commented Oct 13, 2018

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

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

This comment has been minimized.

Member

ljharb commented Oct 13, 2018

@teohhanhui

This comment has been minimized.

teohhanhui commented Oct 15, 2018

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment