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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't transpile ES7 symbol properties #5195

Merged
merged 1 commit into from Feb 7, 2017
Merged

Conversation

taion
Copy link
Contributor

@taion taion commented Jan 23, 2017

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

Pending zloirock/core-js#267, this works around the root cause of #4783 in zloirock/core-js#262 by pulling in the full Symbol polyfill instead of just the Symbol.asyncIterator polyfill.

Generally in the context of using the runtime transform, it'd be almost impossible to avoid pulling in the Symbol polyfill when using async iterators anyway, so in practice this is unlikely to make things worse.

More to the point, as the asyncIterator helper and the core-js imports are set up, async iteration won't work without pulling in the full Symbol polyfill anyway.

It's not clear to me how to add a test case for this, or whether it'd be necessary to do so here.

@mention-bot
Copy link

@taion, thanks for your PR! By analyzing the history of the files in this pull request, we identified @zloirock to be a potential reviewer.

@codecov-io
Copy link

Current coverage is 89.22% (diff: 100%)

Merging #5195 into master will not change coverage

@@             master      #5195   diff @@
==========================================
  Files           203        203          
  Lines          9838       9838          
  Methods        1071       1071          
  Messages          0          0          
  Branches       2624       2624          
==========================================
  Hits           8778       8778          
  Misses         1060       1060          
  Partials          0          0          

Powered by Codecov. Last update b69dc51...46e7c54

@taion
Copy link
Contributor Author

taion commented Jan 23, 2017

I could add a test case asserting that the runtime transform does not pull in the symbol/async-iterator polyfill, but I'm not sure if that's helpful.

@taion
Copy link
Contributor Author

taion commented Feb 2, 2017

Any updates here?

@xtuc xtuc added the PR: Bug Fix 🐛 A type of pull request used for our changelog categories label Feb 3, 2017
@steida
Copy link

steida commented Feb 6, 2017

Please merge, the React ecosystem needs it :-/

@hzoo
Copy link
Member

hzoo commented Feb 6, 2017

instead of transforming Symbol.asyncIterator to _asyncIterator2.default, it transforms it to _symbol2.default.asyncIterator

@hzoo hzoo merged commit 9de9232 into babel:master Feb 7, 2017
@taion taion deleted the symbol-es7 branch February 7, 2017 18:06
nicolo-ribaudo pushed a commit that referenced this pull request Mar 10, 2019
This is a repeat of #5195 to work around the same upstream issue.
@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.

building async iterators with transform-runtime produces wrong build, but babel-node works properly
7 participants