Fixed getComponent not triggering again when it's input values change…

… by creating inputNamesDependentForComponent

Also simplified the way @input() values are passed to their child component and how ngOnChanges is called by using setInput instead

src/app/shared/abstract-component-loader/abstract-component-loader.component.ts

@@ -32,10 +32,22 @@ export abstract class AbstractComponentLoaderComponent<T> implements OnInit, OnC
    */
   protected subs: Subscription[] = [];
 
-  protected inAndOutputNames: (keyof this)[] = [
+  /**
+   * The @{@link Input}() that are used to find the matching component using {@link getComponent}. When the value of
+   * one of these @{@link Input}() change this loader needs to retrieve the best matching component again using the
+   * {@link getComponent} method.
+   */
+  protected inputNamesDependentForComponent: (keyof this & string)[] = [
+    'context',
+  ];
+
+  protected inputNames: (keyof this & string)[] = [
     'context',
   ];
 
+  protected outputNames: (keyof this & string)[] = [
+  ];
+
   constructor(
     protected themeService: ThemeService,
   ) {
@@ -45,7 +57,9 @@ export abstract class AbstractComponentLoaderComponent<T> implements OnInit, OnC
    * Set up the dynamic child component
    */
   ngOnInit(): void {
-    this.instantiateComponent();
+    if (hasNoValue(this.compRef)) {
+      this.instantiateComponent();
+    }
   }
 
   /**
@@ -55,14 +69,14 @@ export abstract class AbstractComponentLoaderComponent<T> implements OnInit, OnC
     if (hasNoValue(this.compRef)) {
       // sometimes the component has not been initialized yet, so it first needs to be initialized
       // before being called again
-      this.instantiateComponent(changes);
+      this.instantiateComponent();
     } else {
-      // if an input or output has changed
-      if (this.inAndOutputNames.some((name: any) => hasValue(changes[name]))) {
+      if (this.inputNamesDependentForComponent.some((name: any) => hasValue(changes[name]) && changes[name].previousValue !== changes[name].currentValue)) {
+        // Recreate the component when the @Input()s used by getComponent() aren't up-to-date anymore
+        this.destroyComponentInstance();
+        this.instantiateComponent();
+      } else {
         this.connectInputsAndOutputs();
-        if (this.compRef?.instance && 'ngOnChanges' in this.compRef.instance) {
-          (this.compRef.instance as any).ngOnChanges(changes);
-        }
       }
     }
   }
@@ -73,7 +87,10 @@ export abstract class AbstractComponentLoaderComponent<T> implements OnInit, OnC
       .forEach((subscription: Subscription) => subscription.unsubscribe());
   }
 
-  public instantiateComponent(changes?: SimpleChanges): void {
+  /**
+   * Creates the component and connects the @Input() & @Output() from the ThemedComponent to its child Component.
+   */
+  public instantiateComponent(): void {
     const component: GenericConstructor<T> = this.getComponent();
 
     const viewContainerRef: ViewContainerRef = this.componentDirective.viewContainerRef;
@@ -86,10 +103,16 @@ export abstract class AbstractComponentLoaderComponent<T> implements OnInit, OnC
       },
     );
 
-    if (hasValue(changes)) {
-      this.ngOnChanges(changes);
-    } else {
-      this.connectInputsAndOutputs();
+    this.connectInputsAndOutputs();
+  }
+
+  /**
+   * Destroys the themed component and calls it's `ngOnDestroy`
+   */
+  public destroyComponentInstance(): void {
+    if (hasValue(this.compRef)) {
+      this.compRef.destroy();
+      this.compRef = null;
     }
   }
 
@@ -99,12 +122,18 @@ export abstract class AbstractComponentLoaderComponent<T> implements OnInit, OnC
   public abstract getComponent(): GenericConstructor<T>;
 
   /**
-   * Connect the in and outputs of this component to the dynamic component,
+   * Connect the inputs and outputs of this component to the dynamic component,
    * to ensure they're in sync
    */
   protected connectInputsAndOutputs(): void {
-    if (isNotEmpty(this.inAndOutputNames) && hasValue(this.compRef) && hasValue(this.compRef.instance)) {
-      this.inAndOutputNames.filter((name: any) => this[name] !== undefined).forEach((name: any) => {
+    if (isNotEmpty(this.inputNames) && hasValue(this.compRef) && hasValue(this.compRef.instance)) {
+      this.inputNames.filter((name: string) => this[name] !== undefined).filter((name: string) => this[name] !== this.compRef.instance[name]).forEach((name: string) => {
+        // Using setInput will automatically trigger the ngOnChanges
+        this.compRef.setInput(name, this[name]);
+      });
+    }
+    if (isNotEmpty(this.outputNames) && hasValue(this.compRef) && hasValue(this.compRef.instance)) {
+      this.outputNames.filter((name: string) => this[name] !== undefined).filter((name: string) => this[name] !== this.compRef.instance[name]).forEach((name: string) => {
         this.compRef.instance[name] = this[name];
       });
     }

src/app/shared/metadata-representation/metadata-representation-loader.component.ts

@@ -24,8 +24,8 @@ export class MetadataRepresentationLoaderComponent extends AbstractComponentLoad
    */
   @Input() mdRepresentation: MetadataRepresentation;
 
-  protected inAndOutputNames: (keyof this)[] = [
-    ...this.inAndOutputNames,
+  protected inputNames: (keyof this & string)[] = [
+    ...this.inputNames,
     'mdRepresentation',
   ];
 

src/app/shared/mydspace-actions/claimed-task/switcher/claimed-task-actions-loader.component.ts

@@ -1,4 +1,4 @@
-import { Component, EventEmitter, Input, OnChanges, OnInit, Output, } from '@angular/core';
+import { Component, EventEmitter, Input, Output, } from '@angular/core';
 import { getComponentByWorkflowTaskOption } from './claimed-task-actions-decorator';
 import { ClaimedTask } from '../../../../core/tasks/models/claimed-task-object.model';
 import { MyDSpaceActionsResult } from '../../mydspace-actions';
@@ -16,7 +16,7 @@ import { GenericConstructor } from '../../../../core/shared/generic-constructor'
  * Component for loading a ClaimedTaskAction component depending on the "option" input
  * Passes on the ClaimedTask to the component and subscribes to the processCompleted output
  */
-export class ClaimedTaskActionsLoaderComponent extends AbstractComponentLoaderComponent<ClaimedTaskActionsAbstractComponent> implements OnInit, OnChanges {
+export class ClaimedTaskActionsLoaderComponent extends AbstractComponentLoaderComponent<ClaimedTaskActionsAbstractComponent> {
   /**
    * The item object that belonging to the ClaimedTask object
    */
@@ -46,12 +46,16 @@ export class ClaimedTaskActionsLoaderComponent extends AbstractComponentLoaderCo
   /**
    * The list of input and output names for the dynamic component
    */
-  protected inAndOutputNames: (keyof this)[] = [
-    ...this.inAndOutputNames,
+  protected inputNames: (keyof this & string)[] = [
+    ...this.inputNames,
     'item',
     'object',
     'option',
     'workflowitem',
+  ];
+
+  protected outputNames: (keyof this & string)[] = [
+    ...this.outputNames,
     'processCompleted',
   ];
 

src/app/shared/object-collection/shared/listable-object/listable-object-component-loader.component.ts

@@ -1,5 +1,4 @@
-import { ChangeDetectorRef, Component, EventEmitter, Input, Output, SimpleChanges } from '@angular/core';
-import { combineLatest, Observable, of as observableOf } from 'rxjs';
+import { ChangeDetectorRef, Component, EventEmitter, Input, Output } from '@angular/core';
 import { take } from 'rxjs/operators';
 import { ListableObject } from '../listable-object.model';
 import { ViewMode } from '../../../../core/shared/view-mode.model';
@@ -74,8 +73,8 @@ export class ListableObjectComponentLoaderComponent extends AbstractComponentLoa
   /**
    * The list of input and output names for the dynamic component
    */
-  protected inAndOutputNames: (keyof this)[] = [
-    ...this.inAndOutputNames,
+  protected inputNames: (keyof this & string)[] = [
+    ...this.inputNames,
     'object',
     'index',
     'linkType',
@@ -84,6 +83,10 @@ export class ListableObjectComponentLoaderComponent extends AbstractComponentLoa
     'showThumbnails',
     'viewMode',
     'value',
+  ];
+
+  protected outputNames: (keyof this & string)[] = [
+    ...this.outputNames,
     'contentChange',
   ];
 
@@ -94,17 +97,16 @@ export class ListableObjectComponentLoaderComponent extends AbstractComponentLoa
     super(themeService);
   }
 
-  public instantiateComponent(changes?: SimpleChanges): void {
-    super.instantiateComponent(changes);
+  public instantiateComponent(): void {
+    super.instantiateComponent();
     if ((this.compRef.instance as any).reloadedObject) {
-      combineLatest([
-        observableOf(changes),
-        (this.compRef.instance as any).reloadedObject.pipe(take(1)) as Observable<DSpaceObject>,
-      ]).subscribe(([simpleChanges, reloadedObject]: [SimpleChanges, DSpaceObject]) => {
+      (this.compRef.instance as any).reloadedObject.pipe(
+        take(1),
+      ).subscribe((reloadedObject: DSpaceObject) => {
         if (reloadedObject) {
-          this.compRef.destroy();
+          this.destroyComponentInstance();
           this.object = reloadedObject;
-          this.instantiateComponent(simpleChanges);
+          this.instantiateComponent();
           this.cdr.detectChanges();
           this.contentChange.emit(reloadedObject);
         }

src/app/workflowitems-edit-page/advanced-workflow-action/advanced-workflow-actions-loader/advanced-workflow-actions-loader.component.ts

@@ -24,8 +24,8 @@ export class AdvancedWorkflowActionsLoaderComponent extends AbstractComponentLoa
    */
   @Input() type: string;
 
-  protected inAndOutputNames: (keyof this)[] = [
-    ...this.inAndOutputNames,
+  protected inputNames: (keyof this & string)[] = [
+    ...this.inputNames,
     'type',
   ];