views:

1677

answers:

9

I have a bunch of console.log calls in my JavaScript.

Should I comment them out before I deploy to production?

I'd like to just leave them there so I don't have to go to the trouble of removing the comments later on if I need to do any more debugging. Is this a bad idea?

+3  A: 

You should at least create a dummy console.log if the object doesn't exist so your code won't throw errors on users' machines without firebug installed.

Another possibility would be to trigger logging only in 'debug mode', ie if a certain flag is set:

if(_debug) console.log('foo');
_debug && console.log('foo');
Christoph
+44  A: 

It will cause Javascript errors, terminating the execution of the block of Javascript containing the error.

You could, however, define a dummy function that's a no-op when Firebug is not active:

if(typeof console === "undefined") {
    console = { log: function() { } };
}

If you use any methods other than log, you would need to stub out those as well.

Jason Creighton
Thanks. That looks like a nice workaround. Let me see if I understand it. You're saying if the console is undefined, then set the console to an empty function. I don't understand the colon syntax after 'log'. What does that do and why is everything inside braces after "console=" ?
Charlie Kotter
The braces define an object literal: https://developer.mozilla.org/en/Core_JavaScript_1.5_Guide/Literals#Object_Literals and the "function() {}" bit defines an anonymous function that takes no arguments and does nothing. (It still works to call "console.log(foo)" with an argument because JS doesn't care if you call a function with too many or too few arguments.)
Jason Creighton
Understood. Thanks for the link too.
Charlie Kotter
Excellent solution man!! :-) I just had this problem the other day. My page was working working in FF only when FireBug was on and it would randomly halt on certain actions in IE. I was like WTF!?! And, then I realized I had the console.log calls still in there and I had to go through and remove them. I will likely add your solution to my main JS file now and never have to worry about that unfortunate scenario again.
KyleFarris
A: 

Agree with Cristoph, for Safari it won't be a problem but in other browsers it will throw errors and might stop javascript to continue. In general not a good idea...

Sinan Y.
FF has the same problem: not everyone has firebug installed!
Christoph
You're right, edited...
Sinan Y.
+15  A: 

As others have already pointed it, leaving it in will cause errors in some browsers, but those errors can be worked around by putting in some stubs.

However, I would not only comment them out, but outright remove those lines. It just seems sloppy to do otherwise. Perhaps I'm being pedantic, but I don't think that "production" code should include "debug" code at all, even in commented form. If you leave comments in at all, those comments should describe what the code is doing, or the reasoning behind it--not blocks of disabled code. (Although, most comments should be removed automatically by your minification process. You are minimizing, right?)

Also, in several years of working with JavaScript, I can't recall ever coming back to a function and saying "Gee, I wish I'd left those console.logs in place here!" In general, when I am "done" with working on a function, and later have to come back to it, I'm coming back to fix some other problem. Whatever that new problem is, if the console.logs from a previous round of work could have been helpful, then I'd have spotted the problem the first time. In other words, if I come back to something, I'm not likely to need exactly the same debug information as I needed on previous occasions.

Just my two cents... Good luck!

Chris Nielsen
+1 It's just unprofessional not to.
annakata
the problem is often during the production cycle as something is being debugged or tested, people view it outside of the development team who do not have a dev console. my personal preference is not to have the app die on them so we don't get calls like 'your new code does nothing for me'.
Dimitar Christoff
+3  A: 

Hope it helps someone--I wrote a wrapper for it a while back, its slightly more flexible than the accepted solution.

Obviously, if you use other methods such as console.info etc, you can replicate the effect. when done with your staging environment, simply change the default C.debug to false for production and you won't have to change any other code / take lines out etc. Very easy to come back to and debug later on.

var C = {
    // console wrapper
    debug: true, // global debug on|off
    quietDismiss: false, // may want to just drop, or alert instead
    log: function() {
        if (!C.debug) return false;

        if (typeof console == 'object' && typeof console.log != "undefined") {
            console.log.apply(this, arguments); 
        }
        else {
            if (!C.quietDismiss) {
                var result = "";
                for (var i = 0, l = arguments.length; i < l; i++)
                    result += arguments[i] + " ("+typeof arguments[i]+") ";

                alert(result);
            }
        }
    }
}; // end console wrapper.

// example data and object
var foo = "foo", bar = document.getElementById("divImage");
C.log(foo, bar);

// to surpress alerts on IE w/o a console:
C.quietDismiss = true;
C.log("this won't show if no console");

// to disable console completely everywhere:
C.debug = false;
C.log("this won't show ever");
Dimitar Christoff
+4  A: 

If you have a deployment script, you can use it to strip out the calls to console.log (and minify the file).

While you're at it, you can throw your JS through JSLint and log the violations for inspection (or prevent the deployment).

This is a great example of why you want to automate your deployment. If your process allows you to publish a js file with console.logs in it, at some point you will do it.

Nosredna
Good point .
Charlie Kotter
+2  A: 

Figured I would share a different perspective. Leaving this type of output visible to the outside world in a PCI application makes you non-compliant.

jm04469
Never would have occurred to me. Thanks.
Charlie Kotter
A: 

this seems to work for me...

if (!window.console) {
    window.console = {
        log: function () {},
     group: function () {},
     error: function () {},
     warn: function () {},
     groupEnd: function () {}
    };
}
nick fox
+1  A: 

A nice one-liner:

(!console) ? console.log=function(){} : console.log('Logging is supported.');
Pedro