Naming arrow functions

[jscriptcoder]

Hi there,

Not sure whether this has been already mentioned before (I would be surpirsed if it never was)... Wouldn't it be a good idea to name arrow functions, which is what apparently ES6 does?. Think about the benefits of this in the stacktrace. It's quite useful to have a "named" function in there, instead of an anonymous one ;-)

So, this:

const double = x => x * 2;

would be transpiled down to:

var double = function double(x) {
  return x * 2;
};

Just to mention that Babel does it like that ;-)

Thanks.

[adidahiya]

This would also be nice to have for arrow functions used as class methods/properties.

private handleDocumentClick = (...) => {
    ...
}

[RyanCavanaugh]

Wouldn't it be a good idea to name arrow functions, which is what apparently ES6 does?

Can you clarify this part?

[RyanCavanaugh]

This would also be nice to have for arrow functions used as class methods/properties.

Note that we can't safely name class methods -- a reference to handleDocumentClick in the method body must refer to whatever that name means in the enclosing lexical scope, not the function itself.

[RyanCavanaugh]

Consulted with @bterlson and we think this is a spec-affecting behavior, specifically the places where the abstract SetFunctionName operation is invoked https://tc39.github.io/ecma262/#sec-setfunctionname

Regular function expressions have very noncomplaint behavior in current runtimes and should probably just be left alone (specifically, Edge, Chrome, and Node all produce an own property of name on functions, when they shouldn't)

[jscriptcoder]

Hi @RyanCavanaugh, answering your question... as far as I know in ES6 if an arrow function is defined on the right-hand-side of an assignment expression, the ‎engine will take the name on the left-hand-side and set the arrow function's ".name" based on that

Thanks

[nojvek]

Hi @RyanCavanaugh, could you explain the SetFunctionName operation? I couldn't fully grasp why it should be an issue.

I tried this in chrome.

let l = {log(){}}
console.log(l.log.name) //outputs 'log'

Generated typescript:

var l = { log: function () { } };
l.log.name // empty

So it seems under the covers v8 makes nice named anonymous functions.

[basarat]

Tried the following in the chrome dev console. Was surprised 🌹

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

[dsherret]

@basarat It didn't happen for me in v50.0.2661.102m, but I just upgraded to v51.0.2704.63m and now it's doing that.

[mhegazy]

PRs welcomed

[mpodlasin]

I will attempt to make a PR in following days or weeks, depending on how difficult it turns out to be. ;)

[mpodlasin]

@mhegazy For clarification - should PR for this issue fix all naming issues according to ES6 spec (naming arrow function object properties etc...), or only this one case of const x = () => {}?

Also, should it only target arrow functions or regular anonymous functions - function () {} - as well?

Also, is it ok to open work in progress PR to make sure changes go in good direction and get some help from community (as a newbie contributor)?

[mhegazy]

I would say all lambdas/function expressions should behave as such. feel free to send us incomplete PRs if you want to get early feedback. please tag @rbuckton on your PRs.

[sebinsua]

See also: #6757.

I notice the PR hasn't been merged, is anybody working on this?

[mpodlasin]

I worked on it for some time but I am currently not.

[bresleveloper]

using TypeScript 2.3.3, issue hasn't been solved, #6757 is a dup.

[christianalfoni]

@mpodlasin Would you have some pointers on where to start with this issue? I would love to fix it as the name of functions are valuable when building devtools.

[mpodlasin]

No, my approach was not good as you will see in review of my PR: #16001

I did not work on it after that.

Good luck!

[trotyl]

For symbols the function name would be wrapped in brackets:

const sym = Symbol('SYMBOL_NAME')
const obj = { [sym]: () => 42 }
console.log(obj[sym].name) // [SYMBOL_NAME]

As long as arrow function as property being respected, it may need adding runtime helpers to properly setting the name per spec.

[sabrehagen]

My issue #33408 was marked as a duplicate and closed. I have a working repro here where native JavaScript vs JavaScript compiled from TypeScript produces different functionality:

git clone https://github.com/sabrehagen/tsc-function-repro
yarn --cwd tsc-function-repro test

[sabrehagen]

Can we get an ETA on the resolution of this bug? There have been at least 6 github issues created since January 2016 regarding it, and it's still unresolved. Would be great to get some clarity on likelihood of resolution as our production code is affected by this functional inconsistency.

[jscriptcoder]

I cannot believe this is still happening after I reported this issue almost 4 years ago. Maybe we need a reminder:

Typescript today:

Babel, at least 4 years ago:

Chrome since arrow functions were introduced:

[RyanCavanaugh]

