search

Yomguithereal / baobab

JavaScript & TypeScript persistent and optionally immutable data tree with cursors.
MIT License
3.15k stars 115 forks source link

Use global isNaN and parseInt for IE/Safari support #480

Closed jrust closed 7 years ago

jrust commented 7 years ago

Unfortunately neither Safari or IE support the versions of isNaN and parseInt that are on the Number object.

Yomguithereal commented 7 years ago

I guess the goal of @Nimelrian when using those functions was to align the splice function to JS specs. Can you ensure that moving to the global version does not change that (I know for instance that Number.isNaN and isNaN don't perform the same thing exactly.

And, if we can go back to the globals, parseInt(n, 10) should probably become +n, it's way easier.

jrust commented 7 years ago

Thanks for detailing where the change came from. I read through the ToNumber specification and tested splice in Chrome and adapted baobab's splice to further conform to it. It actually turns out that using parseInt resulted in some incorrect behavior because it parses booleans and empty strings as NaN whereas the spec allows those. Reworked the commit and its message with the new behavior and some tests.

Nimelrian commented 7 years ago

Looks like I missed some of the more specific parts of the specification there. Thanks for the fix!

Yomguithereal commented 7 years ago

Merging right now and publishing a new version soon :)

Yomguithereal commented 7 years ago

v2.4.3 is published on npm.

jrust commented 7 years ago

Great, thanks!