search

cujojs / when

A solid, fast Promises/A+ and when() implementation, plus other async goodies.
Other
3.44k stars 396 forks source link

Fix console for ie8 #436

Closed unional closed 9 years ago

unional commented 9 years ago

IE8 typeof returns "object" instead of "function", so checking for "function" doesn't work.

scothis commented 9 years ago

this is one of my personal favorite WTFIEs

briancavalier commented 9 years ago

@unional Thanks! Is it safe to use typeof localConsole.log !== 'undefined' ?

sompylasar commented 9 years ago

There are more issues that may arise in IE8. Just accessing a property on the console object may throw an exception (e.g. typeof console.log may throw). Everything around the console object, including property access and method calls, should be wrapped in try..catch blocks.

unional commented 9 years ago

@briancavalier I think it is ok to do typeof localConsole.log !== 'undefined', since this really only affect ie <=8 and quirks mode. I will update the code accordingly. Also, I'll update the code to better support scenario without JSON. Currently, it will throw error in ConsoleReporter.log.

briancavalier commented 9 years ago

@unional when.js 3.x requires an ES5 environment, and since JSON is part of the ES5 spec, we can assume it's there and works. Folks needing to support older browsers can polyfill to ES5.

In contrast, console isn't part of EcmaScript. So, I'm cool with jumping through a few more hoops to make sure we're not breaking envs that have console quirks, like IE.

briancavalier commented 9 years ago

@unional looking good. If you squash the commits, I'll get this merged asap!

briancavalier commented 9 years ago

@unional Ping. I'm happy to merge this if you'll squash the commits. Thanks!

unional commented 9 years ago

@brian, sorry for the late reply. Have been busy lately. I have attempted to squashed the commits. I don't know if I'm doing it correctly. If I wasn't, please let me know and I will redo it.

Thanks,

On Mon, Mar 23, 2015 at 5:24 AM, Brian Cavalier notifications@github.com wrote:

@unional https://github.com/unional Ping. I'm happy to merge this if you'll squash the commits. Thanks!

— Reply to this email directly or view it on GitHub https://github.com/cujojs/when/pull/436#issuecomment-84970912.

briancavalier commented 9 years ago

@unional Thanks. There's a merge commit in the mix now, unfortunately. No worries, though, I'll merge your change in 168a26d manually.

briancavalier commented 9 years ago

@unional Sorry for letting this sit for a while! I just merged your change into master in bfee33d. I'll pull together a new release later this week.

Thanks again!