Why is the call to $apply needed in $http?

[thorn0]

I started to think about it looking at #12557. At first glance the answer is obvious: we need to update the views after we got a response, so we trigger a digest. But hey, it'll be triggered anyway because $http uses $q internally. And when a $q deferred is resolved, it schedules a digest. So can't we just remove that call to $rootScope.$apply (with all the $applyAsync thing altogether)? Am I missing something? If it's needed only for the tests, the $apply call should be moved to $httpBackend.flush.

[gkalpak]

Hey, this sounds about right. I would try replacing $q with $$q inside $httpand keep $http's apply (sync or async) - probably disable-able with an extra argument.

(This has been discussed before (more or less). See #2049 and #2049 (comment).)

@thorn0, would fancy giving it a shot ?

[thorn0]

I would try replacing $q with $$q inside $http and keep $http's apply (sync or async)

It won't work this way. The $apply call in $http is useless (or seems so if I'm still missing something) because it happens just after the response comes, but before any callbacks are called, which means before any scopes are modified. So what changes is it supposed to digest? After git blame'ing, it seems to me that this $apply call is merely a historical rudiment.

And please note that it's not the same issue as #2049 and #12557. It's more of a small performance refactoring. $applyAsync isn't needed. We'll get the same effect if we just remove this $apply call.

[gkalpak]

True. Yet, by merely removing $http's apply, we can't have the applyAsync perf improvement (which we don't have now either I think).

So, I would still try replacing $$q with $q and add another callback at the end of the promise chain to make the digest (sync or async). That way we can easily have normal apply (using $rootScope.$degest()), async apply (using $rootScope.$applyAsync()) or add an extra option to skip apply altogether (in which case we should not add anything at the end of the chain).

_{Again, this is off the top of my head, so take it with a grain of salt 😃}

[gkalpak]

Or do you mean removing the $rootScope.$apply(), but keep the $rootScope.$applyAsync(...) ?
That would work as well, I guess, but we would still get an extra $digest in the async case (I think).

Plus, replacing $q with $$q would also enable skipping apply altogether (as an option).

[thorn0]

try replacing $$q with $q and add another callback at the end of the promise chain to make the digest

We can break someone's response interceptors this way.

Or do you mean removing the $rootScope.$apply(), but keep the $rootScope.$applyAsync(...) ?

No, I mean we can safely remove all these lines:

        if (useApplyAsync) {
          $rootScope.$applyAsync(resolveHttpPromise);
        } else {
          resolveHttpPromise();
          if (!$rootScope.$$phase) $rootScope.$apply();
        }

and leave only

        resolveHttpPromise();

[gkalpak]

try replacing $$q with $q and add another callback at the end of the promise chain to make the digest

We can break someone's response interceptors this way.

How exactly ?

Or do you mean removing the $rootScope.$apply(), but keep the $rootScope.$applyAsync(...) ?

No, I mean we can [...] leave only resolveHttpPromise();.

That would essentially remove the useApplySync optimization.
We don't want to remove that one (I suppose it was added for a reason).

[thorn0]

try replacing $$q with $q and add another callback at the end of the promise chain to make the digest

We can break someone's response interceptors this way.

How exactly ?

Those interceptors can modify the scopes (or other $watched values) and expect a digest to happen earlier than in the end of the interceptor chain.

No, I mean we can [...] leave only resolveHttpPromise();

That would essentially remove the useApplySync optimization.
We don't want to remove that one (I suppose it was added for a reason).

No, we'll just get the same effect for free.

[gkalpak]

OK, I'll believe you when I see it 😛

[thorn0]

OK, please see this PR #13111.

[thorn0]

I think this issue is important in the first place because $http should be a model for building async services. It should be a canonical example of how async operations and the digest cycle should be married so that libraries like bramski/angular-indexedDB could learn from it. But unfortunately, this example is misleading now. It leaves people wonder why it was written the way it was, why this $apply call can be needed.