fix($timeout/$interval): if invokeApply is false, do not use evalAsync

[caitp]

$evalAsync triggers a digest, and is unsuitable when it is expected that a
digest should not occur.

BREAKING CHANGE

Previously, even if invokeApply was set to false, a $rootScope digest would occur during promise resolution. Workarounds include manually triggering $scope.$apply(), or returning $q.defer().promise from a promise callback, and resolving or rejecting it when appropriate.

var interval = $interval(function() {
  if (someRequirementFulfilled) {
    $interval.cancel(interval);
    $scope.$apply();
  }
}, 100, 0, false);

or:

var interval = $interval(function (idx) {
  // make the magic happen
}, 1000, 10, false);
interval.then(function(idx) {
  var deferred = $q.defer();
  // do the asynchronous magic --- $evalAsync will cause a digest and cause
  // bindings to update.
  return deferred.promise;
});

[rodyhaddad]

I see you called it $qq instead of $$q. I believe the idea was to have it stay private

---

Don't we need any tests for $$q?
I see that currently we just test qFactory and never test $q, hmm

[caitp]

I don't think we need tests for $$q since it's identical to $q, and the $timeout and $interval tests cover the only difference

[rodyhaddad]

I believe we can use $browser.defer.flush here, which makes more sense than using $timeout.flush for this.
Same thing for test in timeoutSpec

Edit: $timeout.flush just calls $browser.defer.flush anyway

[caitp]

$timeout.flush is $browser.defer.flush, there's no difference --- it's just easier to read.

[rodyhaddad]

I feel it introduces more concepts to have in mind while reading the test, but that's just me.

[caitp]

As for it staying private, I don't really care, it's just a name, $$-prefixed providers aren't really any more private than any other providers --- we could call it $qNoDigest or whatever if we wanted

[rodyhaddad]

Yeah, but making it $$ means we don't have to document the difference or support it for the future. Maybe someone comes up with a more elegant solution than this in the future

[caitp]

renaming it is easy, but $$ doesn't really have any real meaning, I just want you to understand that --- third party modules would still be able to use $$q, and they'll still complain if it breaks. The fact that it's not documented is what really makes it private and discourages its use

[rodyhaddad]

Yup.
The contract with users of angular is: "anything you see in the codebase that start with $$-, don't count on it staying the same in the future".
I know that part.

[caitp]

There is no real contract --- it's a convention, and that's perfectly fine, but this isn't any kind of protection against people using it, or against people complaining when it breaks. It's all well and good to say that, but you know fully well that when people depend on it for something, you will start to see issues opened about it.

And in fact, people will probably open issues about the breaking change mentioned in the description here, there's really no end to what people will consider to be bugs, and while it's easy to close each one as "works as expected", it doesn't change the fact that it's going to make people unhappy at some point. It's essentially a guarantee, and no $$ prefix will protect against that.

Now, I will rename this after I finish working on another patch I'm writing

[IgorMinar]

lgtm

[IgorMinar]

We have a process to deal with breaking changes for public apis but that process doesn't apply to $$ apis. if people depend on $$* apis and we break them then it's their fault.

[caitp]

I don't disagree, but you're still going to see people unhappy with it, that's just the way it is :( anyways, I pushed the renamed version, so I'll check that in when travis is green, I guess

[IgorMinar]

but they'll be unhappy because of their mistake. that's the difference.

On Fri, Jun 27, 2014 at 8:18 AM, Caitlin Potter notifications@github.com
wrote:

I don't disagree, but you're still going to see people unhappy with it,
that's just the way it is :( anyways, I pushed the renamed version, so I'll
check that in when travis is green, I guess

—
Reply to this email directly or view it on GitHub
#7999 (comment).

[caitp]

Yeah, but they won't consider it their mistake --- the same way people won't consider it their mistake if they use angular.extend and that behaviour changes, they'll say "wow an exported API from a package I'm using broke, this makes me very unhappy! they clearly don't care about how they're affecting their users applications! I'm going to write an unhappy blog about this!"

Ignoring it is great, but those things don't happen in a vacuum, they can have an impact on the credibility of the library.

Of course on the other hand, if you don't export them, people will still complain about "useful APIs not being exported", so maybe there's no way to win here.

The bigger concern here is that it's probably going to break people who were depending on promises from $timeout/$interval to act like promises from the rest of the library, I'm sure those people are out there somewhere.

[thatmarvin]

Can/Will this land in 1.2.x?

[caitp]

it's a breaking change, so I feel like probably not, but see what @IgorMinar thinks

[thatmarvin]

Ah ok. Btw, this wasn't actually documented as a breaking change in the CHANGELOG for 1.3.0-beta.14 too.

[caitp]

yes, you're right. Weird.

[caitp]

I've made a CL to fix that, #8474 --- https://github.com/caitp/angular.js/blob/changelog/CHANGELOG.md look okay to you?

[thatmarvin]

Lgtm @caitp!