angular / angular.js

AngularJS - HTML enhanced for web apps!
https://angularjs.org
MIT License
58.8k stars 27.48k forks source link

$q constructor function creates fulfilled promise. #6427

Closed joshperry closed 10 years ago

joshperry commented 10 years ago

It would be nice to have $q() create a fulfilled promise like Q does. This is nice for services that return data that could be cached but you always want to return a promise.

   getData: function(key) {
     if(cache[key]) {
       return $q(cache[key]);
     } else {
       return db.find(key).then(function(doc) {
         cache[key] = doc;
       });
     }
   }

I would be happy to put together a PR with this, I just don't want to go through the work if it's not going to get looked at for inclusion.

pkozlowski-opensource commented 10 years ago

@joshperry the current implementation of $q has a method - $q.when(value) that can be ussed to achieve your desired effect. You could simply write:

getData: function(key) {
     if(cache[key]) {
       return $q.when(cache[key]);
     } 

While not a constructor, it hopefully does a trick for you.

btford commented 10 years ago

@caitp I heard you're working on a rewrite of $q in 1.3

caitp commented 10 years ago

@lgalfaso and I have both come up with fixes to get us passing the aplus tests, and I generally prefer Lukas' fixes as a first pass for the rewrite. We can ship those in the next few weeks.

There has also been discussion about performing better (in terms of memory usage) by using fly-weight style code which does not create new copies of closures for every promise instance. This is a lot harder to do (and is the primary reason why I prefer Lukas' fix for the aplus tests). These performance fixes are really the next step in evolving $q.

So basically, none of these fixes really add new features per se, and I'm not sure it's necessarily a good idea to add new features to the 1.x stuff, for compatibility reasons, but if it's something you really need (and are really opposed to using $q.when() to accomplish), then we can look at this after Lukas' fixes are merged.

joshperry commented 10 years ago

I don't need this functionality; $q.when() gives me what I "need". In fact, because of #6087 I would not be able to use this feature how I want anyway. What I was looking at doing was using promises with the reduce function like described in this Promise Antipatterns post under "Collection Kerfuffle".

I guess I have another question. Why can't $q be like jqLite? Maybe qLite; it is used when Q isn't already imported, purposefully keeping the API for $q a superset of Q. I can open another issue to discuss this in more detail if that would help.

pkozlowski-opensource commented 10 years ago

@joshperry using Q (when available) instead of the built-in $q was discussed in the past in https://github.com/angular/angular.js/issues/1998

caitp commented 10 years ago

Personally, I feel like if we're going to make $q do something "as a constructor", it should behave similar to ES6 promises (eg $q(function(resolve, reject) {}).

And as far as I'm concerned, there's probably no reason why we can't say $q.resolve is an alias of $q.when --- which would be closer to the natural order of things. But I'm not sure it makes a lot of sense to add a totally bizarro behaviour.

Compared with the other changes to $q, at least those ones seem doable within a time frame for the next release.