Looking for improvements on my jQuery-UI tagging widget

I am looking for any possible improvements on this tagging widget.

I ask for special attention to be paid to my blur handler in the event of a autocomplete option being specified but any optimizations or critiques are welcome.

Demo: http://webspirited.com/tagit
Js File: http://webspirited.com/tagit/js/tagit.js
Css File: http://webspirited.com/tagit/css/tagit.css

Code:

```
(function($) {
$.widget("ui.tagit", {

// default options
options: {
tagSource: [],
triggerKeys: ['enter', 'space', 'comma', 'tab'],
initialTags: [],
minLength: 1
},

_keys: {
backspace: 8,
enter: 13,
space: 32,
comma: 44,
tab: 9
},

//initialization function
_create: function() {

var self = this;
this.tagsArray = [];

//store reference to the ul
this.element = this.element;

//add class "tagit" for theming
this.element.addClass("tagit");

//add any initial tags added through html to the array
this.element.children('li').each(function() {
self.options.initialTags.push($(this).text());
});

//add the html input
this.element.html('');

this.input = this.element.find(".tagit-input");

//setup click handler
$(this.element).click(function(e) {
if (e.target.tagName == 'A') {
// Removes a tag when the little 'x' is clicked.
$(e.target).parent().remove();
self._popTag();
}
else {
self.input.focus();
}
});

//setup autcomplete handler
this.options.appendTo = this.element;
this.options.source = this.options.tagSource;
this.opti

Cocky comment saying that this is almost max-optimized deserves a harsh review.

I don't know anything about the $.widget, from my brief look of it none of my comments can be ignored because it's "Something you need to do to play nicely with the $.widget".

Admittedly there are no obvious mistakes. Most of it is well structured and the code is readable. There are various nit picks all over the place though. Half of them can be ignored under the motto of that's a style specific comment.

Over all the code was pretty good.

Here are the main points:

Redundant:

this.element = this.element;

That's just plain redundant.

if (lastLi.hasClass('selected'))
     lastLi.removeClass('selected');

jQuery is clever enough to not remove a class that doesn't exist. The hasClass check is just as expensive as the removeClass call.

_popTag: function() {
    return this.tagsArray.pop();
}

That particular abstraction feels redundant when tagsArray is public aswell.

tags: function() {
    return this.tagsArray;
}

again it feels like a redundant abstractions when tagsArray is public. You do know tagsArray is public right?

HTML Strings versus DOM manipulation

this.element.html('');

As mentioned in the comments I don't like messing with HTML directly. Use DOM manipulation methods instead.

this.input = $(document.createElement("input"));
this.input.addClass("tagit-input");
this.input[0].type = "text";
var li = $("");
li.addClass("tagit-new");
li.append(this.input);
this.element.empty();
this.element.append(li);

This can actaully be improved on with jQuery 1.4

this.input = $(document.createElement("input"), {
     "class": "tagit-input",
     "type": "text"
});
var li = $(document.createElement("li"), {
     "class": "tagit-new"
});
this.element.empty().append(li.append(this.input));

=== vs ==

if (e.target.tagName == 'A')

Always use === end of.

name duplication

this.options.appendTo = this.element;
this.options.source = this.options.tagSource;

Why do this? Why have the same thing mapped under different names? It's just hard to read and makes maintenance more of a pain. Also harder to refactor.

Return from a function or use if-else

if (e.which == self._keys.backspace)
     return self._backspace(lastLi);
if (self._isInitKey(e.which)) {

Might as well use else if here instead of returning. Avoid returning multiple times. There are some exceptions like guard statements or trivial functions.

Code smells

self.lastKey = e.which;

Storing the last key that was pressed? That smells. Refactor or abstract this away.

_removeTag: function() {
    this._popTag();
    this.element.children(".tagit-choice:last").remove();
}

You have both a remove and pop function. Refactor this so you only have one.

Caching

self._addTag($(this).val());
$(this).val('');

Really aught to cache it var $this = $(this). Get into this habit some calls to $ are really expensive.

Extending native prototypes

String.prototype.trim

Don't do it. Simple as. You'll break everyone else's code. The problem is code like this:

for (var c in "foo") {
     console.log(someString[c]); 
}
"f"
"o"
"o"
"function() { ... "

This is why we check for hasOwnProperty in for in loops. Personally I prefer to just assume no-one extends native prototypes and shout anyone that does that rather then use hasOwnProperty. Not to mention there's lots of 3rd party code you use that doesn't check for it.

if (x) return true

if (this.tagsArray.length == 0 || $.inArray(value, this.tagsArray) == -1)
     return false;
return true;

Ignoring the fact that if statements without brackets make me rage. This really aught to be

return this.tagsArray.length !== 0 && $.inArray(value, this.tagsArray) !== -1;

widget destroy function

destroy: function() {
    $.Widget.prototype.destroy.apply(this, arguments); // default destroy
    this.tagsArray = [];
}

The reason the widget makes call to .destroy is because there is no garbage collector for the DOM. Your supposed to clean up the dom. Your supposed to remove this.element, detach any event handlers. And whatever else needs cleaning up.

You do not need to clean up javascript. JS has a garbage collector.

Here is the full code annotated:

just ctrl-f //*

```
(function($) {
$.widget("ui.tagit", {

// default options
options: {
tagSource: [],
triggerKeys: ['enter', 'space', 'comma', 'tab'],
initialTags: [],
minLength: 1
},

_keys: {
backspace: 8,
enter: 13,
space: 32,
comma: 44,
tab: 9
},

//initialization function
_create: function() {

var self = this;
this.tagsArray = [];

//store reference to the ul
//* WTF? Seriously? you know theres only one this object right?

StackExchange Code Review Q#824, answer score: 5