Skip to content

Commit

Permalink
Improve console method stubbing
Browse files Browse the repository at this point in the history 
Avoid browsers with `console` and `console.log` throwing errors for some
of the other methods that they did not support.

Fix gh-1206
Close gh-1218
  • Loading branch information
@devinrhode2 @necolas
devinrhode2 authored and necolas committed Sep 22, 2012
1 parent e1b1630 commit 578f377
Showing 1 changed file with 16 additions and 11 deletions.
27 changes: 16 additions & 11 deletions js/plugins.js
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
// Avoid `console` errors in browsers that lack a console.
if (!(window.console && console.log)) {
(function() {
var noop = function() {};
var methods = ['assert', 'clear', 'count', 'debug', 'dir', 'dirxml', 'error', 'exception', 'group', 'groupCollapsed', 'groupEnd', 'info', 'log', 'markTimeline', 'profile', 'profileEnd', 'table', 'time', 'timeEnd', 'timeStamp', 'trace', 'warn'];
var length = methods.length;
var console = window.console = {};
while (length--) {
console[methods[length]] = noop;
}
}());
}
(function() {
var noop = function noop() {};
var methods = [
'assert', 'clear', 'count', 'debug', 'dir', 'dirxml', 'error',
'exception', 'group', 'groupCollapsed', 'groupEnd', 'info', 'log',
'markTimeline', 'profile', 'profileEnd', 'table', 'time', 'timeEnd',
'timeStamp', 'trace', 'warn'
];
var length = methods.length;
var console = window.console || {};

This comment has been minimized.

Sign in to view

Copy link
@mathiasbynens

mathiasbynens Sep 22, 2012

Member

The reason I was using var console = window.console = {}; is to make sure a global console object would be available, even in browsers that lack one.

Unless I’m missing something, the current code doesn’t expose console to the global scope at all.

/me reads gh-1218…

This comment has been minimized.

Sign in to view

Copy link
@bgrins

bgrins Oct 2, 2012

Member

I agree with @mathiasbynens here. This doesn't actually expose console to browsers without it available. I modified the code to read:

if (!window.console) {
    window.console = { };
}
var console = window.console;

This makes it so that a leftover console.log in code doesn't break stuff in IE without the developer tools open.

This comment has been minimized.

Sign in to view

Copy link
@drublic

drublic via email Oct 2, 2012

Member

while (length--) {
// Only stub undefined methods.
console[methods[length]] = console[methods[length]] || noop;

This comment has been minimized.

Sign in to view

Copy link
@drublic

drublic Sep 22, 2012

Member

Using || is slower than using if here. Please see this test.
Also I like using if better, because it's simple for beginners to understand it.

This comment has been minimized.

Sign in to view

Copy link
@mathiasbynens

mathiasbynens Sep 22, 2012

Member

What @drublic said. This is basically http://jsperf.com/conditional-assignment.

I’d add var method = methods[length] and re-use that to improve readability and save some property lookups while we’re at it.

This comment has been minimized.

Sign in to view

Copy link
@devinrhode2

devinrhode2 Oct 8, 2012

Author Contributor

+1 var method = methods[length]
}
}());

// Place any jQuery/helper plugins in here.

1 comment on commit 578f377

@necolas
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to patch

Please sign in to comment.