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

es6 array iterator polyfill in_iter-define: "Safari has buggy iterators w/o next" - is the logic for this actually correct? #236

Closed
clarabstract opened this issue Sep 3, 2016 · 3 comments
Labels

Comments

@clarabstract
Copy link

I am referring to this line from this file:

  , BUGGY          = !([].keys && 'next' in [].keys()) // Safari has buggy iterators w/o `next`

it is used when deciding to override Array.prototype[Symbol.iterator], here:

 // Define iterator
  if((!LIBRARY || FORCED) && (BUGGY || VALUES_BUG || !proto[ITERATOR])){
    hide(proto, ITERATOR, $default);
  }

The problem is the value of $default simply grabs the existing native implementation which we just decided is buggy:

   , $native    = proto[ITERATOR] || proto[FF_ITERATOR] || DEFAULT && proto[DEFAULT]
    , $default   = $native || getMethod(DEFAULT)

It will use either Array.prototype[Symbol.iterator] or Array.prototype.values.

So either this logic is wrong, or this specific Safari bug only affects the iterator returned by Array.prototype.keys (and not .values or [Symbol.iterator]) which seems unlikely. Why bother overwriting hide(proto, ITERATOR, $default); in that case?

I ran into this while working with a page inside EAWebKit. Of course this is not an officially supported engine, however, its JS engine definitely exhibits this exact problem ([].keys() returns an iterator without a .next() method - as does .values()). My guess is EA just so happened to fork off the same buggy version of WebKit as Safari did when this behavior was dealt with.

core-js definitely fails to polyfill Array iterators in EAWebKit, so this leaves two possibilities:

  1. Either the core.js logic for this fix is wrong and Safari patched the bug before anyone noticed, or
  2. The bug core.js is trying to fix really did only affect the keys iterator, in which case EAWebKit either introduced the bug or forked a weird version of WebKit nobody has really looked at.

In the latter case we need to seriously re-evaluate EAWebKit and be on the lookout for all sorts of bugs unique to it, so I am very keen on finding out which it actually is.

If it's the former case I am happy to provide a PR.

@clarabstract
Copy link
Author

Hey folks I'd really like to have this merged into the main line if possible. Anything else you require in the PR to make that happen? I'm reasonably sure it affects older WebKits as well, not just the EAWebKit fork.

@zloirock
Copy link
Owner

zloirock commented Dec 9, 2017

Sorry that I missed it.

@zloirock
Copy link
Owner

Fixed in [email protected].

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

No branches or pull requests

2 participants