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

Q resolve outside of $digest #2881

Closed
icholy opened this issue Jun 5, 2013 · 24 comments
Closed

Q resolve outside of $digest #2881

icholy opened this issue Jun 5, 2013 · 24 comments

Comments

@icholy
Copy link
Contributor

icholy commented Jun 5, 2013

I have a lot of code in services that use Q promises for async stuff. I can't use $q because it only resolves in a $digest cycle. I really don't want to include 2 copies of the Q library but, by the looks of it, I have to.

I don't know if this has been discussed, but could there be another method added to deferreds? Something like deferred.resolveNow(/* stuff */) which doesn't use nextTick

@0x-r4bbit
Copy link
Contributor

If you use a Q implementation which lives outside of angular world, wrap the calls in $rootScope.$apply to kick a $digest

@andi1984
Copy link
Contributor

andi1984 commented Jun 5, 2013

This is really annoying...

I also worked on an app with a lot of chained digest events. Q implementation should definitely live inside the angular world or should be integrated!

It's weird that execution sometime stops and proceeds when an UI interaction happens. At least on some mobile devices.

@icholy
Copy link
Contributor Author

icholy commented Jun 5, 2013

@PascalPrecht starting a $digest every time I use a promise is not an option for me. There are performance considerations.
EDIT: misread your comment the first time. I want to avoid an external Q altogether.

@0x-r4bbit
Copy link
Contributor

@andi1984 Uhm... there is a Q implementation inside of angular ? Just use $q service :)

@andi1984
Copy link
Contributor

andi1984 commented Jun 6, 2013

@PascalPrecht No, I'm already used the $q service, but it seems that some mobile devices have problems with chained callbacks, at least via the $q service.

The execution stops at some deeper level and proceeds when a new UI interaction was done.

@icholy
Copy link
Contributor Author

icholy commented Jun 6, 2013

@andi1984 it's like that so you can use promises in your model data & templates and have it work as expected.

@bfanger
Copy link
Contributor

bfanger commented Jun 12, 2013

I've a jsFiddle which indicates the complexity of using $q http://jsfiddle.net/Lpadg/

I think the problem is nextTick in angular. Which could/should schedule the tick handler.

// in nextTick
if (!tickScheduled) {
  tickScheduled = true;
  setTimeout(function () {
    tickScheduled = false;
    handleTick();
  }, 0);
}

@andi1984
Copy link
Contributor

@bfanger The problem could be caused by iOS itself, having problems to schedule the ticks? Issue #2942 could be correlated to that. I'm not an AngularJS Pro, but it could be caused by the same tick scheduler issue.

@mgcrea
Copy link
Contributor

mgcrea commented Jun 17, 2013

