NetanelBasal / angular2-take-until-destroy

Declarative way to unsubscribe from observables when the component destroyed
37 stars 10 forks source link

Destroys subscriptions in all instances #5

Closed jjenzz closed 7 years ago

jjenzz commented 7 years ago

If there are multiple instances of the same component and one of those instances is destroyed, the current version of this decorator will kill the subscriptions across all instances even though they have not been destroyed.

I fixed the problem here but was unable to provide a PR as you only share your dist and not your source.

import { Subject } from "rxjs/Subject";
export function TakeUntilDestroy(constructor) {
    var original = constructor.prototype.ngOnDestroy;
    constructor.prototype._takeUntilDestroy$ = new Subject();
    constructor.prototype.componentDestroy = function () {
        return this._takeUntilDestroy$.asObservable();
    };
    constructor.prototype.ngOnDestroy = function () {
        original && typeof original === 'function' && original.apply(this, arguments);
        this._takeUntilDestroy$.complete();
    };
}
//# sourceMappingURL=angular2-take-until-destroy.js.map

It would be really helpful to get this in 🙏

NetanelBasal commented 7 years ago

Hey, Thanks. I will check it out and let you know.

NetanelBasal commented 7 years ago

By the way, I uploaded the src now, so you can create a PR.

NetanelBasal commented 7 years ago

Are you sure your solution works? Because I could not make it work. I was tested this on multiple instances with ngIf toggling.

jjenzz commented 7 years ago

@NetanelBasal hmm, I think I was half asleep that day because looking at this now it obviously wouldn't work as I was instantiating on the prototype :man_facepalming:. I mustn't have been testing it properly, sorry about that. I think something more along the lines of the following would be what we need but I haven't had a chance to test it yet:

import { Subject } from "rxjs/Subject";
export function TakeUntilDestroy(constructor) {
    var originalInit = constructor.prototype.ngOnInit;
    var originalDestroy = constructor.prototype.ngOnDestroy;
    constructor.prototype.componentDestroy = function () {
        return this._takeUntilDestroy$.asObservable();
    };
    constructor.prototype.ngOnInit = function() {
        originalInit && typeof originalInit === 'function' && originalInit.apply(this, arguments);
        this._takeUntilDestroy$ = new Subject();
    };
    constructor.prototype.ngOnDestroy = function () {
        originalDestroy && typeof originalDestroy === 'function' && originalDestroy.apply(this, arguments);
        this._takeUntilDestroy$.complete();
    };
}

When I get a moment, I'll check and PR :+1: