Repository navigation
Chore: Use arrow functions where possible (lexical this). #7414
Description
- Version: 6.2.1
- Platform:
- Subsystem: various
Following on from #7295, I've spotted similar pattern in many other files:
var self = this;
something(someFunc(...) {
self.whatever();
});In most scenarios, this pattern can be replaced with an arrow function, allowing removal of 'self' var:
something((...) => {
this.whatever();
});I'd like to use this issue ticket as a hub for several commits/PRs relating to this (one changed file per PR, although there may be several commits in the PR - one for each arrow function implemented in the file).
Activity
debugger: use arrow function for lexical `this`
debugger: use arrow function for lexical `this`
debugger: use arrow function for lexical `this`
fs: use arrow function for lexical `this`
fs: use arrow function for lexical `this`
fs: use arrow function for lexical `this`
fs: use arrow function for lexical `this`
addaleax commentedon Jun 25, 2016
It’s up to the one who creates the PR how they do it, but the commits will very likely be squashed together anyway.
fs: use arrow function for lexical `this`
fs: use arrow function for lexical `this`
fs: use arrow function for lexical `this`
originalfoo commentedon Jun 25, 2016
@addaleax it's more so I can interactive rebase or cherry-pick should a specific commit need removing etc; just sort of a habit I got in to.
addaleax commentedon Jun 25, 2016
@aubergine10 As said, it’s up to you. :)
Tbqh, I’d like to hear or work out some kind of rule of thumb of which types of style-only changes are generally considered worthwhile. These kind of changes tend to generate a lot of noise, and while I see that something like #7378 can improve readability noticeably, I’m not sure that’s always the case.
originalfoo commentedon Jun 25, 2016
@addaleax Ultimately it will probably depend on whether there are any improvements in terms of performance, gc, RAM consumption, etc. I'm not skilled enough to determine whether there are any tangible benefits to replacing the
var self...pattern with arrow functions. I imagine the V8 optimising compiler will already seek to avoid creating unnecessaryargumentsobjects, etc?addaleax commentedon Jun 25, 2016
I wouldn’t expect any significant performance difference, no.
originalfoo commentedon Jun 25, 2016
Based on the discussion yesterday I did some googling regarding performance of arrow functions and other ES6 features and found this: http://kpdecker.github.io/six-speed/
If those results are correct, it looks like ES6 features, in general, are currently offering very little benefit over their ES5 counterparts and in many cases result in notably worse performance.
I noticed that the Node.js test scripts include some performance tests but I'm not sure how to extract that data and compare to data from the unmodified scripts... Would be interesting to see what the performance impact is of the current two batches of edits. Without a clear improvement in performance, I don't think there's any benefit to using arrow functions over the ES5 approach.
Another interesting read on arrow functions: https://blog.getify.com/arrow-this/
Fishrock123 commentedon Jun 27, 2016
Arrow functions are great for replacing
Function#bind()-- I think we've already done most or all of that though.lance commentedon Jul 8, 2016
Heh - so, after submitting a PR for
repl.json this, I decide to read the comments. I'll leave the PR open for now anyway...Curious that certain arrow operations are slower than ES5 (semi-)equivalents. Without any real understanding of how V8 implements these language features it seems arrow functions should typically perform faster because the runtime doesn't need to allocate a new
thisorargumentsfor the function. But that's just napkin scratching.debugger: use arrow function for lexical `this`
debugger: use arrow function for lexical `this`
debugger: use arrow function for lexical `this`
debugger: use arrow function for lexical `this`
debugger: use arrow function for lexical `this`
debugger: use arrow function for lexical `this`
debugger: use arrow function for lexical `this`
jasnell commentedon May 30, 2017
Closing. This has been happening incrementally