PR: Promises Implementation

[ydaniv]

This is a first attempt. Still not quite sure about API design.

What's in there:

• velocity.defaults.promise: false by default and if set to true it will always return a promise.
• options.promise: if set to true then a promise is returned for that call.
• Works with basic $.fn.velocity() calls and also using an action, e.g. slideUp.
• Bad calls to $.fn.velocity() return a rejected promise.
• Works with native Promises by default. Can be overridden by setting velocity.Promise to a constructor that behaves the same.
• All resolved and rejected promises return the call's context elements as value/reason.

Example: http://jsfiddle.net/faa8e/

Not in there:

• Support for progress notifications.
• jQuery.Deferred() integration.

[domenic]

Rejection reasons should always be Error instances, just like thrown exceptions.

The fact that you can turn on promises library-wide, instead of having to do { promise: true } on every call, is very nice. As long as this library is exposed as a jQuery plugin, that might be the best compromise between respecting jQuery users' expectations, and giving easy promise support.

+1 on bad calls returning rejected promises (instead of e.g. throwing); that is how it should work.

[ydaniv]

@domenic thanks!

Rejection reasons should always be Error instances, just like thrown exceptions.

Agree. My intention was to allow the user to continue chaining from the rejection handler, but following the spec is better.

I'm still thinking we should make the default promise : true so users can opt out of it changing defaults or by passing promise : false.

@julianshapiro ?

[mikaelkaron]

I'm torn on promise : true by default. In a non jQuery world this would be awesome, in a jQuery world you'd expect jQuery style chaining.

Would it be possible to keep it true as long as we depend on jQuery and then false when we don’t?

[domenic]

Make sure to return the promise object, not a deferred. That is, there should be no resolve or reject methods.

[ydaniv]

@mikaelkaron jQuery's .animate() API actually has done, fail properties in the options object, but then you're back to using callbacks. It's possible though to keep it false when using jQuery/Zepto.

Moreover, IMHO it looks like you wouldn't want to touch the DOM after you start an animation, and actually rather, in most cases, defer actions till the end.

@domenic but of course. Now I'm returning the result of new Promise(), assuming that we're using the native implementation.
What I did was make the scope of the .velocity() call a deferred, so the resolve and reject methods are scoped variables inside it.

[julianshapiro]

@domenic, @mikaelkaron, @ydaniv, @Hiswe.

Thank you so much everyone. It is exciting to see this implemented so thoughtfully.

Three responses:

1. Setting promise: true globally by default, and breaking the jQuery call stack everywhere, would be a very bad idea. However, as mentioned, I don't mind breaking the chain when users explicitly set promise: true because -- if they're doing this -- they're an advanced user and can deal with the repercussions appropriately.

2. Conversely, setting promise: true globally when jQuery isn't present on the page, does make a lot of sense. (Great idea, @mikaelkaron.) As it stands, when jQuery isn't present, the targeted element set is returned, which is only partially valuable: While you could technically chain onto the returned value, you'd be limited to raw DOM properties/methods; you couldn't chain a new Velocity call on since Velocity doesn't extend the HTMLElement object.

(Although certainly a tangential point, does anyone have additional thoughts on this last point? Where might not returning the element set in a non-jQuery environment harm the user's workflow?)

[mikaelkaron]

Hang on.. Am I crazy when I remember a .promise() in jQuery that you can call to get something promise like back from .animate()?

I don't think I am as http://api.jquery.com/promise/ is there.

Could we make use of it?

And by extension... if we're using the jQuery .queue(), does that mean that we already have promises? Reading from the docs:

Return a Promise object to observe when all actions of a certain type bound to the collection, queued or not, have finished.

I wonder if .animate() fiddles with that or if .queue() does it...

[ydaniv]

@mikaelkaron that's brilliant! I never knew you can just do .promise() on a simple jQuery selection. Probably since Velocity is built on $.queue() we get this OOTB, so in case we find a framework I'm setting the default to promise : false.

There's still a problem with Zepto not having a deferred/promise implementation, but I think we can live with that.

Of course, we still check for native-like Promise object on the window so it's easy to polyfill.

The rejected handler is sending a new Error() as per specs.

@julianshapiro I think that wraps it up.

[mikaelkaron]

As a bonus, I would not be surprised if this would allow you to stack velocity and jQuery animations after each other (and probably anything pushed on the fx queue).

Before we merge this I was hoping to hear from @domenic (or as we call him here, Mr Promise) about having $el or equiv in non-jQuery world passed as a fulfilment value. I think this may be important in the future.

PS. Come to think of it, you could produce some really funky stuff by combining promises from various queues in jQuery to coordinate animation by just using something like when.all or the somewhat equiv $.when.

[domenic]

I don't really see any downside to having the element (or $element in the jQuery case) passed as the fulfillment value, although in most cases it will be useless. Perhaps it would make the most sense to stick with non-$ element in both cases, to avoid surprising inconsistency?

And yes, using Promise.all to coordinate animations is a pretty common technique :)

[lozandier]

@mikaelkaron Technically you could stack velocity and jQuery animations since you can cast jQuery Deferreds with Promise.resolve() if necessary, AFAIK.

I think jQuey animations are already thenable, so you don't have to worry about it as much...

[mikaelkaron]

Btw. Ran across this in my rss feed today, 2K compressed promises anyone?

[julianshapiro]

Great find. We can recommend this in the docs. By the way, promises will be implemented very shortly.

Sent from my phone.

On Jun 16, 2014, at 5:38 PM, Mikael Karon notifications@github.com wrote:

Btw. Ran across this in my rss feed today, 2K compressed promises anyone?

—
Reply to this email directly or view it on GitHub.

[mikaelkaron]

Btw, thanks for being so attentive with all the tickets, must be a pain to keep track of all things velocity. Really appreciate it.

[julianshapiro]

It's my pleasure. Thank you so much for contributing, Michael. It's when someone like you takes interest in the project that I get motivated to continue improving it.

[mikaelkaron]

No its my pleasure to help just a little. Anyways, instead of messing up the thread I'll just leave it alone with one last comment of "great job", and I'm sure many others around here would concur.

[ydaniv]

This has become too messy in git terms. Will open a new PR.

[julianshapiro]

Please update both Velocity and the UI pack. Promises are now implemented: http://velocityjs.org/#promises