28

I have a FormGroup with ValueChanges event that is not being released from memory when the user moves from component's route to another component and then they return to the component.

What this means is that if the user navigates away from component and then back to the component 5 times, the onFormChange method fires 5 times, but only 1 of those calls is for the current component.

I figured that the problem was that I needed to unsubscribe from the the valueChanges event in the NgDestroy event, but there is no unsubscribe method available on the valueChanges event.

I am sure I have to unsubscribe or release memory for something, but I'm not sure what.

import * as _ from 'lodash';
import {Observable} from 'rxjs/Rx';

import {Component, Input, Output, EventEmitter, OnInit, OnDestroy} from '@angular/core';
import {FormGroup} from '@angular/forms';

import {formConfig} from './toolbar.form-config';
import {JobToolbarVm} from '../view-model/job-toolbar.vm';
import {BroadcastService} from '../../../services/broadcast/broadcast.service';

@Component({
    selector: 'wk-job-toolbar',
    template: require('./toolbar.html'),
})
export class JobToolbarComponent implements OnInit, OnDestroy {

  protected form: FormGroup;

  @Input()
  toolbar: JobToolbarVm;

  @Output()
  toolbarChanged = new EventEmitter<JobToolbarVm>();

  @Output()
  refresh = new EventEmitter<string>();

  constructor(private broadcast: BroadcastService) {
  }

  ngOnInit() {

    this.form = formConfig;
    this.form.setValue(this.toolbar, {onlySelf: true});

    // This ALWAYS RUNS when the form loads, ie. on the job route
    console.log('FORM VALUE');
    console.log(JSON.stringify(this.form.value, null, 2));

    this.form.valueChanges
      .debounceTime(2000)
      .subscribe(
        this.onFormChange.bind(this)
      );
  }

  ngOnDestroy() {
    //this.form.valueChanges.unsubscribe();
    //this.onChanges.unsubscribe();
    //this.toolbarChanged.unsubscribe();
    //this.form = null;
  }

  onFormChange(data: any) {
    // This runs whenever I go to a different route and then come back to this route
    // There is also a memory leak, because this method fires multiple times based on how
    // often I navigate away and come back to this route.
    // e.g. Navigate away and then back 5 times, then I see this log statement 5 times 
    console.log('FORM VALUE2 - THIS KEEPS FIRING FOR EACH INSTANCE OF MY COMPOMENT');
    console.log(JSON.stringify(this.form.value, null, 2));

    JobToolbarVm.fromJsonIntoInstance(data, this.toolbar);

    this.onChanges('data-changed');
  }

  onChanges($event: any) {
    console.log('onChanges: ' + $event);
    // console.log(this.toolbar);

    // Send the toolbar object back out to the parent
    this.toolbarChanged.emit(this.toolbar);

    // Broadcast an event that will be listened to by the list component so that it knows when to refresh the list
    this.broadcast.broadcast('job-admin-toolbar-changed', this.toolbar);
  }
}
David Cruwys
  • 6,262
  • 12
  • 45
  • 91

6 Answers6

41

The subscribe() call returns a Subscription and this is what you use to unsubscribe:

class JobToolbarComponent

  private subscr:Subscription;

  ngOnInit() {
    ...
    this.subscr = this.form.valueChanges ...
    ...
  }

  ngOnDestroy() {
    this.subscr.unsubscribe();
  }
}
Günter Zöchbauer
  • 623,577
  • 216
  • 2,003
  • 1,567
  • 7
    Hello! Thank you very much for your answers. I constantly read them. Subscribed to github on you. But could you explain why we need to unsubscribe if the component and the form are destroyed by themselves, shouldn't the garbage collector delete the subscription from memory? – Smiranin Nov 08 '18 at 18:43
  • 2
    Thanks :-) Usually it's a good practice to imperatively unsubscribe if you imperatively subscribed. It can also be that garbage collection does not immediately dispose of the component instance and then it might still receive some events util it's finally gone. The subscribe callbacks might cause errors when the component is not alive anymore or might cause expensive operations (network request) that are not necessary anymore when the component is disposed. – Günter Zöchbauer Nov 08 '18 at 18:49
  • 1
    @GünterZöchbauer Ok. Thanks!;) – Smiranin Nov 08 '18 at 19:23
  • 3
    When the `this.form` has been created in the component, which event could trigger a `ValueChange` after the component has been destroyed? I cannot imagine a case where I need to unsubscribe from own created form. Would also be interesting if `unsubscribe` can stop the flow after `debounceTime` (see example above). BTW your answer is correct - my comment is more about "do we have to unsubscribe?". – hgoebl Feb 12 '19 at 11:46
  • 1
    It's a general rule that if you subscribe (explicitly lock a resource) that you explicitly release it. Of course there can be situations where this might not be necessary, but often code is modified later in a way that releasing would be necessary but then it's easily overlooked and can cause hard to find bugs. It's just defensive programming to avoid bugs. – Günter Zöchbauer Feb 12 '19 at 12:17
