-
Notifications
You must be signed in to change notification settings - Fork 148
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New transform: for loops to for of
#166
Comments
for of
for of
This is definitely something that's been in the plans for a long time. Unfortunately it's not quite so simple transforms to write, so haven't really taken up it so far. There are several things to consider: Skipping cases where array index is also used I think the following can't be transformed: for (var i = 0; i < fruits.length; i++) {
console.log(fruits[i], i);
} Naming of the loop variable Sometimes there's a var declared inside the loop, which we could eliminate: for (var i = 0; i < fruits.length; i++) {
var fruitItem = fruits[i];
console.log(fruitItem);
}
// -->
for (var fruitItem of fruits) {
console.log(fruitItem);
} At other times we'll need to make up the name by ourselves. A fairly good case is when the array is named in plural then we can pick a variable name that's the singular form of it: for (var i = 0; i < fruits.length; i++) {
console.log(fruits[i]);
}
// -->
for (var fruit of fruits) {
console.log(fruit);
} I'm not really sure what to do with other cases where we're looping over a variable like for (var i = 0; i < array.length; i++) {
console.log(array[i]);
}
// -->
for (var arrayItem of array) {
console.log(arrayItem);
} But this way we might also end up with silly stuff like Automatically generating useful variable names is a tricky business... Avoid naming conflicts When we introduce a new name we'll also have to watch out for possible existing variables with the same name, like: var fruit = "banana";
for (var i = 0; i < fruits.length; i++) {
console.log(fruits[i]);
} PS: And this transform should not change from |
After laying out my thoughts above I realized it might not be so hard after all and did an initial implementation. Currently only supporting the case where we already have a name for loop variable. There's also the case that variable scoping is not properly accounted for. I wonder whether it would make sense to only perform the transform when the loop uses |
@nene What is the expectation from a transform (or in general with lebab)? Is it to convert all your code to es2015? Take this for example- Accurate singular names for I see lebab as a companion (not as a tool) to convert code to es6 That being said, I'm perfectly okay, if all my code was not transformed. Pick the obvious cases and only convert those. |
@siddharthkp I think that's my general opinion as well. One really shouldn't use Lebab blindly - the changes need some human review. Then again, Lebab should not do changes that might break stuff. Ideally when reviewing the transformed code you would not need to fix any actual bugs, but more like manually transform the cases that Lebab didn't handle or adjust the code style when Lebab didn't format the code to the best of your preferences. In that regard some transforms are very safe, like |
Require that the 'let' transform is run beforehand. This eliminates the need to do complex scope-detection within this transform - let transform will take care of that, and if it's unable to perform the transform, the for-of transform can also not be applied. Refs #166
Directing users to use let transform first. Refs #166
Luckily we already have a helper for that Refs #166
This is now released as v2.2.0 |
Closing this for now, but created a follow-up for further development: #169 |
Convert this
to this
The text was updated successfully, but these errors were encountered: