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

Fix esnext.map.delete-all.js #1302

Closed
wants to merge 1 commit into from

Conversation

jhmaster2000
Copy link

Currently the Map#deleteAll polyfill is returning a boolean whether it deleted everything successfully or not, this is not the correct behavior of the current ES proposal: https://tc39.es/proposal-collection-methods/#Map.prototype.deleteAll

The expected behavior is to return the map itself (this), so I've also removed all the successful deletion tracking code as it was no longer needed.

@zloirock
Copy link
Owner

Thanks, it's a good catch. However, let's take a look at Set#deleteAll draft text - that returns a boolean, but they should be consistent. It seems it's need to raise this issue in the proposal repo.

@zloirock
Copy link
Owner

@zloirock
Copy link
Owner

See this issue tc39/proposal-collection-methods#53 - it seems the current semantic is correct and it should be fixed in the spec text.

@jhmaster2000
Copy link
Author

Oh I missed that inconsistency, I did find it odd to return this as the boolean return does feel way more helpful too. I'll leave it up to you if you want to close this already or wait until there's an official resolution on the proposal issue, either works for me.

@zloirock
Copy link
Owner

I did find it odd to return this as the boolean return does feel way more helpful too.

If you think so, you can speak out in the above-mentioned issue -)

@zloirock
Copy link
Owner

I don't think that we will have the answer to tc39/proposal-collection-methods#53 soon, but it seems we can consider tc39/proposal-collection-methods#2 as that, so I think that we can close this PR.

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

Successfully merging this pull request may close these issues.

None yet

2 participants