path: join refactor

[Mnwa]

Change variables init from var to let/const, added destructurization to function arguments and change old for to new for of

Local bench tests join

                                                                         confidence improvement accuracy (*)   (**)   (***)
 path/join-posix.js n=1000000 paths='/foo|bar||baz/asdf|quux|..'                         2.42 %       ±7.16% ±9.54% ±12.42%
 path/join-win32.js n=1000000 paths='C:\\\\foo|bar||baz\\\\asdf|quux|..'                -0.92 %       ±5.18% ±6.90%  ±8.98%

Checklist

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

[Mnwa]

ping @nodejs
Can someone review it?

[addaleax]

CI: https://ci.nodejs.org/job/node-test-pull-request/20111/
Benchmark CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/286/

[apapirovski]

I’m afraid I don’t quite understand the purpose of this change. It’s functionally equivalent and doesn’t improve performance. It also makes git blame harder going forward. There is nothing fundamentally wrong with var.

[Mnwa]

Hi, @apapirovski! I was just update path join to actually js standards

[addaleax]

@apapirovski Do you feel differently about other refactors to using more modern JS patterns? I’m not happy about the effects on git blame myself, but this isn’t the first PR that does a change these lines.

[apapirovski]

@addaleax I’ve blocked most of them when I see them. I think I’ve been reasonably consistent. I’m happy to remove my objection if someone else wants to approve it.

[gireeshpunathil]

@apapirovski - pls revisit your position on this PR, in the context of several recent changes in the code base to modernize the variable qualifiers.

@Mnwa - can you pls rebase?

[Mnwa]

@gireeshpunathil Hey! I do it

[Mnwa]

But after rebase there is too small changes. I think we can close it becouse someone did a most my changes before