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

Support detaching/re-attaching scope #5301

Closed
chrisirhc opened this issue Dec 6, 2013 · 40 comments
Closed

Support detaching/re-attaching scope #5301

chrisirhc opened this issue Dec 6, 2013 · 40 comments

Comments

@chrisirhc
Copy link
Contributor

This is basically, a reversible $destroy for scopes.

Detaching the scope would mean:

  • Detaching the scope from the scope's current parent so that watchers no longer fire when the scope's current parent's $digest is called
  • On re-attaching to a DOM element (as a child), it would be attached as the child scope of the DOM element

I could see this being used for optimizations such as not running watchers on hidden DOM (that could be detached).

I think a factor that affects the adoption of Angular is the fact that having many watchers could slow down the application. Currently, there's no way to "pause" the watchers on DOM that's not currently visible.

It's possible to hack out a directive to do this, but I'm wondering if this feature is of value to the rest of the Angular community as part of the Angular core.

I noticed that jqLite doesn't have .detach. .remove actually destroys the scope of the element so after re-adding it to the DOM, it actually picks up the another scope and has already lost its own scope.

@btford btford removed the gh: issue label Aug 20, 2014
@vahan-hartooni
Copy link

+1 for this feature as we ran across an optimization issue while developing a tab system with Angular. Each tab had a template inside it with multiple watchers. We didn't want the user to lose their work (e.g. their form inputs and UI states like scroll position), so we just hid the templates, but this lead to the application to slow down on each digest. Our templates should unbind from the scope while they are hidden and rebind when back in view.

Right now there's a few optimization workarounds this lack of feature:

@tjn487
Copy link

tjn487 commented Dec 11, 2014

+1 for this feature as I'm facing same issue while creating dashboard widgets where I don't want the user to lose their work when close/add pods in dashboard. I'm using jqLite .detach() method so that I can add pods once again in DOM without losing any data but pods pick another scope due to which handlers, watchers not worked.

@shahata
Copy link
Contributor

shahata commented Dec 13, 2014

Take a look at https://github.com/shahata/angular-viewport-watch - it basically disables and enables watchers automatically as they get scrolled out of viewport and back in (perf for long ng-repeat), but you can also manually disable and enable watchers for your use case. /cc @petebacondarwin

@vahan-hartooni
Copy link

I've decided to fork and modify Angular to add a flag that allows the digest loop to skip a $scope's children.

I wrote up my motivation and the details of watcher pausing here. Appreciate any feedback on this optimization strategy.

@f-mon
Copy link

f-mon commented Apr 30, 2016

+1. I've build a routing/navigation system on top of Angular, in witch it is mantained a stack of "opened activity" that the user is working with, but the opened activity that are not in the top of the stack are "paused", i'd like not to loose the user temporary data on "paused" activity (very much like Android systems) but also not to slow down the app when the stack of acrtivity grows.. so this issue is very very Important to me.

@petebacondarwin petebacondarwin modified the milestones: Ice Box, Backlog Jun 22, 2016
@petebacondarwin
Copy link
Contributor

Although this is a significant pain point for certain kinds of applications built with Angular 1, it is not a straightforward thing to fix without creating many many opportunities for subtle bugs.
Angular 2 is maturing nicely and has great support for solving this problem. For such applications that require suspending change detection on parts of the application then it would be better to upgrade to Angular 2.

Perhaps one could make use of ngUpgrade to allow those parts of the app that need to be suspended to be controlled by Angular 2?

@poshest
Copy link
Contributor

poshest commented Jul 26, 2016

+1 Surely if this guy can do it, the Angular team could. I can't really think of an app with multiple "views" that wouldn't benefit from this feature. Please please please put it into 1.x branch.

@petebacondarwin ngUpgrade to a framework that isn't even out of release candidate? Most of the Angular developers I know are moving/moved to React because of this kind of treatment.

@petebacondarwin
Copy link
Contributor

petebacondarwin commented Jul 27, 2016

@poshest for sure it is easy to toggle watching for a childscope.

The trouble is that there are subtle issues that can occur if you do not carefully control the boundary between the suspended scope and the rest of the scope hierarchy. In Angular 1 there a many paths that can lead to a value being made available across a scope boundary, such as use of $parent, transclusion, services and so on.

We have spent a lot of time thinking about this over the years and this is in fact one of the driving forces behind the big rewrite that is Angular 2. In Angular 2 there is much more control over the movement of data between components and so it is much safer to select parts of the component tree which do not need to run dirty checking.

If we implemented one of the many suggested solutions to this problem we would then be faced with maintaining it, and more importantly supporting the complicated corner cases that are really hard to control.

But more importantly, application developers might find that their apps are not updating as they expect and it will not necessarily be obvious or easy to identify the cause.

There are already a number of production applications running on Angular 2 in the wild. My impression from those projects is that they enjoy the improved ergonomics of the new version of Angular. Of course each project must make a choice about what level of risk it is willing to take on and the timelines of the application that is being developed.

@poshest
Copy link
Contributor

poshest commented Jul 28, 2016

@petebacondarwin thank you. A very sound response. I do see the motivations behind pushing people to Angular 2 and iceboxing most requests for 1.x functionality (although why not just close them if you're not going to build them).

I started building in Angular 1.x a couple of years ago precisely because I felt it was suitable for non-trivial apps. I'm still getting over my disappointment that it simply is not and in that short time has become legacy. It's like buying a car that you didn't know was limited to 30mph, and being told that the only fix is buy the new faster model.

My major concern with your recommendation to upgrade is that I have no guarantee that Angular 2 will fix this issue. To that end, perhaps you can help me understand what Angular 2 does that will help in this precise situation? Will it just create less watches so you can ng-hide more? Or will it have some smart disabling of watches when they're hidden?

The use case, as explained above, is lots of heavy functionality (ng-repeats, ng-ifs/shows etc, and deeply nested components) for each tab, and wanting not to have to re-render those every time the tab is re-visited. I am already using 100% 1.5 components and one-way bindings.

@wardbell
Copy link

wardbell commented Jul 29, 2016

Someone will do better than I will. But here's the short of it.

Angular 2 (A2) addresses the digest churn problem in two ways

  1. It only makes a single change detection pass down the component tree whereas Angular 1 has to re-cycle the digest cycle until the model proves to be stable (minimum of 2 cycles) or you exceed the limit (10 cycles). The A2 one-pass guarantee is already a performance win w/o the developer having to do anything.

  2. In Angular 2, you can set a component's ChangeDetectionStrategy to OnPush. When you do, Angular stops further change detection for that component and its sub-tree of components. This is the equivalent of one-time binding BUT ...

We can trigger a new round of change detection for that component whenever we want by calling this.changeDetectorRef.markForCheck() (after injecting ChangeDetectorRef).

I think that's the on-demand refresh option that you want. It's not precisely a disable/enable switch. It's more a switch for taking over when change detection happens for a particular component and its sub-tree.

Read about it in this post from Pascal Precht of ThoughtRam (an oldie but goodie).

This facility is baked into A2. Just one of many reasons to like Angular 2.

@poshest
Copy link
Contributor

poshest commented Jul 29, 2016

@wardbell thanks for your response and the link.

The digest churn isn't the problem I'm trying to address here, although removing the second and later passes will obviously help. I have two problems.

(a) It takes a long time to render my complex components. This is the root cause and this is not addressed by this thread. To mitigate this somewhat I wish to at least not render those complex components more than once. But...

(b) If I render a component and then keep it around (hidden by ng-hide), it will continue to be dirty checked in the background. Because of the complexity of my app, I might have 1000s of watches hidden behind the ng-hide.

Halving (or better) the digest cycle time is great, but it doesn't solve the problem. In addition, in response to your point 1 "w/o the developer having to do anything"... as I understand it, in Angular 2 the developer has to do lots - ie change the entire app to use the "data flows down, functions flow up" pattern. This is a fundamental change that involves potentially a lot of work for the developer, and is the exact reason why Angular is able to give the one-pass guarantee. It's like me saying "my framework is really fast, because loops aren't allowed - if you create a loop, the framework will throw an error". Two-way data binding was what MADE Angular 1.x popular.

Your point 2. is much closer what I am after. The article you referenced seems complicated and I'll have to read it a few more times. Once again I'm disappointed that I need to understand so much about the Angular internals to make it work.

But I'm still left yearning for a very simple ng-ifshowwatch, which

  1. before first evaluation to true acted like an ng-if (so that components aren't rendered until the user visits them)
  2. after first evaluation to true acted like ng-show, and
  3. when hidden in subsequent evaluations to false gets the component's (and all its childrens') watches disabled

This is a lovely experience for the developer. markForCheck is nice for those fine-grained control requirements, but requires too much finessing for this very common use case.

BTW, you can currently do 1 and 2 with this, which is everywhere in my code.

<div ng-if="::condition || undefined" ng-show="condition"></div>

"Oh, ng-ifshowwatch... ng-if="twere_only_for_thee""

@petebacondarwin
Copy link
Contributor

@poshest here is another article - https://www.lucidchart.com/techblog/2016/05/04/angular-2-best-practices-change-detector-performance/ - but be aware that the API has changed since this was written. In it the author writes:

By default, the change detection strategy on any component or directive is CheckAlways. There is another strategy, OnPush, which can be much more efficient if you build your application carefully.

OnPush means that the change detector will only run on a component or directive if one of the following occurs:

  • An input property has changed to a new value
  • An event handler fired in the component or directive
  • You manually tell the change detector to look for changes
  • The change detector of a child component or directive runs

If you’re able to use OnPush throughout your application, you will ideally only ever run the change detector on components that have actually changed (and their direct ancestors). This reduces the time complexity of the change detector from O(n) to O(log n) in the number of component instances in your application.

As you say this does require the developer to use immutable data structures and one way data-flow if you don't want to do any change detection yourself.

But I think the key for your goal is that in Angular 2 we have full control over the change detection strategy for each component. For example one can use changeDetectory.detach() and changeDetector.reattach() in your component to get the exact behaviour that you are describing that you want in Angular 1. There are some examples of doing this in the docs:

https://angular.io/docs/ts/latest/api/core/index/ChangeDetectorRef-class.html

@poshest
Copy link
Contributor

poshest commented Oct 21, 2016

@vahan-hartooni FYI the broken link on your post. It's really helpful, so pls update. :)

@poshest
Copy link
Contributor

poshest commented Oct 26, 2016

@petebacondarwin do you have any feedback on @vahan-hartooni 's approach to this? It's my favourite way to achieve this much needed feature. It's a couple of lines of code, and Vahoon has already determined that it passed all the AngularJS tests. Worst case, it doesn't work in some of those "subtle" cases you mention, but if you create the new flag as $skipChildWatchers as Vahoon suggests or even $$skipChildWatchers if you felt it should be an "unsupported, undocumented" feature, no one will be affected unless they use it. And if they're using this, they're already deep in the belly of the Angular 1.x beast. What's the downside? Please consider adding his change to your next 1.5.x release. :)

@petebacondarwin petebacondarwin modified the milestones: 1.5.x, Ice Box Oct 26, 2016
@petebacondarwin
Copy link
Contributor

I've added it to the 1.5.x milestone so that we can consider it further.

@poshest
Copy link
Contributor

poshest commented Oct 26, 2016

Wonderful! Thank you @petebacondarwin :)

