search

zloirock / core-js

Standard Library
MIT License
23.99k stars 1.62k forks source link

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

Closed jhmaster2000 closed 6 months ago

jhmaster2000 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

https://github.com/tc39/proposal-collection-methods/issues/2

zloirock commented 6 months ago

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

jhmaster2000 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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