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

Distinguish Set and Map iterators, with tests #437

Merged
merged 1 commit into from
Apr 7, 2017

Conversation

Xotic750
Copy link
Contributor

@Xotic750 Xotic750 commented Mar 19, 2017

Closes #387. Closes #391.

Copy link
Collaborator

@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.

Thanks!

test/set.js Outdated
@@ -634,4 +634,22 @@ describe('Set', function () {

it.skip('should throw proper errors when user invokes methods with wrong types of receiver', function () {
});

it('SetIterator identification', function () {
Copy link
Collaborator

Choose a reason for hiding this comment

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

we should conditionally skip this first test when Object.getPrototypeOf is not available (in a separate it; same in the Map tests)

@ljharb ljharb changed the title Distinguish Set and Map iterators. Distinguish Set and Map iterators, with tests Mar 20, 2017
@Xotic750
Copy link
Contributor Author

Xotic750 commented Apr 2, 2017

Were there any change requests that I missed, or is there anything more to add for this?

es6-shim.js Outdated
@@ -2842,7 +2842,11 @@
};

MapIterator.prototype = {
isMapIterator: true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

it occurs to me that this is too easily forgeable; but that since instanceof can't work cross-realm, this might be the best we can get.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can't think why anyone would want to forge it, but I also can't think of anything to make it more unforgeable.

@ljharb ljharb force-pushed the identifyCollectionIterators branch from fdf7c68 to b1d9b84 Compare April 7, 2017 05:26
@ljharb ljharb merged commit b1d9b84 into paulmillr:master Apr 7, 2017
@ljharb
Copy link
Collaborator

ljharb commented Apr 7, 2017

For posterity; this PR included ba1770b, 79d80a6, and b1d9b84; I mistakenly merged the first two and forgot the third.

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

Successfully merging this pull request may close these issues.

2 participants