4

I have created this following function

export function AutoUnsubscribe(exclude = []) {

    return function (constructor) {

        const original = constructor.prototype.ngOnDestroy;

        constructor.prototype.ngOnDestroy = function () {
            for (let prop in this) {
                const property = this[prop];
                if (!exclude.includes(prop)) {
                    if (property && (typeof property.unsubscribe === "function")) {
                        property.unsubscribe();
                    }
                }
            }
            original && typeof original === 'function' && original.apply(this, arguments);
        };
    }

}

which actually you can use to auto unsubscribe all the watchers but you have to store them in public properties so that this function can intercept it and invoke unsubscribe on that. How you use it is mentioned below:-

@AutoUnsubscribe()
@Component({
    selector: 'account-login',
    templateUrl: './login.component.html',
    styleUrls: ['./login.component.scss']
})
export class LoginComponent implements OnInit {


    public submitWatcher: Subscription;

     submit() {
        this.submitWatcher = this.authService.login(this.loginForm.getRawValue())
            .subscribe(res => {
                if (this.returnUrl) {
                    this.router.navigate([this.returnUrl]);
                }
                else {
                    this.router.navigate(['/special']);
                }
            }, (error) => {
                alert(JSON.stringify(error.data));
            });
    }

}

For more info on how to use decorator please read this blog that's where I have taken the idea from and it is pretty cool

Blog

Atul Chaudhary
  • 3,698
  • 1
  • 31
  • 51
  • Atul.. I liked the idea of custom decorators... but in this case using take, takeWhile and takeUntil operators make much more sense. Using the above decorators is not the efficient way of unsubscribing... What do you think? – Ritesh Waghela Sep 14 '18 at 06:40
4

You could do the following:

// private variable to hold all your subscriptions for the component
private subscriptions: Subscription[] = [];

// when you subscribe to an observable,
// you can push all subscription this.subscriptions

this.subscriptions.push(
       this.form.valueChanges.pipe(
            .debounceTime(2000)) 
            .subscribe(val => this.onFormChange.bind(this)),

       this.observe2$.subscribe(val => this.somemethod(val))
    );

// in ngondestroy
    ngOnDestroy(): void {
        if (this.subscriptions && this.subscriptions.length > 0) {
            this.subscriptions.forEach(s => s.unsubscribe());
        }
    }

2

The easiest to put together the subscriptions is to use the add method : subscription.add( anotherSubscription ) so now you just have to unsubscribe from only one subscription.

You can find the following example from the official doc : https://rxjs.dev/guide/subscription

import { interval } from 'rxjs';
 
const observable1 = interval(400);
const observable2 = interval(300);
 
const subscription = observable1.subscribe(x => console.log('first: ' + x));
const childSubscription = observable2.subscribe(x => console.log('second: ' + x));
 
subscription.add(childSubscription);
 
setTimeout(() => {
  // Unsubscribes BOTH subscription and childSubscription
  subscription.unsubscribe();
}, 1000); 
Gauthier Peel
  • 1,438
  • 2
  • 17
  • 35
1

I am not sure if this is a good idea but this is very simple to implement and it works well for my school project.

var sub = this.items.subscribe(snapshots => {
    console.log(snapshots);
    sub.unsubscribe();
});

Source: https://github.com/angular/angularfire2/issues/377

Harry
  • 1,147
  • 13
  • 13
0

I prefer to use takeUntil rather than keeping track of all my subscriptions and manually unsubscribing from them all.

RxJS: takeUntil() Angular component's ngOnDestroy()

sirbradwick
  • 61
  • 1
  • 10