1

The IDE I use (Cloud 9) incorporates JSHint by giving me a warning when it detects something wrong. As a principle I try to correct my code to rectify these errors. There is one that I have now which I can't see a way around:

$("#component-manager tr:not(.edit-component) td").on("click", function() {
    if ($(this).index() !== 0) {
        var checkbox = $(this).parent().find("input[name=component-selected]");
        checkbox.trigger("click");
    }
});

This code gives me a warning: "Use of 'this' in callback function".

I have two questions:

  1. What should I do to improve my code and get rid of this warning.
  2. Why is it better practice (for that is what JSHint tries to promote) to not use this in callback functions?
Cilan
  • 10,658
  • 3
  • 31
  • 51
microbug
  • 175
  • 1
  • 2
  • 16
  • I'd consider that a bug or a misconfiguration in the IDE. Using `this` in a jQuery callback like that is explicitly and enthusiastically encouraged. – Pointy Feb 16 '14 at 21:54
  • JShint does not give me any warnings or errors. – Kyle Needham Feb 16 '14 at 21:54
  • 1
    you could use `function(e){ $(e.target) ...` instead of `function(){ $(this) ...` – lordvlad Feb 16 '14 at 21:55
  • The only thing you might do to improve your code is to cache the jQuery object: `var $this = $(this);`. It won't get rid of the error but it's considered good practice particularly if you're repeatedly making the same selection again and again. Probably not something you need here, but something to consider for future code. – Andy Feb 16 '14 at 22:00

3 Answers3

3

There is nothing wrong with your code, nor any "better" way to code it. This is just something that jsHint does, perhaps only because some people make mistakes with the use of this in other types of callbacks.

There is a comment you can insert into your code that will tell jsHint to ignore this particular use. I will go look and see if I can find it and add it to my answer if I find it. You can use the validthis option for jsHint: http://www.jshint.com/docs/options/#validthis to ignore this warning for a specific function.

I haven't tried it myself, but according to the documentation, you can add this to the function:

/* jshint validthis: true */

Or, a configuration file can be used with jshint that changes the default for this option (you'd have to figure out how to do that within your IDE).

jfriend00
  • 580,699
  • 78
  • 809
  • 825
1

There's nothing wrong with that code, your IDE just isn't smart. In my IDE, a function always returns something, but since I use a conditional if and else, it gave me a warning that maybe nothing will be returned and to add 'void' (Using XCode). Do what I do, and ignore it.

If it bugs you extremely, and this only happens when you use this, you could do what lordvlad says and use function(e){ and replace $(this) with e.target.

Cilan
  • 10,658
  • 3
  • 31
  • 51
  • I am now doing what you suggested. It doesn't bug me much but as suggested by @OregonTrail I think it is a better way of going about things. – microbug Feb 16 '14 at 22:11
  • @jfriend00 I suppose it is a less efficient mechanism: I can live with the error! (changing answer...) – microbug Feb 16 '14 at 22:13
1

This comes from the commonly mis-implemented this that pattern. See this question for an example.

There are usually better ways to get a reference to the object in question. In this case you'll want to declare an argument to your callback function and refer to the event target explicitly.

$("#component-manager tr:not(.edit-component) td").on("click", function(event) {
    if ($(event.target).index() !== 0) {
        var checkbox = $(event.target).parent().find("input[name=component-selected]");
        checkbox.trigger("click");
    }
});
Community
  • 1
  • 1
OregonTrail
  • 7,245
  • 4
  • 36
  • 53