added: 'onSet' & 'onRemove' callback support

[MarQuisKnox]

added: 'onSet' & 'onRemove' callback support

[Krinkle]

Hm..

if( typeof options.onSet === 'undefined' ) {
} else {
  if( typeof options.onSet === 'function' ) {
    ..
  }
}

This can just be reduced to:

if( typeof options.onSet === 'function' ) {
  ..
}

Or even:

if( options.onSet ) {
  ..
}

[Krinkle]

You misunderstood the error from Travis (and presumably didn't test the code). !=== does not exist, there is no such operator in the programming language.

== and != are weak equality operators. === and !== are the strict equality operators. Our code quality tool (JSHint) is configured to disallow use of weak equality operators (due to type coercion), and was merely telling you to use !== instead of !=. Not !=== (which doesn't exist). You probably thought we were crazy when you felt the need to do a separate if. I'm happy to inform you, we're not crazy 😄

[Krinkle]

In javascript, parameters are quite flexible. There is no need to declare them in order to pass them as arguments in a custom version of the function (or even at all). When JSHint complained about unused parameters in the default function, the solution is to remove the parameter, not to insert a confusing return statement that uses the variable.

- onSet: function( result ) {}
+ onSet: function() {}

[MarQuisKnox]

The first commit works in the browser: MarQuisKnox@2ce5bdd

The other stuff I just did to try to pass the Travis CI tests. I will look at it again when I have time to install grunt et al locally. No time now. It works in the browser (Waterfox 30) — didn't test any others.

Also, just FYI, I would not have requested a PR on the initial commit if it did not work.

[Krinkle]

@MarQuisKnox I'd recommend going back to that first commit and only updating it to:

• Use === 'function' in place of == 'function'.
• Remove the obsolete !== 'undefined' check (because they're mutually exclusive).
• Put var result; on line 56 right above // Write.

The warning for result was only for setCookie, not removeCookie. In setCookie it's due to scope confusion. Having two variable declarations in one function for the same identifier (eventhough the two statements don't run both) is confusing because javascript doesn't have block scope. The engine unconditionally hoists var out of the conditional block to the top of the function. It thus defined the variable twice. It's confusing to "create" variables like that in a conditional because, while you can write them there (it's valid syntax), they'll be moved at execution time.

[MarQuisKnox]

@Krinkle I just cleaned up the PR. Let me know if anything else needs to be changed. Thanks.

[carhartl]

Thanks for the PR. Though I don't really see the need for this. The cookie operations are not asynchronous. So onSet is basically the same as:

$.cookie('foo', 'bar');
alert('onSet');

If you need it to implement some sort of pub/sub pattern in a larger application this should be implemented right there, since it is too specific a use case for this lightweight plugin:

App.setCookie = function(name, value, options) {
  cookie = $.cookie.apply(null, arguments);
  publish('cookie:set', cookie);
}

(pseudo code)