In case it isn't clear, anyone's free to send a PR when an issue is in the Backlog milestone. Otherwise we could probably do this for 3.8; there are a lot of bugs in the 3.7 milestone already.

[BrendanBall]

So I'm using typescript 3.6 with node v12.10.0 and targeting es2018 with module commonjs.
this works:

const foo = () => ({ foo: 'foo' })
export { foo }
console.log(foo.name)

This doesn't:

export const foo = () => ({ foo: 'foo' })
console.log(foo.name)

directly exporting a const changes the name behaviour

[nojvek]

@BrendanBall if you’re targeting es2018 then TS may not even be transpiling and letting node handle the arrow function.

This is really about how TS down transpiles arrow functions.

If I am correct this fix would be in the emit stage. Arrow functions need to know if they are assigned to a const. If so, emit with the name of the const

[BrendanBall]

@nojvek You are correct that TS isn't transpiling the arrow function, however it's still the TS transpilation that breaks the naming for me and the end result is the same, my arrow functions aren't named when they should be. Shall I open a separate issue for this? In my case it's breaking because of how export const is being transpiled to commonjs.

[nathonius]

This is the main reason that this change would be useful to me, since this is my React devtools at the moment, at least without manually setting .displayname.

[dragomirtitian]

@OfficerHalf doesn't react use babel for compilation anyway? So what TS transpilation looks like should not really matter..

@DanielRosenwasser In order to fix this babel mangles the variable names beyond recognition (ex).

Wouldn't a partial fix here for functions that do not reference themselves in the body be good enough ? Adding a name to those will not cause (*) any shadowing issues and it will cover most common scenarios while not mangling the output.

On the other hand this solution will require an extra pass over all arrow function to determine if they do reference themselves, which may impact performance so it might just make sense to label this as will not fix and closing it 🤷‍♀️

(*) as far as I can tell

[nathonius]

Sure, if you use CRA it uses babel. But you don't have to - if you use "jsx": "react" in your tsconfig, typescript will handle that part of the compilation. In this particular project we don't use babel.

[RyanCavanaugh]

I'm inclined to agree with @dragomirtitian that we should just Won't Fix this due to the rapidly-evaporating ecosystem of ES5-only runtimes. The actual impact here is quite minimal compared to the sheer ugliness of the required emit.

[ljharb]

IE 11 isn’t vanishing particularly rapidly.

[nathonius]

Agreed - over 50% of traffic I see in my particular industry comes from IE11. We can't deprecate it yet since some customers are unable to upgrade even if they wanted to.

[DanielRosenwasser]

Would it be acceptable if the names were set to something "sufficiently close" to the original name? The point in that being that it would make debugging easier even if it's not the correct behavior. So in

const foo = () => {
}

we would rewrite that to

var foo = function foo_1() {
}

[ljharb]

while that's certainly better than outputting no name at all, or a mangled name, that seems like a short-term workaround pending an actual fix.

[basarat]

Why would A be easier than the desired B?

• A (proposed workaround)

var foo = function foo_1() {
}

• B (desired)

var foo = function foo() {
}

[dragomirtitian]

@basarat Picking a name that does not conflict with anything else would remove the need to rewrite variable names if the body of the function references the the variable.

Ex:

var x;
x = recursive => {
    console.log("original x")
    return x(true); // The function assigned to x below is actually called
}
var y = x;
x = function () {
  console.log("new x")
}
y(true)

If you give the first function the name x, then the body of the function will see the function itself as x not the variable x (which is reassigned later) which will make the behavior different to the original code in ES2015 and above.

So you end up having to do a lot of variable name rewrites, which get pretty ugly pretty fast: ex

[ljharb]

Variable names aren’t observable, however, while function names are.

[remorses]

Can someone implement this please?

[jednano]

This appears to be fixed now, no?

[RyanCavanaugh]

@jedmao no. The relevant test would be

const n = () => "hello";

which should produce

var n = function n() { return "hello"; };
                 ^               

under target: es5

[rbuckton]

At this point I'm not sure its entirely necessary to fix this, except in possibly a very small number of cases. If you emit to ES5/ES3, but are running in an ES2015+ host, function names are automatically assigned from their variables using NamedEvaluation (which evolved from variable declaration evaluation in ES2015).

This means that if you have the following typescript:

const f = () => {};
console.log(f.name);

And we transpile it to the following ES5:

var f = function() {};
console.log(f.name);

Running that transpiled output will print f. However, if you are running in an ES5/ES3 host environment, all bets are off. .name wasn't standardized until ES2015, so there are no guarantees if you'll get "f", "", or undefined. The number of ES3/ES5 runtime environments is vanishingly small, and many (if not most) libraries and frameworks are dropping support for them.

