search

paulmillr / es6-shim

ECMAScript 6 compatibility shims for legacy JS engines
http://paulmillr.com
MIT License
3.11k stars 387 forks source link

Add tests for Number constructor that highlight deficiencies in certain environments. #366

Closed Xotic750 closed 8 years ago

Xotic750 commented 8 years ago

These were recently fixed in lodash trunk, _.toNumber.

Original PR https://github.com/lodash/lodash/pull/1577 Actual merge https://github.com/lodash/lodash/commit/37955345ef3d515e9c2799b82fad545a0c22efce

Xotic750 commented 8 years ago

There are a couple of others that I will try to add, but you should have everything you need.

Xotic750 commented 8 years ago

Changed those to String and 'NaN', hope that helps you a little more. On Node v0.10 and v0.12 the very first test on line 392 is failing to begin with. expect(String(Number('0b12'))).to.equal('NaN'); https://github.com/paulmillr/es6-shim/pull/366/files#diff-cc37320af8aeb8b7582dad90cc114e55R392

ljharb commented 8 years ago

Thanks, this is highlighting a real failure. Can you please rebase these two commits down to one? I'll merge it manually, along with the fix.

Xotic750 commented 8 years ago

There are some other tests that highlight other issues but I guess we can take those separately?

ljharb commented 8 years ago

I can leave this PR open, and you can keep adding commits to it, but if you rebase the existing diff into a single commit, I can cherry-pick it into master.

Xotic750 commented 8 years ago

I can do that when I have a little time (next week), I think some of this affects your https://github.com/ljharb/es-abstract and https://github.com/ljharb/es-to-primitive projects.

ljharb commented 8 years ago

It definitely does - please open issues/PRs there as well when you can :-)

ljharb commented 8 years ago

Turns out to-primitive doesn't have the bug, as it doesn't apply there. es-abstract is now fixed. Thanks!

Xotic750 commented 8 years ago

No problem, I think you still have the badHex issue though, some older environments, node v0.8 accepts +/-Hex value. But you may not be worried about such environments. I think the other (missing tests) are to check whitespace trimming, making sure that the correct ones are stripped.

ljharb commented 8 years ago

@Xotic750 I am indeed worried about those environments - more tests are welcome.