What are your guys thoughts on the following jQuery plugin?
$.fn.with = function( fn ) {
// Verify function
if( jQuery.isFunction( fn ) ) {
var temp = fn.call(this);
// Check fn for valid response
if( temp instanceof jQuery ) {
return temp;
}
}
return this;
};
I believe this method to very useful as it allows for better chaining, thus increasing flow. Before developing this method, I found myself creating a lot of temporary variables just to preform a conditional test.
Examples would be in cases where .hasClass
and .is()
are used:
// Example From: http://api.jquery.com/is/
$("ul").click( function( event ) {
var $target = $(event.target);
if ( $target.is("li") ) {
$target.css("background-color", "red");
}
});
However, using the proposed .with()
method, the code can become cleaner:
$("ul").click( function( event ) {
$(event.target).with(function() {
if( this.is("li") ) {
this.css("background-color", "red");
}
})
});
Note: If the function processed by .with()
returns an instance of jQuery it, instead of the calling instance, will be returned into the chain.
-
2\$\begingroup\$ The resulting code is an order of magnitude uglier, even taking an extra indentation level. \$\endgroup\$Esailija– Esailija2013年07月20日 16:19:08 +00:00Commented Jul 20, 2013 at 16:19
-
\$\begingroup\$ I would suggest you submit that to jQuery. It seems like a viable method and might be added to the core. \$\endgroup\$Jonny Sooter– Jonny Sooter2013年07月30日 22:20:23 +00:00Commented Jul 30, 2013 at 22:20
-
\$\begingroup\$ Do you know what the process is for that or a site for reference? I've not found anything at www.jquery.com. \$\endgroup\$roydukkey– roydukkey2013年08月16日 15:43:12 +00:00Commented Aug 16, 2013 at 15:43
1 Answer 1
The code
I'm not quite sure of the purpose of this part:
if( temp instanceof jQuery ) {
return temp;
}
This seems to be related to the idea of chaining methods, but it prevents me from doing something like this:
var width = $('ul').show().with(function() {
return this.width();
}
Also, when chaining, there is no benefit of only returning the result if it is an instance of jQuery, because if you try to chain methods and you return nothing from with with()
, it'll result in "undefined is not a function".
You also check if the fn
passed to with()
actually is a function. I wouldn't do that, since this will silently ignore a wrong parameter to with()
, which is very hard to debug. Instead you can just disable the check, or even better do something like this:
if(jQuery.isFunction(fn)) {
[...]
} else {
throw "Parameter passed to `with()` was not a function: " + fn;
}
The idea in general
I have to side with @Esailija here. While there won't be a noticeable performance difference in any modern browser (if you omit the instanceof
check), I can't see the benefit of replacing a local variable with an extra level of unnecessary indentation. But that is obviously a purely personal opinion.
Aside from the extra level of indentation, I find the code using with()
harder to read, since it removes context from the individual lines. I would prefer something like
$targetEl.css("background-color", "red");
to
this.css("background-color", "red");
any day, because in the former I don't have to scan the surrounding code to see what "this" currently means (which can be hard in JavaScript). Of course this is no concern in a snippet as short as your examples.