Skip to content

Chore: Use arrow functions where possible (lexical this). #7414

Description

@originalfoo
  • 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

  1. addaleax commented on Jun 25, 2016

    @addaleax
    Member

    although there may be several commits in the PR - one for each arrow function implemented in the file

    It’s up to the one who creates the PR how they do it, but the commits will very likely be squashed together anyway.

  2. originalfoo commented on Jun 25, 2016

    @originalfoo
    ContributorAuthor

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

  3. addaleax commented on Jun 25, 2016

    @addaleax
    Member

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

  4. originalfoo commented on Jun 25, 2016

    @originalfoo
    ContributorAuthor

    @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 unnecessary arguments objects, etc?

  5. addaleax commented on Jun 25, 2016

    @addaleax
    Member

    I wouldn’t expect any significant performance difference, no.

  6. originalfoo commented on Jun 25, 2016

    @originalfoo
    ContributorAuthor

    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/

  7. Fishrock123 commented on Jun 27, 2016

    @Fishrock123
    Contributor

    Arrow functions are great for replacing Function#bind() -- I think we've already done most or all of that though.

  8. lance commented on Jul 8, 2016

    @lance
    Member

    Heh - so, after submitting a PR for repl.js on 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 this or arguments for the function. But that's just napkin scratching.

  9. added
    lib / srcIssues and PRs involving general changes in the lib/ or src/ directories.
    on Nov 23, 2016
  10. jasnell commented on May 30, 2017

    @jasnell
    Member

    Closing. This has been happening incrementally

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    lib / srcIssues and PRs involving general changes in the lib/ or src/ directories.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions