So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this State. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

I have added some additional comments to the docs to indicate some of the dangers of using this feature.

Added another test, the isSuspended() method, and some more notes to warn developers.

I'm very heartened that none of the warnings raised so far and/or added to the docs are surprising or problematic to me.

I see this being used in my project and by most people simply to "turn off" whole sub-trees, eg in a version of ng-show that simply stops the $digestion of hidden content, and I don't think any of the mentioned edge cases will cause problems in such an implementation.

But the proof is in the pudding. I'll test it soon in my app and give feedback on how it goes in practice.

Thanks for the review @gkalpak - I am wary of using names that are too similar to Angular, since this is different enough that it could end up being more confusing when migrating to Angular.

OK, I think I have found a possible Catch 22 with this implementation from my early testing.

I started by creating my most desired perf feature since one-time binding. At the moment it's just a hack of ng-show. I call it p-show ("p" for "performance"! :D). The idea is to disable the descendants' scopes upon hide, and re-enable upon show.

angular.module('myApp').directive('pShow', ['$animate', function($animate) {
 return {
 restrict: 'A',
 multiElement: true,
 link: function(scope, element, attr) {
 scope.$watch(attr.pShow, function pShowWatchAction(value) {
 // ADDED poshest 
 var NG_HIDE_CLASS = 'ng-hide'; // to remain compatible with original ng-show code
 var NG_HIDE_IN_PROGRESS_CLASS = 'ng-hide-animate'; // ditto
 if (value) scope.$resume();
 else scope.$suspend();
 console.log('pShow:', value, '$isSuspended:', scope.$isSuspended());
 // END: ADDED poshest 
 
 // we're adding a temporary, animation-specific class for ng-hide since this way
 // we can control when the element is actually displayed on screen without having
 // to have a global/greedy CSS selector that breaks when other animations are run.
 // Read: https://github.com/angular/angular.js/issues/9103#issuecomment-58335845
 $animate[value ? 'removeClass' : 'addClass'](element, NG_HIDE_CLASS, {
 tempClasses: NG_HIDE_IN_PROGRESS_CLASS
 });
 });
 }
 };
}]);

I realised the problem pretty early on... in almost all the use cases for .$suspend() that I can think of, the .$resume() code will need to be located INSIDE a $watch or $onChanges on that very suspended scope! The result is that hiding works, but showing doesn't.

I read back to vahan-hartooni's version of this and he, I think rightly wrote this as a suspension of child scopes only.

I don't see how also suspending the current scope could be workable, unless anyone has other ideas about how I can trigger .$resume() in an "Angular"-way or if I've mis-used or mis-understood scope.$suspend.

I think you want to make the directive create a new scope, e.g. scope: true in the declaration...
Then you can watch the attribute value which will be bound to the parent scope.

I added scope: true to the declaration. That caused this error in multiple places Multiple directives [pShow, myComponent] asking for new/isolated scope..., so I changed all of these

<my-component p-show="$ctrl.showMe"></my-component>

to these

<div p-show="p-show="$ctrl.showMe"><my-component></my-component></div>

and I still get the same result.

I've got a deadline til Sunday night on something else, but I'll get a codepen going on Monday so we're on the same page and can eliminate the possibility that I messed something silly up. Enjoy your weekends! :)

Ahah! Of course, the component is requesting an isolate scope, so you can't also ask for a new scope :-(

