Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

fix($q) allow $q to work well with decorators #3670

Closed
wants to merge 1 commit into from

Conversation

dcherman
Copy link
Contributor

Re-work the $q service a bit so that rather than invoking functions stored in a closure internally ( defer, reject, etc.. ), it uses the methods of the object that's returned so that when it's overridden or modified in a decorator, the desired behavior is maintained throughout

Fixes #3659

@ThomasBurleson
Copy link

Decorators are used to intercept, extend, or modify service behaviors. Do such to a Promise/Future seems fundamentally wrong. What is your use case for decorator $q Futures ?

@dcherman
Copy link
Contributor Author

At the time this was filed, my use case was that a recent breaking change ( promise.always -> promise.finally ) was causing us a lot of pain between browsers, linting, and always having to use bracket notation as a result.

The goal was to restore the .always method as an alias of .finally to avoid all of that pain.

@schmod
Copy link
Contributor

schmod commented Mar 20, 2014

I could see an argument for adding bits of the original Q that were omitted from Angular.

I sure do wish that Angular's $q had a .spread() method. $q.all is super-awkward to use without it.

@ThomasBurleson
Copy link

Simply use a decorator to add/inject the spread functionality to $q

On Thursday, March 20, 2014, Andrew Schmadel notifications@github.com
wrote:

I could see an argument for adding bits of the original Qhttps://github.com/kriskowal/qthat were omitted from Angular.

I sure do wish that Angular's $q had a .spread() method. $q.all is
super-awkward to use without it.


Reply to this email directly or view it on GitHubhttps://github.com//pull/3670#issuecomment-38180568
.

Thomas Burleson
Mindspace LLC

http://www.gridlinked.info
http://www.linkedin/in/ThomasBurleson
https://github.com/ThomasBurleson

@ToucheSir
Copy link

+1 A spread method would be great. However, decorating $q promises appears to be more or less impossible with the current implementation. If only angular switched over $q to use a promise constructor (like other libraries) instead of the rather awkward deferred model....

@ThomasBurleson
Copy link

You asked... so here is the code for a $q decorator

@ToucheSir
Copy link

I may not have explained this very well...it’s easy to modify the $q service using a decorator, but the main draw of .spread() is that you can utilize it like .then() - on promise instances. Currently, $q hides the creation of promise instances within .defer(), making it difficult to add convenience methods such as .spread() without resorting to hackery.

@lgalfaso
Copy link
Contributor

This should be possible when 1.3 is out, look at #5976 and 01a909f

@ThomasBurleson
Copy link

@ToucheSir
Can you provide a Gist of what you mean? I am not grasping the need/concept. Thx

@dcherman
Copy link
Contributor Author

@ThomasBurleson If you look at the unit test that was added in this PR, it should be fairly straightforward. The implementation at the time this PR was made used method references that are stored in the closure rather than invoking the ones on the returned interface which makes it impossible to decorate them ( in this case, adding a new method to whatever is returned from .defer )

@ToucheSir
Copy link

@lgalfaso I remember seeing that pr a little while ago, looking forward to 1.3 :)
@ThomasBurleson Can't come up with any specific examples, but a good case study would be a promise library like bluebird. It uses a promise constructor, so extending the functionality is as simple as adding new methods to the promise prototype. $q, I believe, keeps most of the promise implementation encapsulated within defer(), which means the only way to ensure all created promises have some additional method is to wrap each and every promise after it's created.

@lgalfaso
Copy link
Contributor

$q went thru several refactors and this PR is no longer valid

@lgalfaso lgalfaso closed this Dec 31, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Re-work $q a bit to better support decorators
6 participants