@yjaaidi
Copy link

yjaaidi commented Nov 1, 2016

Hi,

It would be really nice to have an implementation of a ChangeDetector similar to Angular 2 with an OnPush mode.

I wrote a module that imitates Angular 2's behaviour but it would be really nice and cleaner to have a similar implementation integrated in Angular 1.5 or Angular 1.6.

http://www.blog.wishtack.com/single-post/2016/10/05/Boost-your-AngularJS-performance-with-ng-steroids-and-get-ready-to-migrate-to-Angular-2

https://github.com/wishtack/ng-steroids

If you agree with the behaviour, I would be glad to get some feedback before making a pull request.

Thanks!

@poshest
Copy link
Contributor

poshest commented Nov 20, 2016

Is there any chance of something happening on $skipChildWatchers or similar in the 1.6 release candidates?

@gkalpak
Copy link
Member

gkalpak commented Nov 20, 2016

The feature set for 1.6.0 is more or less fixed, so I don't see it as likely.

@petebacondarwin petebacondarwin modified the milestones: 1.6.x (post 1.6.0), 1.5.x Nov 20, 2016
@Narretz Narretz modified the milestones: 1.6.x, 1.6.5 Apr 19, 2017
@timruffles
Copy link
Contributor

@petebacondarwin not sure how people should read your final two comments? Are the team planning to implement this ('we have agree to implement...'), or are you looking for outside contribution?

If @shahata's PR was modified to have a more Angular Change Detection feel, would that be merged?

@petebacondarwin
Copy link
Contributor

Hi Tim
We have agreed to implement it but we have very few resources right now to implement it.
Also I wanted to have a view on how close we could get it to the semantic vision of Angular.
Both the view and the implementation are up for grabs if someone in the community wants to get involved.

@poshest
Copy link
Contributor

poshest commented Oct 2, 2017

@petebacondarwin we have very few resources to migrate to Angular 2 right now. My only option is getting max juice out of ol' AngularJS.

Can I suggest that, in the meantime, the @vahan-hartooni suggestion is implemented. Sure, it's not consistent with Angular 2, but it's non-breaking as you said, and would allow us to switch off and on parts of the digest tree for some big big performance gains.

@petebacondarwin
Copy link
Contributor

