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

) 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 <[email protected]>
eclipsesource · Oct 10, 2024 · 1feaf0b · 1feaf0b
1 parent d7f34b9
commit 1feaf0b
Show file tree

Hide file tree

Showing 8 changed files with 121 additions and 130 deletions.
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();
+  }
 }