Broadly emulating NamedEvaluation for ES5 emit would artificially inflate the overall emitted file size, for little benefit. In fact, the only place where I could see it have value is for downleveled exports and class fields, as property assignments do not call NamedEvaluation:

// ts
export const f = () => {};
console.log(f.name);
class C {
  x = () => {};
};
console.log(new C().x.name);

// es5 (commonjs)
exports.f = function () {};
console.log(exports.f.name);

var C = (function () {
  function C() {
    this.x = function() {};
  }
  return C;
})();
console.log(new C().x.name);

In ES2015+ with native ES modules, the above TS code prints f and x, but when targeting ES5 and commonjs, it prints empty strings.

This could possibly be resolved by leaning on NamedEvaluation for exports:

// es5 (commonjs)
var f = function() {}
exports.f = f;
console.log(exports.f.name); // prints 'f' in an ES2015+ host

Classes are a bit trickier if there is a naming conflict (though we could rename local variables to accommodate):

// ts
class C {
  x = () => {};
  constructor() {
    var x = 1;
  }
};

// es5 (commonjs)
// option 1, rename locals
var C = (function () {
  function C() {
    var x = function() {};
    this.x = x;
    
    var x_1 = 1;  // renamed local `x` to `x_1`
  }
  return C;
})();

// option 2, leverage _NamedEvaluation_ using object literals
var C = (function () {
  function C() {
    this.x = { x: function () {} }.x; // NamedEvaluation gives the name `x` to the function
    
    var x = 1;  // local not renamed
  }
  return C;
})();

One of the reasons to lean on NamedEvaluation is that we can't blindly add a name to a function expression as that can introduce scope conflicts. The name introduced in the function expression is locally scoped to the function expression body. Consider the two following cases:

// case 1
export let fib = (i: number) => i === 1 ? 0 : i === 2 ? 1 : fib(i - 1) + fib(i - 2);
const fib2 = fib;
fib = i => { console.log("i:", i); return fib2(i); };
console.log("# case 1");
console.log("result:", fib(3));

// case 2
function f(s: string) { console.log(s); }
class C {
  f = (s: string) => { f("Hello " + s); }
}

console.log();
console.log("# case 2");
new C().f("Alice");

If you emit to ESNext using native ES modules, you would get the following results:

# case 1
i: 3
i: 2
i: 1
result: 1

# case 2
Hello Alice

If we were to emit to ES5 and inject names into the functions, the output would change due to scope conflicts:

# case 1
i: 3
result: 1

# case 2
Error: Stack overflow

The reason for this would be the resulting emit:

exports.fib = function fib(i) { return i === 1 ? 0 : i === 2 ? 1 : fib(i - 1) + fib(i - 2) };
var fib2 = fib;
fib = function (i) { console.log("i:", i); return fib2(i); };

function f(s) { console.log(s); }
var C = (function () {
  function C() {
    this.f = function f(s) { f("Hello " + s); };
  }
  return C;
})();
new C().f("Alice");

In case 1, the fib inside of the arrow function ends up pointing at the name fib we might give to the function expression, rather than the outer fib variable.

In case 2, the f we call ends up invoking the function expression we assigned to the field f, rather than the name in the outer scope.

[ljharb]

When it was standardized isn't important; it was present in every version of every web browser except IE, and it's web reality that code expects it to work. It's also polyfillable in IE 9+, with https://npmjs.com/function.prototype.name, and there's lots of websites that rely on that working, including airbnb.com (when i last checked, at least).

[DanielRosenwasser]

Every product will have a set of tradeoffs it has to make, and prioritizing the work to do this "by the book" is work with a likely tail of bugs and emit size implications. I can't speak for others who are on this issue, but I tend to agree with Ron that the people who would benefit from this have shrunk a lot as a group. For example:

there's lots of websites that rely on that working, including airbnb.com (when i last checked, at least).

Windows 10 (released late July 2015) actually makes it hard to access IE11. In fact, it will automatically punt you to Edge unless you disable a setting within Edge. But for the sake of it, here's Airbnb running in IE11:

abnb-ie11.mp4

[ljharb]

@DanielRosenwasser that's a totally reasonable assessment; my pushback was against trying to justify not doing the work as a result of when something was standardized.

Regardless of what's done by default, why not offer this as a config option, for those who want to emit correct code?

[sandersn]

This is not practical to fix: the costs far outweigh the benefits.

[ljharb]

@sandersn even behind an option?