We would like to implement @shahata's PR here #10658
If you would be interested in helping to get that rebased and tested @poshest

@poshest
Copy link
Contributor

poshest commented Oct 3, 2017

Sorry for ignoring all the refs to the chosen @shahata solution. I see that it's pretty similar to @vahan-hartooni's code, but with nice $suspend and $resume functions. I'm interested to help, but I've never done something like "rebase and test" before. I will probably mess up stuff more and take more of core members' time than if core members just did it.

For starters, what does testing even involve? Just running the existing automated tests? Or do I need to write some new tests to properly address @igalfaso in #10658 who says "this needs to be done in a way that does not involve a lot of work for developers to upgrade to these new features, nor break common patterns used today". I'm not even remotely equipped to do that.

@petebacondarwin
Copy link
Contributor

No problem. Perhaps we can put together a new PR and you can try it out for us?

@petebacondarwin petebacondarwin self-assigned this Oct 3, 2017
@poshest
Copy link
Contributor

poshest commented Oct 3, 2017 via email

@poshest
Copy link
Contributor

poshest commented Oct 24, 2017

@petebacondarwin any progress on that PR? :)

@petebacondarwin
Copy link
Contributor

No movement yet. Sorry. I'll find some time this week.

@poshest
Copy link
Contributor

poshest commented Oct 30, 2017 via email

@petebacondarwin
Copy link
Contributor

I have created a new rebased PR at #16308. @poshest - please take a look.

petebacondarwin added a commit to petebacondarwin/angular.js that referenced this issue Oct 31, 2017
This can be very helpful for external modules that help making the digest
loop faster by ignoring some of the watchers under some circumstance.
Example: https://github.com/shahata/angular-viewport-watch

Thanks to @shahata for the original implementation.

Closes angular#5301
@petebacondarwin petebacondarwin modified the milestones: 1.6.x, 1.7.0 Oct 31, 2017
@poshest
Copy link
Contributor

poshest commented Oct 31, 2017 via email

petebacondarwin added a commit to petebacondarwin/angular.js that referenced this issue Nov 1, 2017
This can be very helpful for external modules that help making the digest
loop faster by ignoring some of the watchers under some circumstance.
Example: https://github.com/shahata/angular-viewport-watch

Thanks to @shahata for the original implementation.

Closes angular#5301
petebacondarwin added a commit to petebacondarwin/angular.js that referenced this issue Nov 1, 2017
This can be very helpful for external modules that help making the digest
loop faster by ignoring some of the watchers under some circumstance.
Example: https://github.com/shahata/angular-viewport-watch

Thanks to @shahata for the original implementation.

Closes angular#5301
petebacondarwin added a commit to petebacondarwin/angular.js that referenced this issue Nov 1, 2017
This can be very helpful for external modules that help making the digest
loop faster by ignoring some of the watchers under some circumstance.
Example: https://github.com/shahata/angular-viewport-watch

Thanks to @shahata for the original implementation.

Closes angular#5301
petebacondarwin added a commit to petebacondarwin/angular.js that referenced this issue Nov 3, 2017
This can be very helpful for external modules that help making the digest
loop faster by ignoring some of the watchers under some circumstance.
Example: https://github.com/shahata/angular-viewport-watch

Thanks to @shahata for the original implementation.

Closes angular#5301
petebacondarwin added a commit to petebacondarwin/angular.js that referenced this issue Nov 21, 2017
This can be very helpful for external modules that help making the digest
loop faster by ignoring some of the watchers under some circumstance.
Example: https://github.com/shahata/angular-viewport-watch

Thanks to @shahata for the original implementation.

Closes angular#5301
petebacondarwin added a commit to petebacondarwin/angular.js that referenced this issue Nov 22, 2017
This can be very helpful for external modules that help making the digest
loop faster by ignoring some of the watchers under some circumstance.
Example: https://github.com/shahata/angular-viewport-watch

Thanks to @shahata for the original implementation.

Closes angular#5301
petebacondarwin added a commit to petebacondarwin/angular.js that referenced this issue Dec 11, 2017
This can be very helpful for external modules that help making the digest
loop faster by ignoring some of the watchers under some circumstance.
Example: https://github.com/shahata/angular-viewport-watch

Thanks to @shahata for the original implementation.

Closes angular#5301
Narretz pushed a commit that referenced this issue Apr 12, 2018
This can be very helpful for external modules that help making the digest
loop faster by ignoring some of the watchers under some circumstance.
Example: https://github.com/shahata/angular-viewport-watch

Thanks to @shahata for the original implementation.

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

No branches or pull requests