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

Added self-returning Symbol.iterator/@@iterator method to iterators #38

Closed
wants to merge 3 commits into from
Closed

Conversation

jaawerth
Copy link

@jaawerth jaawerth commented Sep 5, 2015

closes #37

  1. Adds self-returning protocols.iterator method to created iterators
  2. For consistency, wraps malformed iterators with same
  3. Updates the iterator test to check for iter[protocol]() === iter

@jaawerth
Copy link
Author

jaawerth commented Dec 2, 2015

Hi @jlongster,

Have you had a chance to take a look at it, and does that look alright for a merge? I've been using my fork for the ES6 interop, but I'm trying not to forget to check in before switching back. Thanks!

@jaawerth
Copy link
Author

jaawerth commented Mar 23, 2017

Due to inactivity, I'm going to close this for now so I can create a clean fork with the fix and publish it scoped, pointing back to this one. May recreate later.

@jaawerth jaawerth closed this Mar 23, 2017
@jlongster
Copy link
Owner

@jaawerth I'm sorry! I dropped out of open source for much of 2016 but I am watching things now, but haven't gone through all the old stuff. If you reopen this I can merge it.

@@ -4,7 +4,7 @@
var symbolExists = typeof Symbol !== 'undefined';

var protocols = {
iterator: symbolExists ? Symbol.iterator : '@@iterator'
iterator: symbolExists && Symbol.iterator || '@@iterator'
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is my only complain: can you revert this? I think it's less readable. Everything else looks good!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For sure, I saw that line when going back over it the other day and I honestly don't know what I was thinking there 🙃

@jaawerth
Copy link
Author

@jlongster No worries; figured it was something like that, but I wanted to use this in a new project so I figured I'd just publish a fork, but no need with you back in open sourceville - welcome back!

Since I'd foolishly created this PR from the master branch of my clone, I closed this one so I could create a new fork and use master for an updated README/package.json and such. That means I can't revert anything here, but I'll get you a new PR shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

no @@iterator method on iterators?
3 participants