http: convert var to let or const for http

[BeniCheni]

Refs #26486

@ BeniCheni Can you put the var -> let/const stuff in a separate commit? (Separate PR would be even better!)

Per this comment in #26486 ☝️ , this PR follows up to convert var variables to let or const, in the scope of http source in lib/ path.

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• commit message follows commit guidelines

[mscdex]

I think we've generally avoided these kinds of commits/PRs and preferred to only update them if you're already affecting those variables because of other changes.

[BeniCheni]

@mscdex, thanks for letting me know. Should I then try A) or B) below? Happy to address either way.

A) Revert everything except lib/_http_client.js, which this PR is trying to follow up with the comment in another PR

B) Just close this PR.

[jasnell]

I generally have no problem with these kinds of changes when done in batch. Lots of small changes like this get noisy.

[himself65]

i don’t think these changes have more improvements, but add some noisy commits

[BeniCheni]

i don’t think these changes have more improvements, but add some noisy commits

@himself65, while it’s true that there are no major improvements, there is an subtle improvement to use “const” over “var” as it will throw an error to “promote” immutability, if a “const” variable’s value is reassigned after its declaration.

[himself65]

@BeniCheni
this task which change “var” to “const” or “let” should do when refactoring code or add new methods or anything else.
only commit with change “var” is unnecessary i think

[BeniCheni]

@himself65, this is an actual follow-up per a suggestion from another PR, which is an actual change. (could be traced by 1st comment of this PR) The suggestion was to punt the var => let / const to a separate PR.

[BridgeAR]

I guess we might just go ahead and accept a couple PRs like this. Otherwise this is going to come up frequently and as soon as we are through with it, we do not have to worry about it anymore.

I am aligned with @jasnell that we should do this as batched change. It is actually quite simple and straight forward to use eslint to auto fix these. Should we just do that?

[ZYSzys]

Would changes like this make git blame less efficiently ?

[BridgeAR]

@ZYSzys it adds one more step to find the actual author. Most tools I know / use allow to directly jump to the former blame as well, so I personally do not see this as a big downside anymore.

[shisama]

@BridgeAR

It is actually quite simple and straight forward to use eslint to auto fix these. Should we just do that?

I agree this. Also, I think it is better to add no-var rule in eslint so that we won't worry about 'var' anymore. However, I think it needs agreement from some collaborators.

[BridgeAR]

@nodejs/tsc PTAL

[mcollina]

Good work! I don’t think this is a good idea just yet. There is still a perf gap in Node 8 when using let, and we still have to do backports there till the end of the year.

[BeniCheni]

Thanks for your time and feedback. I’m closing this PR to avoid potential further confusions.