angular: Unsubscribe from state subscriptions on renderer destroy (#2377)

LukasBoll · lucas-koehler · web-flow · commit 1feaf0b66462 · 2024-10-10T11:12:21.000+02:00
Previously, subscriptions to the JSON Forms state in Angular were never removed when renderer components are destroyed. With this commit, all subscriptions will be unsubscribed when the component is destroyed. The base renderer offers an addSubscription method to register one or multiple subscriptions to automatically be unsubscribed on destroy. With this, extending renderers do not need to implement the unsubscribe again. Closes #2354 Co-authored-by: Lucas Koehler <lkoehler@eclipsesource.com>
diff --git a/packages/angular-material/src/library/layouts/array-layout.renderer.ts b/packages/angular-material/src/library/layouts/array-layout.renderer.ts
@@ -22,12 +22,7 @@
   OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
   THE SOFTWARE.
 */
-import {
-  ChangeDetectionStrategy,
-  Component,
-  OnDestroy,
-  OnInit,
-} from '@angular/core';
+import { ChangeDetectionStrategy, Component, OnInit } from '@angular/core';
 import {
   JsonFormsAngularService,
   JsonFormsAbstractControl,
@@ -169,7 +164,7 @@ import {
 })
 export class ArrayLayoutRenderer
   extends JsonFormsAbstractControl<StatePropsOfArrayLayout>
-  implements OnInit, OnDestroy
+  implements OnInit
 {
   noData: boolean;
   translations: ArrayTranslations = {};
diff --git a/packages/angular-material/src/library/layouts/categorization-layout.renderer.ts b/packages/angular-material/src/library/layouts/categorization-layout.renderer.ts
@@ -38,12 +38,11 @@ import {
   rankWith,
   uiTypeIs,
 } from '@jsonforms/core';
-import { Component, OnDestroy, OnInit } from '@angular/core';
+import { Component, OnInit } from '@angular/core';
 import {
   JsonFormsAngularService,
   JsonFormsBaseRenderer,
 } from '@jsonforms/angular';
-import { Subscription } from 'rxjs';
 
 @Component({
   selector: 'jsonforms-categorization-layout',
@@ -69,41 +68,36 @@ import { Subscription } from 'rxjs';
 })
 export class CategorizationTabLayoutRenderer
   extends JsonFormsBaseRenderer<Categorization>
-  implements OnInit, OnDestroy
+  implements OnInit
 {
   hidden: boolean;
   visibleCategories: (Category | Categorization)[];
-  private subscription: Subscription;
   categoryLabels: string[];
 
   constructor(private jsonFormsService: JsonFormsAngularService) {
     super();
   }
 
   ngOnInit() {
-    this.subscription = this.jsonFormsService.$state.subscribe({
-      next: (state: JsonFormsState) => {
-        const props = mapStateToLayoutProps(state, this.getOwnProps());
-        this.hidden = !props.visible;
-        this.visibleCategories = this.uischema.elements.filter(
-          (category: Category | Categorization) =>
-            isVisible(category, props.data, undefined, getAjv(state))
-        );
-        this.categoryLabels = this.visibleCategories.map((element) =>
-          deriveLabelForUISchemaElement(
-            element as Labelable<boolean>,
-            state.jsonforms.i18n?.translate ??
-              defaultJsonFormsI18nState.translate
-          )
-        );
-      },
-    });
-  }
-
-  ngOnDestroy() {
-    if (this.subscription) {
-      this.subscription.unsubscribe();
-    }
+    this.addSubscription(
+      this.jsonFormsService.$state.subscribe({
+        next: (state: JsonFormsState) => {
+          const props = mapStateToLayoutProps(state, this.getOwnProps());
+          this.hidden = !props.visible;
+          this.visibleCategories = this.uischema.elements.filter(
+            (category: Category | Categorization) =>
+              isVisible(category, props.data, undefined, getAjv(state))
+          );
+          this.categoryLabels = this.visibleCategories.map((element) =>
+            deriveLabelForUISchemaElement(
+              element as Labelable<boolean>,
+              state.jsonforms.i18n?.translate ??
+                defaultJsonFormsI18nState.translate
+            )
+          );
+        },
+      })
+    );
   }
 }
 
diff --git a/packages/angular-material/src/library/layouts/layout.renderer.ts b/packages/angular-material/src/library/layouts/layout.renderer.ts
@@ -23,7 +23,6 @@
   THE SOFTWARE.
 */
 import {
-  OnDestroy,
   OnInit,
   ChangeDetectorRef,
   Component,
@@ -42,18 +41,16 @@ import {
   UISchemaElement,
   JsonSchema,
 } from '@jsonforms/core';
-import type { Subscription } from 'rxjs';
 
 @Component({
   template: '',
 })
 export class LayoutRenderer<T extends Layout>
   extends JsonFormsBaseRenderer<T>
-  implements OnInit, OnDestroy
+  implements OnInit
 {
   hidden: boolean;
   label: string | undefined;
-  private subscription: Subscription;
 
   constructor(
     private jsonFormsService: JsonFormsAngularService,
@@ -63,20 +60,16 @@ export class LayoutRenderer<T extends Layout>
   }
 
   ngOnInit() {
-    this.subscription = this.jsonFormsService.$state.subscribe({
-      next: (state: JsonFormsState) => {
-        const props = mapStateToLayoutProps(state, this.getOwnProps());
-        this.label = props.label;
-        this.hidden = !props.visible;
-        this.changeDetectionRef.markForCheck();
-      },
-    });
-  }
-
-  ngOnDestroy() {
-    if (this.subscription) {
-      this.subscription.unsubscribe();
-    }
+    this.addSubscription(
+      this.jsonFormsService.$state.subscribe({
+        next: (state: JsonFormsState) => {
+          const props = mapStateToLayoutProps(state, this.getOwnProps());
+          this.label = props.label;
+          this.hidden = !props.visible;
+          this.changeDetectionRef.markForCheck();
+        },
+      })
+    );
   }
 
   trackElement(_index: number, renderProp: OwnPropsOfRenderer): string {
diff --git a/packages/angular-material/src/library/other/label.renderer.ts b/packages/angular-material/src/library/other/label.renderer.ts
@@ -22,7 +22,7 @@
   OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
   THE SOFTWARE.
 */
-import { Component, OnDestroy, OnInit } from '@angular/core';
+import { Component, OnInit } from '@angular/core';
 import {
   JsonFormsAngularService,
   JsonFormsBaseRenderer,
@@ -36,7 +36,6 @@ import {
   rankWith,
   uiTypeIs,
 } from '@jsonforms/core';
-import { Subscription } from 'rxjs';
 
 @Component({
   selector: 'LabelRenderer',
@@ -51,33 +50,27 @@ import { Subscription } from 'rxjs';
 })
 export class LabelRenderer
   extends JsonFormsBaseRenderer<LabelElement>
-  implements OnDestroy, OnInit
+  implements OnInit
 {
   label: string;
   visible: boolean;
 
-  private subscription: Subscription;
-
   constructor(private jsonFormsService: JsonFormsAngularService) {
     super();
   }
   ngOnInit() {
-    this.subscription = this.jsonFormsService.$state.subscribe({
-      next: (state: JsonFormsState) => {
-        const props = mapStateToLabelProps(
-          state,
-          this.getOwnProps() as OwnPropsOfLabel
-        );
-        this.visible = props.visible;
-        this.label = props.text;
-      },
-    });
-  }
-
-  ngOnDestroy() {
-    if (this.subscription) {
-      this.subscription.unsubscribe();
-    }
+    this.addSubscription(
+      this.jsonFormsService.$state.subscribe({
+        next: (state: JsonFormsState) => {
+          const props = mapStateToLabelProps(
+            state,
+            this.getOwnProps() as OwnPropsOfLabel
+          );
+          this.visible = props.visible;
+          this.label = props.text;
+        },
+      })
+    );
   }
 }
 
diff --git a/packages/angular/src/library/abstract-control.ts b/packages/angular/src/library/abstract-control.ts
@@ -39,7 +39,6 @@ import {
   ValidationErrors,
   ValidatorFn,
 } from '@angular/forms';
-import type { Subscription } from 'rxjs';
 
 import { JsonFormsBaseRenderer } from './base.renderer';
 import { JsonFormsAngularService } from './jsonforms.service';
@@ -58,7 +57,6 @@ export abstract class JsonFormsAbstractControl<
   @Input() visible: boolean;
 
   form: FormControl;
-  subscription: Subscription;
   data: any;
   label: string;
   description: string;
@@ -99,41 +97,45 @@ export abstract class JsonFormsAbstractControl<
   }
 
   ngOnInit() {
-    this.jsonFormsService.$state.subscribe({
-      next: (state: JsonFormsState) => {
-        const props = this.mapToProps(state);
-        const {
-          data,
-          enabled,
-          errors,
-          label,
-          required,
-          schema,
-          rootSchema,
-          visible,
-          path,
-          config,
-        } = props;
-        this.label = computeLabel(
-          label,
-          required,
-          config ? config.hideRequiredAsterisk : false
-        );
-        this.data = data;
-        this.error = errors;
-        this.enabled = enabled;
-        this.isEnabled() ? this.form.enable() : this.form.disable();
-        this.hidden = !visible;
-        this.scopedSchema = schema;
-        this.rootSchema = rootSchema;
-        this.description =
-          this.scopedSchema !== undefined ? this.scopedSchema.description : '';
-        this.id = props.id;
-        this.form.setValue(data);
-        this.propsPath = path;
-        this.mapAdditionalProps(props);
-      },
-    });
+    this.addSubscription(
+      this.jsonFormsService.$state.subscribe({
+        next: (state: JsonFormsState) => {
+          const props = this.mapToProps(state);
+          const {
+            data,
+            enabled,
+            errors,
+            label,
+            required,
+            schema,
+            rootSchema,
+            visible,
+            path,
+            config,
+          } = props;
+          this.label = computeLabel(
+            label,
+            required,
+            config ? config.hideRequiredAsterisk : false
+          );
+          this.data = data;
+          this.error = errors;
+          this.enabled = enabled;
+          this.isEnabled() ? this.form.enable() : this.form.disable();
+          this.hidden = !visible;
+          this.scopedSchema = schema;
+          this.rootSchema = rootSchema;
+          this.description =
+            this.scopedSchema !== undefined
+              ? this.scopedSchema.description
+              : '';
+          this.id = props.id;
+          this.form.setValue(data);
+          this.propsPath = path;
+          this.mapAdditionalProps(props);
+        },
+      })
+    );
     this.triggerValidation();
   }
 
@@ -146,9 +148,7 @@ export abstract class JsonFormsAbstractControl<
   }
 
   ngOnDestroy() {
-    if (this.subscription) {
-      this.subscription.unsubscribe();
-    }
+    super.ngOnDestroy();
     removeId(this.id);
   }
 
diff --git a/packages/angular/src/library/base.renderer.ts b/packages/angular/src/library/base.renderer.ts
@@ -22,18 +22,30 @@
   OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
   THE SOFTWARE.
 */
-import { Directive, Input } from '@angular/core';
+import { Directive, Input, OnDestroy } from '@angular/core';
 import {
   JsonSchema,
   OwnPropsOfRenderer,
   UISchemaElement,
 } from '@jsonforms/core';
+import { Subscription } from 'rxjs';
 
 @Directive()
-export class JsonFormsBaseRenderer<T extends UISchemaElement> {
+export class JsonFormsBaseRenderer<T extends UISchemaElement>
+  implements OnDestroy
+{
   @Input() uischema: T;
   @Input() schema: JsonSchema;
   @Input() path: string;
+  protected subscriptions: Subscription = new Subscription();
+
+  protected addSubscription(subscription: Subscription | Subscription[]) {
+    if (Array.isArray(subscription)) {
+      subscription.forEach((sub) => this.subscriptions.add(sub));
+    } else {
+      this.subscriptions.add(subscription);
+    }
+  }
 
   protected getOwnProps(): OwnPropsOfRenderer {
     return {
@@ -42,4 +54,8 @@ export class JsonFormsBaseRenderer<T extends UISchemaElement> {
       path: this.path,
     };
   }
+
+  ngOnDestroy(): void {
+    this.subscriptions.unsubscribe();
+  }
 }
diff --git a/packages/angular/src/library/jsonforms-root.component.ts b/packages/angular/src/library/jsonforms-root.component.ts
diff --git a/packages/angular/src/library/jsonforms.component.ts b/packages/angular/src/library/jsonforms.component.ts