Another quick observation while I think of it is that $scope.$digests which are manually called on descendant scopes are still firing despite an ancestor having been suspended. I expect this based on my understanding of the implementation (.$$suspended isn't propagated down the tree), but it might be confusing to others in cases when they do stuff outside Angular and then call a $digest on the descendant to get Angular back in sync. They may expect that $digest not to fire because of the suspended ancestor. Maybe just another note for the docs, perhaps as a clarification to the note...

If a parent scope is suspended then all its descendants will be excluded from future digests...

Those digests need to be initiated from the parent or above, right?

Thanks @poshest for doing the tests! I think you are right with your concerns.
But first, here's a directive that would make "suspend scope on hide" work: http://plnkr.co/edit/5Ci3xUAmEduroFUF31LK?p=preview
It uses transclusion into a child scope (instead of the parent scope) to make it work.
The problem is that we'd have to ship such a directive as part of core, as it would need to be implemented anyway if someone wants to use this feature.

So in general, I think it makes sense to suspend the child scopes when calling suspend on a scope.

I assume a general use case will be to cut of whole parts of an application from digesting if they are hidden, e.g. if a modal is open, you don't need to digest the rest of the UI. That means most of the time the control of the suspension does not lie with the scope owner ( e.g. component), but with the parent. For example, if I want to use a third party component, I should be able to put a "suspender" directive on the element, that will toggle digestion on the component's scope (template) (the part which I don't own), while still allowing the component element (which might have other bindings) to digest. At the moment, this seems like it's unsupported.

Manual $digests will digest the scope on which they are called, and all descendant scopes. This is to be expected. You should use scope.$apply anyway, which does not have this problem - it digest the rootScope and will stop on the suspended scope.

Is there actually anything wrong with @Narretz's example directive? Does it work if there is an isolate component on the element being transcluded?

The problem is that we'd have to ship such a directive as part of core, as it would need to be implemented anyway if someone wants to use this feature.

Why would this need to be shipped with core?

Because of

I realised the problem pretty early on... in almost all the use cases for .$suspend() that I can think of, the .$resume() code will need to be located INSIDE a $watch or $onChanges on that very suspended scope! The result is that hiding works, but showing doesn't.

i.e. you need one scope that handles the watch that suspends / unsuspends another scope. You can't have both on the same scope, because the watch won't fire on a suspended scope.

Here is a directive that appears to work as I would expect: http://plnkr.co/edit/N0w01zkzUPugUJ10wRUp?p=preview

Can you comment?

The transclusion trick works for me and so do your directives @Narretz and @petebacondarwin. They resolve my issue of $watch disabling, albeit in a way that I wouldn't easily come to myself. The suspend children/descendants method might be more intuitive for less experienced developers, but then as you said, this feature isn't for them :)

Since it is a bit tricky to get it to work just-so, how about we do implement an ngSuspend directive, that is similar to what @Narretz and I have been suggesting? But lace its documentation with a lot of warnings...
Alternatively we just create a separate module that contains this directive and release it as a third party library and then link to it from our documentation?

@petebacondarwin I'm happy either way. :)

But I'm curious why not just go with the suspend children option? I don't really understand @Narretz
​

Technically I think suspending only the children is a bit more difficult because you cannot be sure that the scope you call it on has a child scope at the moment.

Why does that matter? If there are no child scopes there's nothing to suspend (works as expected). The suspension will still work if that scope later acquires children, no?

I guess. I haven't looked into the implementation. I think it's easy to break the scope traversion loop when we find a scope that whose children are suspended.

@petebacondarwin directive has the advantage that you can put it on the same element.
However, the directive also suspends the host element, not only the component contents. I find that a bit unflexible. My previous example: if I include a 3rd party component which I want to suspend, but still be able to change bindings on the host element, e.g. http://plnkr.co/edit/xEkRBjswFMrcdEF5SG0x?p=preview
You can see that the color only changes if I digest the component. To me, that's a least unexpected. I'd find it easier to understand if only the child (isolate) scope was suspended.

I think it's easy to break the scope traversion loop when we find a scope that whose children are suspended.

vahan-hartooni's version of this simply omits the watchers = !current.$$suspended && bit from

if ((watchers = !current.$$suspended && current.$$watchers)) {

So that effectively only the change to this line

if (!(next = ((!current.$$suspended && current.$$watchersCount && current.$$childHead)

remains. Does that effectively target only the child/descendent scopes? Or is this a naive approach?

As a thought experiment consider the following situation where there is a directive suspendOn that doesn't create a scope of its own but toggles the suspended state of the current scope.

<div ng-controller="MyController"> <!-- this creates a scope for us -->
 <div>{{ a }}</div>
 <div suspend-on="value">
 {{ b }}
 <some-component></some-component>
 </div>
 <div>{{ c }}</div>
</div>

The scope at the suspendOn directive is the one defined by the controller.

a) If we go with the current implementation, then such a directive would suspend updates to all the interpolations ({{ a }}, {{ b }} and {{ c }}) and the someComponent component.
b) If we go with suspending children implementation, then the template/transclusion of someComponent will be suspended (since it creates a new isolate/transclusion scopes) but {{ b }} will not be suspended.

In both cases this is crazy and confusing.

So, our suspend-on directive must create a scope of its own in order to work in an intuitive sense. As @poshest pointed out, you cannot simply insist on suspendOn asking for a new scope since that prevents it from being put on an element that is also a component. This leaves us requiring that suspendOn uses the transclusion approach to ensure that a new scope is created. Further by using transclude: 'element' we get the bonus that we can put it on an element that has a component and that component will either appear inside or outside the suspended scope depending upon its priority. If the component has a lower priority then it will be part of the transclusion, if it is higher then the suspendOn directive will be part of the component's transclusion.

For me, this makes sense. I believe that changing to suspend children rather than suspending the current scope is no more intuitive than what this PR proposes, since the directive that would need to be implemented is just as complex.

I agree that suspending only children is tricky/not ideal (but probably for different reasons than @Narretz 😁).

Basically, change detection should be a responsibility of the component that contans the bindings, not the user of the component. The primary (only) reason for suspending a scope is performance and only the implementor should be concerned with when is an appropriate time to suspend/resume. The user of the component should not know anything about that and not care.

For example, imagine a component that displays user info and only cares to update its view when the logged-in user changes. It suspends its scope once the use logs in and temporarily resume once the UserService emits a user change. This is an internal implementation detail that should not leak outside of the component. The consumer should just drop the component on a page/template and not care what the component does to keep itself efficient and when it needs to be resumed.

By suspending children only we make it essentially impossible for a component to control its own change detection (unless every components that needs that adds an extra wrapper in its template and creates a new scope for it).
This is also the reason why I don't particularly like the transclude directives posted above.

Furthermore, this approach deviates from how things work in Angular (2+). Although we can't get a 1-to-1 match in bahavior, keeping the concepts and usage patterns as similar as possible would make sense to me.

Still, I don't have a brilliant alternative suggestion 😁 Maybe the $suspend()/$resume() primitives are just too low-level to bake into the component pattern and we can instead build on top of them.

Ideally, I would like an easy way to comfigure this on the CDO (Component Definition Object) - similar to Angular's changeDetection: ChangeDetectionStrategy.OnPush - and an easy way to request a one-off change detection (or automatically set it up when $onChanges() fires).

But I think these can be built on top of $suspend()/$resume() as non-breaking features.

In any case, I wouldn't go too far recommending an alternative usage pattern (e.g. as shown with the transclude directives).

Woohoo! I monkey-patched my way to (a naive implementation of) ChangeDetectionStrategy.OnPush 🎉

Demo

Looks like you and I look at this from 2 different perspectives @gkalpak :>
I agree that it makes sense for components to decide on their update strategy.
But if the component owner wants to cut off a part of the page from the digest cycle, why shouldn't he? If he knows that this component won't need to be updated, then it should be possible to do that. Ofc a component could use onPush, but what if you are using a component that is unlikely to get updated with this new API? Then you want to be able to suspend it completely yourself.
So I think both are valid use cases. Yours, because it makes it easier to write Angular2 style apps, and mine because it makes it easier to perf-tune existing apps.
So maybe we

• keep it so that the actual scope gets suspended
• add the transclude-suspend directive
• add an "onPush" option

@petebacondarwin

Darn, you're absolutely right. $suspend either way without some kind of scope transclusion makes no sense. I'm really warming to an ngSuspend. How would that work if you had <div ng-if="myVar" ng-suspend="myVar"></div>? Do they both create their own scope? Does it matter if you have multiple transcluded scopes cascading from the same element? I guess it's no different from something like <div ng-if="myVar" ng-repeat="..."></div> and the priority determines the order of transclusion?

Oh, and that just made me think... in the case of <div ng-suspend="myVar" ng-repeat="..."></div>, I would expect ng-suspend to suspend all of the ng-repeat scopes. Is that how it would work (given the appropriate priority)?

@gkalpak

My problem with your "user info" example is that your use case is a simple component with no children appearing once on the page. This is not a situation calling for perf optimisation. AngularJS suffers in situations when you have massive ng-repeat lists. So a better example is a "report" component, which has many table rows. Even if the parent report isn't scope-suspended, all of its ng-repeated child scopes will be, which gives you the orders of magnitude perf improvement you're looking for. In any case, I believe Pete's thought experiment renders this thread redundant.

On the other hand, ChangeDetectionStrategy.OnPush? AMAZING DUDE! That is way beyond expectations, but would be an equally, if not more important perf feature for me. Would it be possible to add a markForCheck() equivalent? That would work best with my architecture where I'm using something like observables, but if I have to change everything to immutables to take advantage, I won't be unhappy.

@Narretz

Maybe...? Definitely! :) Oh, it's gonna be a great Christmas :)

@poshest regarding usage of ng-suspend with ng-if and ng-repeat it comes down to priority. All three directives use transclude: 'element' which passes lower priority directives down to the transcluded content.

@poshest, @gkalpak, @Narretz and @mgol are we agreed that merging this PR with just the suspend functionality is an acceptable start? We can discuss building on top of it later?

Would it be a breaking change to start by not suspending $broadcast events but then adding it in later?

@petebacondarwin sure, let's merge now and make some progress with this. With all that I've learned here, $suspend as implemented in this PR is plenty enough for me to use for everything I'd hoped.

Do you mean will the later adding of $broadcast suspension be a breaking change? Yes, I think so. I prefer to resolve that now and not create expectations of future changes.

I'm happy to go with not suspending $broadcasts as the "final" solution.

I still argue that some developers might find this confusing, but with appropriate warnings in the documentation it can work. On the other hand, not suspending $broadcasts gives the developer more options, as @gkalpak argues

• keep state up-to-date where events change component state
• potentially use events to trigger suspends/resumes

And we can always manually "ignore" broadcasts on suspended tree branches by checking for ancestries' .isSuspended().

I think suspending the events would be a breaking change. I also don't think we should suspend / stop the events anyway because they are independent from the digest cycle.
Otherwise, 👍 for merging with just the suspend functionality.