Convert var to const/let in src files by aminya · Pull Request #22103 · atom/atom · GitHub
Skip to content
This repository was archived by the owner on Mar 3, 2023. It is now read-only.

Convert var to const/let in src files - #22103

Merged
sadick254 merged 4 commits into
atom:masterfrom
atom-community:no-var
May 19, 2021
Merged

sadick254 merged 4 commits into
atom:masterfrom
atom-community:no-var

Conversation

@aminya

@aminya aminya commented Mar 30, 2021 •

Copy link
Copy Markdown
Contributor

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
```
Comment thread src/compile-cache.js Outdated

@driktea driktea left a comment

Copy link
Copy Markdown

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

@sadick254 sadick254 left a comment

Copy link
Copy Markdown
Contributor

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