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

Do other checks before GetIteratorDirect #264

Closed
kmiller68 opened this issue Feb 11, 2023 · 3 comments · Fixed by #265
Closed

Do other checks before GetIteratorDirect #264

kmiller68 opened this issue Feb 11, 2023 · 3 comments · Fixed by #265

Comments

@kmiller68
Copy link

Right now the current way the spec is phrased makes it impossible to polyfill/implement the proposal in JS as a for-of loop. Since you'd like to do something like:

function* map(callback) {
  if (!isCallable(callback))
    throw new TypeError();

  for (let item of this)
    yield callback(item); 
}

but the current spec prohibits such an implementation because it opens the iterator before checking for is callable and doing something like:

function* map(callback) {
  let didChecks = false;

  for (let item of this) {
    if (!didChecks) {
      didChecks = true;
      if (!isCallable(callback))
        throw new TypeError();
    }
    yield callback(item); 
  }
}

still doesn't work because it pumps the iterator once before checking if the callback is callable. Could we change the spec to move the Let iterated be ? [GetIteratorDirect](https://tc39.es/proposal-iterator-helpers/#sec-getiteratordirect)(this value). line of each method next to the appropriate Let next be ? [IteratorStep](https://tc39.es/ecma262/#sec-iteratorstep)(iterated). line?

@bakkot
Copy link
Collaborator

bakkot commented Feb 11, 2023

Hm, yeah. I also think it would be more consistent with the rest of the spec to do all of the validation (of this and the arguments) before we start reading from this. I agree we should change these.

@bakkot
Copy link
Collaborator

bakkot commented Feb 11, 2023

Opened #265. Assuming other people are on board with this change we'll ask for consensus for it in March.

@bakkot
Copy link
Collaborator

bakkot commented Feb 13, 2023

I should note that it even with this tweak it's non-trivial to polyfill with a for-of because iterator helpers use this.next directly, rather than using this[Symbol.iterator]().next as a for-of would.

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 a pull request may close this issue.

2 participants