benchmark,lib: var to const

[BridgeAR]

This came up recently again. I just ran a modified eslint rule against lib and benchmark and I have no strong opinion about actually doing this and I am absolutely fine with closing this again. It's just there to resolve a comment in the referenced PR.

Note: I did not update any eslint rules as there is no explicit rule to do this. Instead, I just modified the prefer-const rule to match against var instead of let.

Refs: #26679

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

[nodejs-github-bot]

@BridgeAR build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/3044/pipeline

[gengjiawen]

I am +1 on this.
Any thought on var to let ? Maybe in the end ,we can add https://eslint.org/docs/rules/no-var.

[tniessen]

I know we have shot these PRs down multiple times, but this one is only 343 lines and if that's all it takes to make all possible var → const replacements, I think we should do it.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/21919/

[Trott]

Note: I did not update any eslint rules as there is no explicit rule to do this. Instead, I just modified the prefer-const rule to match against var instead of let.

const and let have the same scoping rules so that ESLint rule makes sense. But var is different. Is there a chance that this introduces subtle bugs where some code expects a variable to be hoisted but now it isn't? I'm guessing there isn't a problem because all the tests are passing, but an extra careful eye in review and maybe even a CITGM run might be worthwhile. (Not saying it's semver major--just saying extra test exercising via CITGM might be a good extra-cautious measure.)

[BridgeAR]

@Trott eslint itself should warn in case the scope is not correct anymore. There could in theory also be changes that can not be auto detected but that's unlikely. Especially with our test suite.

[Trott]

@Trott eslint itself should warn in case the scope is not correct anymore.

Ah, yes, of course.

There could in theory also be changes that can not be auto detected but that's unlikely. Especially with our test suite.

👍

[BridgeAR]

Rebased due to conflicts.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/21982/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/21993/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/22007/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/22010/ ✔️

[targos]

Landed in f86f573