Skip to content
This repository was archived by the owner on Mar 3, 2023. It is now read-only.

Conversation

aminya
Copy link
Contributor

@aminya aminya commented Mar 30, 2021

Description

This converts var to let/const.

Verification

This conversion is done using jscodeshift developed by Facebook which parses JavaScript down to AST level and performs scope analysis and reference counting to safely replace var with const or let. This is much safer than converting these by hand which is error-prone.

npm install -g jscodeshift
git clone cpojer/js-codemod.git
jscodeshift -t js-codemod/transforms/no-vars.js ./src

All the tests and CI must pass.

Benefits

  • Using var results in unwanted behavior because its scope is leaked upwards. This PR prevents these kinds of bugs. JsCodeshift already finds the cases where the conversion is not safe and var is leaked intentionally and leaves them as is.

  • This allows V8 to perform better optimizations for the code

Release Notes

  • Convert var to let/const in src folder

This PR supersedes the var conversion in #22100

aminya added 2 commits March 30, 2021 10:12
```
npm install -g jscodeshift
git clone https://github.com/cpojer/js-codemod.git
jscodeshift -t js-codemod/transforms/no-vars.js ./src
```
Copy link

@driktea driktea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@darangi darangi added the triaged label Apr 6, 2021
Copy link
Contributor

@sadick254 sadick254 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@sadick254 sadick254 merged commit a8aa5ea into atom:master May 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants