21

What are the JQuery bad/worst practices you have seen, written or thing should be avoided?

Damjan Pavlica
  • 31,277
  • 10
  • 71
  • 76
piyer
  • 746
  • 4
  • 11
  • 1
    I know I already commented, but this question is an exact duplicate: http://stackoverflow.com/questions/1229259/jquery-pitfalls-to-avoid – Doug Neiner Jan 15 '10 at 08:08

4 Answers4

33

A thing you should avoid is using the "easy to use" selectors in every line once again, because the JavaScript implementation of selectors is not that efficient. Of course, the jQUery guys are optimizing it, however I think you should use it as little as possible.

So, this is a bad practice.

$("li ul span").show();
$("li ul span").toggleClass("bubu");

Chaining is good

$("li ul span").show().toggleClass("bubu");

And remembering things in a local variable is also not bad:

var allspans = $("li ul span");
allspans.show();
allspans.toggleClass("bubu");
naivists
  • 32,681
  • 5
  • 61
  • 85
17

There are two that I see alot:

First, in a click event, the id is accessed like this:

$("a").click(function(e){
   var id = $(this).attr('id');
});

That creates a new jQuery object around the DOM node, and calls a function. The following is the correct way:

$("a").click(function(e){
   var id = this.id;
});

Note: That you will also see $(this).attr('href'), but that is correct because of the way jQuery normalizes it across browsers.

The second is passing anything except a DOM node into the scope parameter of the jQuery call:

$(".child", $(".parent")).doSomething();
// or
$(".child", ".parent").doSomething();

There is no speed gain at all by doing this. Where you do see a speed increase, is when you already have the DOM element:

$('div').click(function(){
   $('img', this).doSomething(); // This is good
});
Doug Neiner
  • 65,509
  • 13
  • 109
  • 118
5

James Padolsey has written an excellent article on jQuery code smells. I recommend reading it.

ndnenkov
  • 35,425
  • 9
  • 72
  • 104
Phil.Wheeler
  • 16,748
  • 10
  • 99
  • 155
2

Still using the old document ready function:

$("document").ready(function(){ });

Instead of the very common:

$(function(){ });

It's not really bad, but I shows people not getting up with new api's.

jerone
  • 16,206
  • 4
  • 39
  • 57
  • 13
    Using `$(document).ready(...)` is often used to make the code explicitly clear. Of course it is not needed, but its hardly a bad practice IMO. – Doug Neiner Jan 15 '10 at 07:49
  • 1
    can you explain, why should I do that? I think, the first version makes more sense - we're attaching an event handler to the DOCUMENT object. The other one.. even tho it maybe works better.. does not explain itself. – naivists Jan 15 '10 at 08:08
  • In that case I reference too http://docs.jquery.com/Core/jQuery#callback Like I said it's not bad practice, it's more a notice that people don't catch up on the latest api. They prefer to make readable code, instead of fast and minified code. – jerone Jan 15 '10 at 12:51