fix(core): lack of unsubscriptions (#65)

Author: Oleg Teterin

projects/ngneat/transloco-persist-lang/src/lib/persist-lang.service.ts

@@ -6,16 +6,18 @@ import {
   PersistStorage,
   TranslocoService
 } from '@ngneat/transloco';
+import { Subscription } from 'rxjs';
 import { skip } from 'rxjs/operators';
 import {
   PersistLangConfig,
   TRANSLOCO_PERSIST_LANG_CONFIG,
   TRANSLOCO_PERSIST_LANG_STORAGE
 } from './persist-lang.config';
-import { Inject, Injectable } from '@angular/core';
+import { Inject, Injectable, OnDestroy } from '@angular/core';
 
 @Injectable({ providedIn: 'root' })
-export class TranslocoPersistLangService {
+export class TranslocoPersistLangService implements OnDestroy {
+  private subscription: Subscription = Subscription.EMPTY;
   private storageKey: string;
 
   constructor(
@@ -38,16 +40,17 @@ export class TranslocoPersistLangService {
     isBrowser() && this.storage.removeItem(this.storageKey);
   }
 
-  private updateStorageOnLangChange() {
-    this.service.langChanges$.pipe(skip(1)).subscribe(lang => {
+  private updateStorageOnLangChange(): Subscription {
+    return this.service.langChanges$.pipe(skip(1)).subscribe(lang => {
       this.save(lang);
     });
   }
 
   private init() {
     // We need to first set the cached lang and then listen to changes
     this.setActiveLang();
-    this.updateStorageOnLangChange();
+    this.subscription.unsubscribe();
+    this.subscription = this.updateStorageOnLangChange();
   }
 
   private setActiveLang() {
@@ -73,4 +76,9 @@ export class TranslocoPersistLangService {
     }
     this.storage.setItem(this.storageKey, lang);
   }
+
+  ngOnDestroy() {
+    this.subscription.unsubscribe();
+  }
+
 }

projects/ngneat/transloco-persist-translations/src/lib/transloco-persist-translations.service.ts

@@ -1,5 +1,5 @@
 import { isObject, isString, Translation, TranslocoLoader } from '@ngneat/transloco';
-import { from, Observable, of } from 'rxjs';
+import { from, Observable, of, Subscription } from 'rxjs';
 import { filter, map, switchMap, take, tap } from 'rxjs/operators';
 import { now, observify } from './helpers';
 import {
@@ -9,13 +9,14 @@ import {
   PERSIST_TRANSLATIONS_STORAGE,
   TranslocoPersistTranslationsConfig
 } from './transloco-persist-translations.config';
-import { Inject, Injectable } from '@angular/core';
+import { Inject, Injectable, OnDestroy } from '@angular/core';
 import { MaybeAsyncStorage } from './transloco.storage';
 
 const getTimestampKey = key => `${key}/timestamp`;
 
 @Injectable()
-export class TranslocoPersistTranslations implements TranslocoLoader {
+export class TranslocoPersistTranslations implements TranslocoLoader, OnDestroy {
+  private subscription: Subscription;
   private merged: TranslocoPersistTranslationsConfig;
   private cache: Translation | null = null;
 
@@ -25,7 +26,7 @@ export class TranslocoPersistTranslations implements TranslocoLoader {
     @Inject(PERSIST_TRANSLATIONS_CONFIG) private config: TranslocoPersistTranslationsConfig
   ) {
     this.merged = { ...defaultConfig, ...(config || {}) };
-    this.clearCurrentStorage();
+    this.subscription = this.clearCurrentStorage().subscribe();
   }
 
   getTranslation(lang: string): Observable<Translation> {
@@ -101,9 +102,9 @@ export class TranslocoPersistTranslations implements TranslocoLoader {
     this.storage.removeItem(this.merged.storageKey);
   }
 
-  private clearCurrentStorage() {
+  private clearCurrentStorage(): Observable<any> {
     const storageKey = this.merged.storageKey;
-    this.getTimestamp(storageKey)
+    return this.getTimestamp(storageKey)
       .pipe(
         filter(time => !!time),
         tap(time => {
@@ -113,6 +114,10 @@ export class TranslocoPersistTranslations implements TranslocoLoader {
           }
         })
       )
-      .subscribe();
   }
+
+  ngOnDestroy() {
+    this.subscription.unsubscribe();
+  }
+
 }

projects/ngneat/transloco-preload-langs/src/lib/transloco-preload-langs.module.ts

@@ -1,6 +1,6 @@
-import { Inject, ModuleWithProviders, NgModule } from '@angular/core';
+import { Inject, ModuleWithProviders, NgModule, OnDestroy } from '@angular/core';
 import { getPipeValue, TranslocoService } from '@ngneat/transloco';
-import { forkJoin } from 'rxjs';
+import { forkJoin, Subscription } from 'rxjs';
 import { tap } from 'rxjs/operators';
 
 declare global {
@@ -31,7 +31,10 @@ window.cancelIdleCallback =
   };
 
 @NgModule()
-export class TranslocoPreloadLangsModule {
+export class TranslocoPreloadLangsModule implements OnDestroy {
+  private idleCallbackId: any;
+  private subscription: Subscription = Subscription.EMPTY;
+
   static preload(langs: string[]): ModuleWithProviders {
     return {
       ngModule: TranslocoPreloadLangsModule,
@@ -41,7 +44,7 @@ export class TranslocoPreloadLangsModule {
 
   constructor(service: TranslocoService, @Inject('PreloadLangs') langs: string[]) {
     if (!langs) return;
-    window.requestIdleCallback(() => {
+    this.idleCallbackId = window.requestIdleCallback(() => {
       const preloads = langs.map(currentLang => {
         const [isScoped, scopedPath] = getPipeValue(currentLang, 'scoped');
         const lang = isScoped ? `${scopedPath}/${service.getActiveLang()}` : currentLang;
@@ -53,7 +56,15 @@ export class TranslocoPreloadLangsModule {
           })
         );
       });
-      forkJoin(preloads).subscribe();
+      this.subscription = forkJoin(preloads).subscribe();
     });
   }
+
+  ngOnDestroy() {
+    if (this.idleCallbackId !== undefined) {
+      window.cancelIdleCallback(this.idleCallbackId);
+    }
+    this.subscription.unsubscribe();
+  }
+
 }

projects/ngneat/transloco/src/lib/transloco.directive.ts

@@ -28,7 +28,7 @@ import { shouldListenToLangChanges } from './shared';
   selector: '[transloco]'
 })
 export class TranslocoDirective implements OnInit, OnDestroy, OnChanges {
-  subscription: Subscription;
+  subscription: Subscription = Subscription.EMPTY;
   view: EmbeddedViewRef<any>;
 
   @Input('transloco') key: string;
@@ -147,7 +147,7 @@ export class TranslocoDirective implements OnInit, OnDestroy, OnChanges {
   }
 
   ngOnDestroy() {
-    this.subscription && this.subscription.unsubscribe();
+    this.subscription.unsubscribe();
   }
 
   private detachLoader() {

projects/ngneat/transloco/src/lib/transloco.pipe.ts

@@ -13,7 +13,7 @@ import { shouldListenToLangChanges } from './shared';
   pure: false
 })
 export class TranslocoPipe implements PipeTransform, OnDestroy {
-  subscription: Subscription;
+  subscription: Subscription = Subscription.EMPTY;
   value: string = '';
   lastKey: string;
   lastParams: HashMap;
@@ -43,7 +43,7 @@ export class TranslocoPipe implements PipeTransform, OnDestroy {
     this.lastParams = params;
 
     /* Clean previous subscription if exists */
-    this.subscription && this.subscription.unsubscribe();
+    this.subscription.unsubscribe();
 
     this.subscription = this.translocoService.langChanges$
       .pipe(
@@ -60,7 +60,7 @@ export class TranslocoPipe implements PipeTransform, OnDestroy {
   }
 
   ngOnDestroy() {
-    this.subscription && this.subscription.unsubscribe();
+    this.subscription.unsubscribe();
   }
 
   private updateValue(key: string, params?: HashMap): void {

projects/ngneat/transloco/src/lib/transloco.service.ts

@@ -1,5 +1,5 @@
-import { Inject, Injectable, Optional } from '@angular/core';
-import { BehaviorSubject, combineLatest, from, Observable, Subject, of } from 'rxjs';
+import { Inject, Injectable, OnDestroy, Optional } from '@angular/core';
+import { BehaviorSubject, combineLatest, from, Observable, Subject, of, Subscription } from 'rxjs';
 import { catchError, distinctUntilChanged, map, retry, shareReplay, tap, switchMap } from 'rxjs/operators';
 import { DefaultLoader, TRANSLOCO_LOADER, TranslocoLoader } from './transloco.loader';
 import { TRANSLOCO_TRANSPILER, TranslocoTranspiler } from './transloco.transpiler';
@@ -28,7 +28,8 @@ export function translate<T = any>(key: TranslateParams, params: HashMap = {}, l
 }
 
 @Injectable({ providedIn: 'root' })
-export class TranslocoService {
+export class TranslocoService implements OnDestroy {
+  private subscription: Subscription;
   private translations = new Map<string, Translation>();
   private cache = new Map<string, Observable<Translation>>();
   private defaultLang: string;
@@ -64,7 +65,7 @@ export class TranslocoService {
     /**
      * When we have a failure, we want to define the next language that succeeded as the active
      */
-    this.events$.subscribe(e => {
+    this.subscription = this.events$.subscribe(e => {
       if (e.type === 'translationLoadSuccess' && e.wasFailure) {
         // Handle scoped lang
         const lang = getLangFromScope(e.payload.lang);
@@ -351,4 +352,9 @@ export class TranslocoService {
 
     return this.load(resolveLang);
   }
+
+  ngOnDestroy() {
+    this.subscription.unsubscribe();
+  }
+
 }

src/app/app.component.ts

@@ -1,13 +1,16 @@
-import { Component } from '@angular/core';
+import { Component, OnDestroy } from '@angular/core';
 import { TranslocoService } from '@ngneat/transloco';
+import { Subscription } from 'rxjs';
 import { take } from 'rxjs/operators';
 
 @Component({
   selector: 'app-root',
   templateUrl: './app.component.html',
   styleUrls: ['./app.component.css']
 })
-export class AppComponent {
+export class AppComponent implements OnDestroy {
+  private subscription: Subscription = Subscription.EMPTY;
+
   constructor(private service: TranslocoService) {}
 
   get activeLang() {
@@ -16,11 +19,17 @@ export class AppComponent {
 
   change(lang: string) {
     // Ensure new active lang is loaded
-    this.service
+    this.subscription.unsubscribe();
+    this.subscription = this.service
       .load(lang)
       .pipe(take(1))
       .subscribe(() => {
         this.service.setActiveLang(lang);
       });
   }
+
+  ngOnDestroy() {
+    this.subscription.unsubscribe();
+  }
+
 }