8

In Google Closure Compiler I get the warning

WARNING - dangerous use of the global this object

Here is an example. The error line and offset refers to the beginning of the word this

function aToggle() {
  if(shown)
    toggle.show()
  else
    toggle.hide()
  $(this).text(shown ? 'Click to hide' : 'Click to show')
  shown = !shown
}
link.onclick = aToggle

I would just change it to an anonymous method, but I am re-using aToggle elsewhere in the file, so it needs to be named.

I could mark aToggle as /**@constructor*/ -- but it is not a constructor. Is there another annotation I can use to eliminate this warning, or am I stuck between marking it as constructor or having a bunch of useless warnings show up?

700 Software
  • 85,281
  • 83
  • 234
  • 341
  • Ok, it's easy to make. Anyhow: `shown` and `toggle` is global? I haven't used Google Closure Compiler, but it probably don't know that your aToggle is an event handler. See if there is somewhere you can tell it that, because then `this` isn't necessary the global `this`. – some Oct 27 '10 at 19:45
  • Yes, I am using aToggle in other places. The above code is just an example I wrote up. – 700 Software Oct 27 '10 at 20:26

3 Answers3

14

Edit: I've been reading Closure: The Definitive Guide, and I just realized that you can simply add the /** @this {Element} */ annotation before your event handler to make Closure Compiler stop complaining.

See the Closure Compiler warning reference. The Closure Compiler gives this warning when you use this within a function that is not either annotated /** @constructor */ or is within the prototype of a class. The compiler assumes that you'll never use this when a function is called in the context of another object (which is what event callbacks do).

Some places that you might have to change to make Closure Compiler stop complaining with this warning:

  • Don't use link.onclick = ... directly, because you have to mess with this and e || window.event. Instead, use jQuery to wrap the event handler, since jQuery's event object has e.currentTarget.
  • If you're using this within a jQuery.each, replace this with the second parameter of your function. E.g., jQuery.each([1, 2, 3], function(i, val) { ... val ... };.
yonran
  • 18,156
  • 8
  • 72
  • 97
1

I don't know JQuery very well but I think you can use something like:

function aToggle(event) {
  if(shown) {
    toggle.show();
  } else {
    toggle.hide();
  }
  $(event.target).text(shown ? 'Click to hide' : 'Click to show');
  shown = !shown;
}

$(link).bind('click', aToggle);

where you retrieve the clicked target from a cross browser generic event object.

EDIT: as a word of advice, use { } with your if else and use semicolons, don't rely on your browser to do it for you.

To make the best use of the closure tools it is advised to use the closure library in combination with the compiler (though not necessary)

Jan
  • 8,011
  • 3
  • 38
  • 60
  • Why do you day that I should use braces and semicolons? – 700 Software Oct 28 '10 at 12:43
  • 'should' is maybe a bit strong but braces really improve readability of programs where it is used to group code. Semicolons: http://stackoverflow.com/questions/444080/do-you-recommend-using-semicolons-after-every-statement-in-javascript . But more importantly, does my solution work for you? – Jan Oct 28 '10 at 13:27
  • I have not tested it yet, I have been moved to other projects but I will not forget to get back to accepting the right answer. I looked at the compiler and it actually strips out the braces and adds the semicolons, which is nice. Thanks for mentioning it. – 700 Software Oct 28 '10 at 14:43
1

First, you are probably doing it wrong. :-)

@Jan had the right idea. But you should probably go with the following:

(function(){
    var toggle = $("#toggle");
    $("#yourLinksID, .orClassName").click(function(e) {
        var shown = toggle.toggle().is(":visible");
        $(this).html(shown ? "Click to hide" : "Click to show");
        e.preventDefault();
    });
}());

and when compiling:

Use the following jQuery externals file that tells Closure Compiler what does what in jQuery: http://code.google.com/p/closure-compiler/source/browse/trunk/contrib/externs/jquery-1.4.3.externs.js

If you just want the warning message to go away replace this with link.

David Murdoch
  • 87,823
  • 39
  • 148
  • 191
  • There is an up to date externs file http://code.google.com/p/closure-compiler/source/browse/trunk/contrib/externs/jquery-1.4.3.externs.js – 700 Software Oct 27 '10 at 20:24
  • I am using aToggle in other places so I can't just replace `this` with `link`. The above code is just an example I wrote up. Thanks for the answer, I will try it soon. – 700 Software Oct 27 '10 at 20:27
  • @GeorgeBailey Ah, I didn't check for an update. It had been so long since the 1.3.2 version I thought it would never be updated. I'll update my answer. – David Murdoch Oct 27 '10 at 20:44