various fix in lib #26562
various fix in lib #26562
Conversation
nodejs-github-bot
added
http
gengjiawen
force-pushed
the
redundant_assign
branch
from
2bf76ed to
3a01e41
Compare
| socket.cork(); | ||
| let ret; | ||
| for (var i = 0; i < outputLength; i++) { |
There was a problem hiding this comment.
I change var to let in my first try. But I got
/home/travis/build/nodejs/node/lib/_http_outgoing.js
802:8 error Use of `let` as the loop variable in a for-loop is not recommended. Please use `var` instead node-core/no-let-in-for-declarationCan we remove this rule in master branch ?
cc @BridgeAR @Trott
There was a problem hiding this comment.
It has been two years since #8637 (when using let/const lost performance), and let/const performance in V8 was improved due to #8637 (comment) and #9729 (comment)
I think maybe it's time to remove this rule now ?
There was a problem hiding this comment.
That rule was instituted because using let with a for loop that way could be drastically slower than using var. The problem was fixed in V8 6.0.
This performance problem still exists in Node.js 6.x (which ships with V8 5.1). Since code that lands on the master branch can be backported to 6.x, the rule remains in place. Support for 6.x ends this June, so starting in July, we can disable the rule. At that time, the oldest supported Node.js version will be 8.x, which currently ships with V8 6.2.
There was a problem hiding this comment.
Or start a pr and add the label don't land on 6.x ?
There was a problem hiding this comment.
6.x is in maintenance. I think we can already disable the rule.
gengjiawen
force-pushed
the
redundant_assign
branch
from
3a01e41 to
d768c5c
Compare
BridgeAR
added
the
author ready
PR-URL: nodejs#26562 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs#26562 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs#26562 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs#26562 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs#26562 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs#26562 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: #26562 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: #26562 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: #26562 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes