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

Normative: close underlying iterators when return is called before next #267

Merged
merged 3 commits into from Mar 30, 2023

Conversation

bakkot
Copy link
Collaborator

@bakkot bakkot commented Feb 16, 2023

Fixes #266 (cc @syg).

Note that we'll need consensus from committee before merging.

(The slot has to be named [[UnderlyingIterator]] instead of [[Iterated]] because we use [[Iterated]] for the name of a slot on %WrapForValidIteratorPrototype% objects, and we don't want to accidentally make iterator helper objects valid .call targets for %WrapForValidIteratorPrototype%.next.)

@ljharb
Copy link
Member

ljharb commented Feb 16, 2023

Any chance you could provide a test case that fails without this PR?

@bakkot
Copy link
Collaborator Author

bakkot commented Feb 16, 2023

Something along the lines of

let returnCalls = 0;
let it = {
  __proto__: Iterator.prototype,
  next() {},
  return() {
    ++returnCalls;
  },
};
let mapped = it.map(() => {});
mapped.return();
assert(returnCalls === 1); // would be 0 without this PR
mapped.return();
assert(returnCalls === 1); // only gets called the first time

@ljharb

This comment was marked as resolved.

@bakkot

This comment was marked as resolved.

@ljharb

This comment was marked as resolved.

spec.html Show resolved Hide resolved
ljharb added a commit to es-shims/iterator-helpers that referenced this pull request Feb 16, 2023
Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

I've implemented and tested the changes in my polyfill, works as advertised.

ljharb added a commit to es-shims/iterator-helpers that referenced this pull request Mar 21, 2023
@michaelficarra michaelficarra merged commit 5a57b6b into main Mar 30, 2023
1 check passed
@michaelficarra michaelficarra deleted the close-not-started branch March 30, 2023 01:41
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.

Calling return() on a suspendedStart iterator helper doesn't seem to call return() on the underlying
3 participants