Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

QueryList.changes Observable is not closed on component destroy (ViewChildren, etc) #18741

Closed
shedar opened this issue Aug 17, 2017 · 1 comment
Labels
area: core Issues related to the framework runtime memory leak Issue related to a memory leak type: bug/fix

Comments

@shedar
Copy link

shedar commented Aug 17, 2017

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report  
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question

Current behavior

QueryList.changes observable (for ViewChildren, ContentChildren and probably others) is not completed/closed on component destroy. So if there are subscriptions to a QueryList.changes - it should be explicitly unsubscribed to prevent memory leaks and performance degradation over time.

Expected behavior

When component is destroyed - it should implicitly close all QueryList.changes observables. Because operations is definitely complete for query list changes on component destroy stage.

Minimal reproduction of the problem with instructions

https://plnkr.co/edit/P6tpwaE5Nt2HZuyiSqWj?p=preview

  1. open browser dev tools console
  2. click "Toggle Child" button (it'll start logging state of the changes.closed (false) every second)
  3. click "Toggle Child" again, so component is removed with ngIf, but ViewChildren.changes is not closed.

What is the motivation / use case for changing the behavior?

It's unintuitive that component-tracking observable is alive after component destruction and you have explicitly unsubscribe. It causes memory leaks, performance degradation in angular apps.

E.g. angular-material components have this issue, such as MdLineSetter, MdFormField and probably more.

Environment



Angular version: 4.3.5

Browser:
- [x] Chrome (desktop) version XX
- [ ] Chrome (Android) version XX
- [ ] Chrome (iOS) version XX
- [ ] Firefox version XX
- [ ] Safari (desktop) version XX
- [ ] Safari (iOS) version XX
- [ ] IE version XX
- [ ] Edge version XX
 
For Tooling issues:
- Node version: XX  
- Platform:  

Others:

@vicb vicb added area: core Issues related to the framework runtime severity3: broken type: bug/fix memory leak Issue related to a memory leak and removed severity3: broken labels Aug 17, 2017
@vicb vicb added this to Prio_col1 in Compiler Aug 17, 2017
marclaval added a commit to marclaval/angular that referenced this issue Aug 28, 2017
marclaval added a commit to marclaval/angular that referenced this issue Aug 28, 2017
mhevery pushed a commit to marclaval/angular that referenced this issue Sep 1, 2017
mhevery pushed a commit to marclaval/angular that referenced this issue Sep 1, 2017
mhevery pushed a commit to marclaval/angular that referenced this issue Sep 1, 2017
@mhevery mhevery closed this as completed in 36d37cc Sep 2, 2017
@tbosch tbosch removed this from Prio_col1 in Compiler Oct 3, 2017
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: core Issues related to the framework runtime memory leak Issue related to a memory leak type: bug/fix
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants