views:

113

answers:

3

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

Now 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?

+1  A: 

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
Why do you day that I should use braces and semicolons?
cjavapro
'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
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.
cjavapro
+1  A: 

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
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
cjavapro
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.
cjavapro
@David Murdoch: Again? ;)
some
@cjavapro 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
+2  A: 

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