Repository navigation
http: use arrow function to take advantage of lexical this? #7295
Description
- Version: 6.2.1
- Platform:
- Subsystem: lib/_http_outgoing.js
In OutgoingMessage.prototype.end there's a self var that's only used once for the finish() function - would it be better to use an arrow function instead?
Current code: https://github.com/nodejs/node/blob/master/lib/_http_outgoing.js#L549
var self = this;
function finish() {
self.emit('finish');
}Proposed change:
let finish = () => this.emit('finish'); // lexical `this`Also, as the finish() function isn't used until the code around L584, would it be worth moving the function closer to where it's used (along with the callback check) to aid code clarity?
// keep these 3 blocks together as they are related
let finish = () => this.emit('finish');
if (typeof callback === 'function')
this.once('finish', callback);
if (this._hasBody && this.chunkedEncoding) {
ret = this._send('0\r\n' + this._trailer + '\r\n', 'latin1', finish);
} else {
// Force a flush, HACK.
ret = this._send('', 'latin1', finish);
}
Activity
Fishrock123 commentedon Jun 16, 2016
We'd be happy to review a pull request if you'd like to make one. :)
originalfoo commentedon Jun 17, 2016
Will do 👍
originalfoo commentedon Jun 23, 2016
PR sent :)
http: replace finish() callback with arrow function