Encountering this as well (see #2993), exposing qFactory could solve theses kind of issues as we would be able to setup a custom $q service with a $scope-independent nextTick().

@dcherman
Copy link
Contributor

It seems silly to have promises only become resolved once a $digest() happens imo.

Consider the following:

setTimeout(function() {
    $http.post( "some-logging-url" );
});

Nothing in there explicitly should require a $digest() to occur, so requiring that code to be wrapped in a $timeout seems silly, however that request will actually never happen until a $digest() occurs since internally $http calls $.when( config ). That behavior is rather surprising, at least to me.

What's the actual benefit of tying a promise's resolution to the $digest() cycle in the first place?

@andi1984
Copy link
Contributor

@dcherman

What's the actual benefit of tying a promise's resolution to the $digest() cycle in the first place?

That's the main question. I also didn't and still don't get it.

But related to isssue #2894, it seems that you have to wrap it by a manual $applycall:

$rootScope.$apply(function(){ 
    deferred.resolve(data);
});

But, as you, I don't see any deeper sense in that...

@mgcrea
Copy link
Contributor

mgcrea commented Jul 11, 2013

@andi1984 in fact you can't rely on $apply as in some cases (like multiple stacked calls), it will trigger the angular-BSOD, $digest already in progress. The only safe option is to use $timeout, way too hacky imho.

What's the actual benefit of tying a promise's resolution to the $digest() cycle in the first place?

I think it was a way to minimize rendering iterations as promises are somehow queued to the $digest cycle and are all resolved at the same time (at the beginning of the next digest). It is a clever design choice, but we need to be able to use actual $q promises for out-of-angularjs world stuff. In my case, Phonegap/Cordova callbacks.

@icholy
Copy link
Contributor Author

icholy commented Jul 12, 2013

@mgcrea something like icholy@81bd59c example

The problem is $http still returns a $q promise.
If it returned a vanilla q promise it could be wrapped in a $q to get the old functionality

$q.when($http.get("url")).then(/* ... */);

Or there could just be another http service (without the $ prefix) which returns a vanilla promise.
That way we're not breaking backward compatibility.

EDIT If backward compatibility wasn't an issue you could rename $q.defer() to $q.$defer() and have $q.defer() return a non-angular deferred object.

@fallanic
Copy link

Same issue here, it's really annoying.
I'm currently using jQuery deferred to avoid weird behavior of my webapps, but would really prefer not to rely on an external lib.

@mgcrea
Copy link
Contributor

mgcrea commented Aug 13, 2013

@icholy have you submitted a PR with your patch? My use-cases are not bound to $http.

Any chance to get this into 1.1.6?

@scribblet
Copy link

Is the problem that the overhead associated with using a $timeout to invoke $digest is too expensive?

I noticed this problem and just wrapped all my resolves in a $timeout, should probably do the same for reject.

E.g.

    var getBrandDetails = function() {
        var deferred = $q.defer();
        console.log('getting brand details');
        TaprApi.Brand.loadSettings('test', 'TAPR_PUBLISHER').then(
            function(result) {
                $timeout(
                    function() {
                        deferred.resolve(result)
                    },
                0);
            }, function(err) {
                deferred.reject(err);
            }
        );
        return deferred.promise;
    };

@kwerle
Copy link

kwerle commented Aug 28, 2013

I'm an angular neophyte. This 'issue' seems to be affecting me, now that I have upgraded my angular library.

I'm using ngResource. If I understand this thread correctly, when I do

resource.save(data)

it just works. But if I do

resource.save(data, function(){callback stuff})

then I have to do something additional? Uh - I have to wrap the callback in a timeout?

resource.save(data, function(result){$timeout(function(result){callback stuff})})

That's correct, and (not to put too fine a point on it) on purpose?

Ah, no - it turns out that I need to do:

$timeout(function() {resource.save(data, function(){callback stuff})})

Or at least that seems to work at this moment in time.

@IgorMinar
Copy link
Contributor

this has been resolved in 1.2.0rc2 by 6b91aa0

closing...

@bfanger
Copy link
Contributor

bfanger commented Oct 16, 2013

Sorry, but even in 1.2.0-rc.3 using $q is impractical compared to the real Q. http://jsfiddle.net/bfanger/Lpadg/

@v1bh0r
Copy link

v1bh0r commented Oct 17, 2013

Till we upgrade the angular version, in our app we plan to go with the following monkey patch in app.config as a work-around:

$provide.decorator("$q", function ($delegate, $rootScope) {
    var originalDefer = $delegate.defer;
    $delegate.defer = function () {
      var deferred = originalDefer();
      var originalResolve = deferred.resolve;
      deferred.resolve = function (data) {
        originalResolve(data);
        if (!$rootScope.$$phase) {
          $rootScope.$apply();
        }
      }
      return deferred;
    };
    return $delegate;
  });

@bfanger
Copy link
Contributor

bfanger commented Mar 29, 2014

Start with $q: it's easy and build in. But once you've used it and experience these frustrations, just move to Q

http://www.slideshare.net/domenicdenicola/the-promised-land-in-angular

@Vibhor86 Your decorator workaround to makes the resolve() synchronous creating subtle bugs, and is neglecting the reject and notify.

@IgorMinar
Copy link
Contributor

@bfanger your fiddle is incorrect because it assumes that angular will use setTimeout for task scheduling to make promises async. the fact is that it uses $timeout, which gets automatically mocked-out when you load angular-mocks.js.

This allows you to write the beautiful synchronous tests without compromising any promise guarantees. Check out this fork of your fiddle: http://jsfiddle.net/ZKcp6/

unfortunately if you mix Q and $q promises (which you can by doing $q.when(Qpromise), in the tests you'll how to deal with both $q and Q's scheduling mechanisms and either mock out Q's scheduling to make it controllable from a test, or you have to write and async test and override $timeout mock with the real implementation, or write async test and call $timeout.flush() as needed).

In any case, the Angular-specfic slides in the slide deck you are referring to are outdated or plain wrong. Unfortunately Domenic misunderstood the reasoning behind some of the $q design decisions and didn't check with us before publishing the slides.

@bfanger
Copy link
Contributor

bfanger commented Mar 30, 2014

After further testing, the fix in 1.2.0rc2 indeed works a advertised.

Thanks for explaining the connection with $timeout, which causes the unexpected $q resolve behavior when using ngMock.

@IgorMinar
Copy link
Contributor

Sure. I'm glad that it works for you :-)
On Mar 30, 2014 4:16 AM, "Bob Fanger" notifications@github.com wrote:

After further testing http://jsbin.com/cicev/7/edit?js,output, the fix
in 1.2.0rc2 indeed works a advertised.

Thanks for explaining the connection with $timeout, which causes the
unexpected $q resolve behavior when using ngMock.

Reply to this email directly or view it on GitHubhttps://github.com//issues/2881#issuecomment-39023049
.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests