code-chunks / angular2-logger

A log4j inspired logger for angular 2.
MIT License
144 stars 41 forks source link

IE 10 error on console.debug.apply #62

Closed philjones88 closed 8 years ago

philjones88 commented 8 years ago

Just thought I'd test my NG2 app works in IE10 and I hit this stack trace:

Error: Uncaught (in promise): EXCEPTION: Error in ./AppComponent class AppComponent_Host - inline template:0:0
ORIGINAL EXCEPTION: TypeError: Unable to get property 'apply' of undefined or null reference
ORIGINAL STACKTRACE:
TypeError: Unable to get property 'apply' of undefined or null reference
   at debug (http://localhost/static/ng2/js/main.bundle.js:23918:35)
   at StorageService (http://localhost/static/ng2/js/main.bundle.js:23736:10)
...... lots of stack trace....
(http://95818bd8.ngrok.io/static/ng2/js/vq.polyfills.bundle.js:15998:27)
   at invoke (http://localhost/static/ng2/js/polyfills.bundle.js:15950:23)

It points to:

        Logger.prototype.debug = function (message) {
            var optionalParams = [];
            for (var _i = 1; _i < arguments.length; _i++) {
                optionalParams[_i - 1] = arguments[_i];
            }
            this.isDebugEnabled() && console.debug.apply(console, arguments);
        };

Which I guess is this in the source:

https://github.com/code-chunks/angular2-logger/blob/master/app/core/logger.ts#L75

Does this library need to do some kind of guards for older IE 10 browsers?

philjones88 commented 8 years ago

Ended up adding a quick hack to my code to use native, if not use log, if not nothing.

// Shim support for console for older browser trying to fall back to log then to nothing
window.console.log = window.console.log || function() { };
window.console.log.apply = window.console.log.apply || function() { };
window.console.debug = window.console.debug || window.console.log || function() { };
window.console.debug.apply = window.console.debug.apply || window.console.log.apply || function() { };
window.console.info = window.console.info || window.console.log || function() { };
window.console.info.apply = window.console.info.apply || window.console.log.apply || function() { };
window.console.warn = window.console.warn || window.console.log || function() { };
window.console.warn.apply = window.console.warn.apply || window.console.log.apply || function() { };
window.console.error = window.console.error || window.console.log || function() { };
window.console.error.apply = window.console.error.apply || window.console.log.apply || function() { };
langley-agm commented 8 years ago

Hi @philjones88 ,

I don't have a PC with IE 10 atm but I tried console.debug using IE 11 in IE 10 mode and it was working fine. Are you sure you were using IE 10? were you running it in an older version mode?

I haven't been able to reproduce it but I'll try to find a way to test it in a real IE 10 browser. Any clues or extra info would be appreciated.

philjones88 commented 8 years ago

Hi,

The issue only shows in IE10 not IE11 set to IE10 compat mode. Technically IE10 is now end of life but we have a customer with a few users which is frustrating, IE6 all over again!

Microsoft give away free VMs for testing IE versions:

https://developer.microsoft.com/en-us/microsoft-edge/tools/vms/

Sent from my iPhone

On 22 Aug 2016, at 19:00, Armando Garcia Moran notifications@github.com<mailto:notifications@github.com> wrote:

Hi @philjones88https://github.com/philjones88 ,

I don't have a PC with IE 10 atm, but I tried console.debug using IE 11 in IE 10 mode and it was working fine. Are you sure you were using IE 10? were you running it in an older version mode?

I haven't been able to reproduce it but I'll try to find a way to test it in a real IE 10 browser. Any clues or extra info would be appreciated.

You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/code-chunks/angular2-logger/issues/62#issuecomment-241496637, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AApVL_P0sSrbPbYqkpnKmX1nYtLKHXj_ks5qieOhgaJpZM4Jp0sO.

langley-agm commented 8 years ago

IE6 all over again!

I feel you !

Thanks for the link, I'm actually downloading one of those right now to see if I can reproduce it there.

philjones88 commented 8 years ago

Thinking back to NG1. The $log code might be a good use for inspiration. Don't have any issues with the NG1 parts of our app and IE10.

Sent from my iPhone

On 22 Aug 2016, at 19:24, Armando Garcia Moran notifications@github.com<mailto:notifications@github.com> wrote:

IE6 all over again!

I feel you !

Thanks for the link, I'm actually downloading one of those right now to see if I can reproduce it there.

You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/code-chunks/angular2-logger/issues/62#issuecomment-241503724, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AApVL9xjtthpxBh_U486W-c_EaN-n-wlks5qiekYgaJpZM4Jp0sO.

langley-agm commented 8 years ago

I was able to reproduce it using a VM.

I don't feel comfortable adding a shim as part of the library code, this should be a user's decision. But I think I can do the following:

A different approach might be to use a global setting that the user can turn on specifying to use console.log as the default console.debug function whenever the latter is missing. I'll sleep on it.

Let me know your thoughts @philjones88 .

philjones88 commented 8 years ago

There are a few shim's available I could find:

https://github.com/paulmillr/console-polyfill

and

https://github.com/kayahr/console-shim

But I couldn't get either of these to work with my scenario of Angular2 + Webpack etc. I didn't try super hard but it wasn't simple for some reason.

I do think for someone coming from AngularJS 1.x it seems strange that a logging library wouldn't handle the browser differences magically.

If you think back to $log in AngualrJS 1.x it seems to handle the console not being there and not require any action or additional setup from the user

https://github.com/angular/angular.js/blob/master/src/ng/log.js#L139

I'd therefore vote in favour of borrowing how $log worked with console?

philjones88 commented 8 years ago

Searching around a few of the seed project (e.g. angularclass's seeds) have reference (but not always enabled) ie-shim

https://github.com/AngularClass/angular2-webpack-starter/blob/master/src/polyfills.browser.ts#L3

https://github.com/AngularClass/angular2-seed/blob/master/src/dll.ts#L4

And the shim:

https://github.com/gdi2290/ie-shim/blob/master/index.js

This seems to shim console, maybe the solution is to test if the shim is compatible with say RC4/RC5 and then simply update the README to say use this shim if you want to support IE <= 10?

langley-agm commented 8 years ago

Angular 1 and Angular 2 are two completely different approaches. Angular 2 also requests the user to add the required shim/polyfills manually.

I am not going to modify the default console implementation. What If the user has implemented its own debug method into it?

The second approach I mentioned requires no extra setup from the user.

Thanks for the links, I'll take a look at them.