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

fix(Symbol.iterator): will not polyfill Symbol iterator unless Symbol… #2082

Merged
merged 1 commit into from
Nov 4, 2016
Merged

fix(Symbol.iterator): will not polyfill Symbol iterator unless Symbol… #2082

merged 1 commit into from
Nov 4, 2016

Conversation

benlesh
Copy link
Member

@benlesh benlesh commented Oct 25, 2016

… exists

  • adds more robust tests
  • removes strange logic added to support es6-shim, as no documentation or reasoning could be found to support its existence
  • ponyfills Symbol rather than polyfills... meaning the module just returns something that can be used as the symbol, but
    if Symbol exists as a function, like it should, and it does not have Symbol.iterator, only then will it polyfill it

BREAKING CHANGE: RxJS will no longer polyfill Symbol.iterator if Symbol does not exist. This may break code that inadvertently relies on this behavior

@benlesh
Copy link
Member Author

benlesh commented Oct 26, 2016

It looks like this may still be an issue with es6-shim, because they're using a substitute string that is unique to es6-shim, rather than @@iterator like other libraries.

… exists

- adds more robust tests
- removes strange logic added to support es6-shim, as no documentation or reasoning could be found to support its existence
- ponyfills Symbol rather than polyfills... meaning the module just returns something that can be used as the symbol, but
  if `Symbol` exists as a function, like it should, and it does not have `Symbol.iterator`, only then will it polyfill it

BREAKING CHANGE: RxJS will no longer polyfill `Symbol.iterator` if `Symbol` does not exist. This may break code that inadvertently relies on this behavior
@coveralls
Copy link

coveralls commented Oct 26, 2016

Coverage Status

Coverage increased (+0.2%) to 97.399% when pulling 9823db5 on blesh:symbol-iterator-coverage into 0271fab on ReactiveX:master.

@jayphelps
Copy link
Member

LGTM

@jayphelps jayphelps merged commit 1138c99 into ReactiveX:master Nov 4, 2016
@lock
Copy link

lock bot commented Jun 6, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants