Editorial: Clarify GetIterator #3021
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
It's easiest to review this PR commit by commit. |
michaelficarra
approved these changes
Mar 2, 2023
ljharb
approved these changes
Mar 2, 2023
jmdyck
previously requested changes
Mar 2, 2023
gibson042
added a commit
to gibson042/ecma402
that referenced
this pull request
Mar 2, 2023
tc39/ecma262#3021 proposes removing optionality from the second argument
gibson042
approved these changes
Mar 2, 2023
There was a problem hiding this comment.
I think you missed the downstream consumers check, but I've opened tc39/ecma402#756 to be ready.
spec.html
Outdated
Comment on lines
7001
to
7014
| IterableToList ( | ||
| _items_: an ECMAScript language value, | ||
| optional _method_: a function object, | ||
| IteratorToList ( | ||
| _iteratorRecord_: an Iterator Record | ||
| ): either a normal completion containing a List of ECMAScript language values or a throw completion |
There was a problem hiding this comment.
@ptomato Assuming this lands, Temporal IterableToListOfType should probably change accordingly.
ryzokuken
pushed a commit
to tc39/ecma402
that referenced
this pull request
Mar 7, 2023
tc39/ecma262#3021 proposes removing optionality from the second argument
bakkot
added
the
ready to merge
5 tasks
This removes the optional _hint_ parameter and the weird recursive call in GetIterator.
Also remove the the optional method parameter and change the callsites to explicitly get the iterator.
This is editorial because Call inside GetIteratorFromMethod would throw on undefined anyways, and this keeps GetIteratorFromMethod cleaner in only accepting function objects.
ljharb
force-pushed
the
clarify-getiterator
branch
from
e5edb51
to
784fc65
Compare
ptomato
added a commit
to tc39/proposal-temporal
that referenced
this pull request
Apr 15, 2023
Based on the rewrite of IterableToList in tc39/ecma262#3021, this makes the corresponding change to IterableToListOfType.
Ms2ger
pushed a commit
to tc39/proposal-temporal
that referenced
this pull request
Apr 17, 2023
Based on the rewrite of IterableToList in tc39/ecma262#3021, this makes the corresponding change to IterableToListOfType.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I find GetIterator's optional parameters confusing because the hint is ignored when the method is passed. There's also a recursive call that serves no real purpose.
This PR gets rid of the optional parameters and does some refactoring.