infinitered / ramdasauce

Ramda smothered in saucy helpers.
69 stars 4 forks source link

Move isNumber helper inside of isWithin body #7

Closed hubciorz closed 7 years ago

hubciorz commented 7 years ago

This may look like a very subtle change but in fact it resolves some issues while running a bundle built with Webpack. As Webpack builds inject polyfills during runtime, in might also affect some base types, like Number. In this particular situation, isNumber helper always returned false on some older platforms (like iOS 8 or Android 4.4-ish) because its prototype chain had been changed during runtime. In other words, it tried to compare the old constructor of a type with the new one after applying the polyfills.

I found this bug while working with https://github.com/skellock/apisauce in an app of mine. I was always getting "UNKNOWN_ERROR" while sending any request, because of the issue I described above (As far as I remember, in200s function from apisauce also returns false, too)

See: http://stackoverflow.com/questions/19683444/why-does-changing-the-prototype-of-a-function-affect-instanceof-for-objects-a https://github.com/ramda/ramda/blob/master/src/is.js

skellock commented 7 years ago

Ok, this is nightmare inducing. How did you ever track this down!?

I award you 1 gold star.

hubciorz commented 7 years ago

@skellock

How did you ever track this down!?

As I said, I faced a problem with apisauce which depends on ramdasauce. It was like a week after I realized what the hell was going on with that "UNKNOWN_ERROR" thing. And then I just debugged the library step-by-step.

Btw, you may also need to update apisauce's dependencies I guess

skellock commented 7 years ago

Roger that. Thx again. I'll push both builds out later today.

hubciorz commented 7 years ago

No problem :)