jQuery plugin to ajaxify forms

Question

This is my first foray into the wild world of jQuery plugin development. The plugin "ajaxifies" forms, which isn't particularly spectacular:

• It gets the basics (action, method) from the form itself, serializes the form's fields, and sends the request.
• Users can provide (via its options) callback functions for the local ajax events (beforeSend, complete, success and error).
• The dataType can also be set from a custom data attribute, which - if present - takes precedence.

Everything seems to be working as expected (repo, with a simple demo). Since this is my first plugin, I'd be particularly interested in critiques of its structure. That said, as always in reviews, any aspect of the code is fair game for criticism.

The plugin code:

(function ($) {
 $.fn.ajaxForm = function (options) {
 var settings = $.extend({}, $.fn.ajaxForm.defaults, options);
 return this.each(function () {
 var $form = $(this);
 dataType = $form.attr("data-dataType");
 if (typeof dataType != "undefined") dataType = settings.dataType;
 $form.on("submit", function (event) {
 event.preventDefault();
 $.ajax({
 type: $form.attr("method"),
 url: $form.attr("action"),
 data: $form.serialize(),
 cache: false,
 dataType: dataType,
 beforeSend: function () {
 $form.addClass("ajaxForm-loading");
 if (typeof settings.beforeSend == "function") settings.beforeSend.call($form);
 },
 complete: function () {
 $form.removeClass("ajaxForm-loading");
 if (typeof settings.complete == "function") settings.complete.call($form);
 },
 success: function (response) {
 if (typeof settings.success == "function") settings.success.call($form, response);
 },
 error: function (XMLHttpRequest, textStatus, errorThrown) {
 if (typeof settings.error == "function") settings.error.call($form, XMLHttpRequest, textStatus, errorThrown);
 }
 });
 });
 });
 };
 $.fn.ajaxForm.defaults = {
 dataType: "json",
 beforeSend: function () {},
 complete: function () {},
 success: function () {},
 error: function () {}
 };
})(jQuery);

Initialization:

$(document).ready(function () {
 $("form").ajaxForm({
 dataType: "html",
 success: function (response) {
 console.log(response);
 },
 error: function (XMLHttpRequest, textStatus, errorThrown) {
 console.log("Oops: " + errorThrown);
 }
 });
});

Answer 1

Structure and style looks fine, although I have some general notes:

• Prefer the strong (in)equality operators (!== and ===) whenever possible
• Always use braces, even for one-line "blocks". It's nice to save a few keystrokes, but personally I prefer the consistency of always using braces
• You could skip the defaults for the beforeSend, complete, etc. callbacks and leave them undefined. Defining them as empty functions is fine, but not necessary with the checks you're doing.

In terms of features/behavior:

I'd highly suggest simply modifying the user's settings, and then passing them to $.ajax(), rather than declare a new object. That way, users can set whatever extra ajax options they want, and not see them ignored. In other words, let your plugin fill in the blanks in the settings the user passes in, not the other way around.

Besides that:

• You may want to skip elements that aren't actually forms
• It'd probably be wise to use $form.data("dataType") instead of $form.attr() in case the data type was set using .data() and isn't a real attribute.
• Speaking of data type, you should check if it's a string specifically, since that's the only thing jQuery wants anyway.
• Lastly about the data type: I don't think your plugin should define a "json" default. Better to let the be defined explicitly by whomever is using the plugin, or leave it at jQuery's default.

Most importantly: Please forward any and all arguments to the callbacks. For instance, the regular beforeSend receives the XHR object and its settings, but if I specify a beforeSend using this plugin, I won't get those. Since the name's the same, I'd expect it to receive the same arguments. I like to use beforeSend to grab the XHR so I can use it as a Deferred/Promise object, but I can't do that here.

By the way, it'd be in the spirit of jQuery to invoke the callbacks in the element's context, not in the context of the jQuery wrapper around the element. But if you follow my suggestion of modifying the settings rather than replacing them, be sure to note in your documentation that this context-switching only applies to those four callbacks. E.g. if I declare a dynamic 201 callback to handle the 201 Created status code, it won't be called in the element's context (and I can think of a clean way to make it happen).

You may also want to get some of the form's attributes using .prop() instead of .attr(). For instance, a method attribute is not required (it'll default to "GET" in that case). Also, the accept attribute is worth checking and carrying over to the ajax options, if present.

Here's what I came up with (untested)

(function ($) {
 $.fn.ajaxForm = function (options) {
 options = $.extend({}, $.fn.ajaxForm.defaults, options);
 // A simple helper function
 function callHandler(name, form, args) {
 if(typeof options[name] === "function") {
 options[name].apply(form, [].slice.call(args, 0));
 }
 }
 return this.each(function () {
 var settings,
 form = this;
 settings = $.extend({}, {
 action: $form.prop("action"),
 method: $form.prop("method"),
 accepts: $form.attr("accept"),
 dataType: $form.data("dataType")
 }, options);
 settings.beforeSend = function () {
 $form.addClass("ajaxForm-loading");
 callHandler("beforeSend", form, arguments);
 };
 settings.complete = function () {
 $form.removeClass("ajaxForm-loading");
 callHandler("complete", form, arguments);
 };
 settings.success = function () {
 callHandler("success", form, arguments);
 };
 settings.error = function () {
 callHandler("error", form, arguments);
 };
 $(this).on("submit", function (event) {
 event.preventDefault();
 $.ajax(settings);
 });
 });
 };
 $.fn.ajaxForm.defaults = {};
})(jQuery);

• yannis: Thanks! I've already applied some of your fixes, you can see the result here. The one thing I ignored (for the moment) is $.data(), will need to look into it a bit more (had no idea what it was). When I get around to writing a readme.md, would it be ok to point to your answer from it?
• Flambino: The .data() stuff is an edge case concern, but yes, do look into it (it's useful regardless). The point is just that .data() will transparently read data-* attributes on the element, but it won't write them. See this fiddle. And sure, you can link to this, if you want. Everything we add here is automatically Creative Commons licensed anyway. But do try writing it yourself - it's always a good exercise and sometimes makes you rethink stuff.