Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix loose for-of with label #5298

Merged
merged 1 commit into from Feb 13, 2017
Merged

Conversation

jridgewell
Copy link
Member

@jridgewell jridgewell commented Feb 11, 2017

Q A
Patch: Bug Fix? Yes
Major: Breaking Change? No
Minor: New Feature? No
Deprecations? No
Spec Compliancy? Yes?
Tests Added/Pass? Yes
Fixed Tickets N/A
License MIT
Doc PR N/A
Dependency Changes N/A

Transforming a for-of with a label in loose mode was dropping the label. 馃挬

@mention-bot
Copy link

@jridgewell, thanks for your PR! By analyzing the history of the files in this pull request, we identified @hzoo, @loganfsmyth and @danez to be potential reviewers.

@existentialism existentialism added the PR: Bug Fix 馃悰 A type of pull request used for our changelog categories label Feb 11, 2017
@danez
Copy link
Member

danez commented Feb 12, 2017

Do you know where/why it gets lost?

@jridgewell
Copy link
Member Author

I'm guessing it's because of the #replaceWithMultiple. Where would the label go if there really were multiple nodes?

@jridgewell
Copy link
Member Author

jridgewell commented Feb 12, 2017

Confirmed

@hzoo hzoo merged commit ca45904 into babel:master Feb 13, 2017
@hzoo
Copy link
Member

hzoo commented Feb 13, 2017

Thanks @jridgewell!

danez added a commit that referenced this pull request Feb 14, 2017
* Add new flow preset (#5288)

* Fix PathHoister hoisting JSX member expressions on "this". (#5143)

The PathHoister ignored member references on "this", causing it
to potentially hoist an expression above its function scope.

This patch tells the hoister to watch for "this", and if seen,
mark the nearest non-arrow function scope as the upper limit
for hoistng.

This fixes #4397 and is an alternative to #4787.

* Fix PathHoister hoisting before bindings. (#5153)

Fixes #5149 and enables a few additional safe hoists.

* Fix linting error

* feature: Support pure expressions in transform-react-constant-elements (#4812)

* Fix loose for-of with label (#5298)

* Rewrite Hub as interface #5047 (#5050)

* Rewrite Hub as interface #5047

* Update index.js

* Avoid adding unnecessary closure for block scoping (#5246)

When you write

```
for (const x of l) {
  setTimeout(() => x);
}
```

we need to add a closure because the variable is meant to be block-scoped and recreated each time the block runs. We do this.

However, we also add the closure when no loop is present. This isn't necessary, because if no loop is present then each piece of code runs at most once. I changed the transform to only add a closure if a variable is referenced from within a loop.

* Add greenkeeperio-bot to mention-bot blacklist (#5301) [skip ci]

* Upgrade lerna to current beta. (#5300)

* Revert "Upgrade lerna to current beta." (#5303)

* Add charset so tests work with convert-source-map@>1.4 (#5302)

* Add CHANGELOG for 6.23.0 [skip ci] (#5304)

* Update babel-types README from script.

* v6.23.0

* Revert change that lerna force-committed.

* Revert "Rewrite Hub as interface #5047" (#5306)

* v6.23.1

* Revert lerna again
@hzoo
Copy link
Member

hzoo commented Feb 15, 2017

@jridgewell was wondering if you'd like to be added as a collaborator (since you've been making a lot great prs/fixes!)

@jridgewell
Copy link
Member Author

jridgewell commented Feb 15, 2017

That'd be great! 馃榾

existentialism pushed a commit that referenced this pull request May 19, 2017
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 6, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Bug Fix 馃悰 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants