Skip to content

Commit

Permalink
Merge branch 'memory-leak-fixes_contribute-7.4' into memory-leak-fixe…
Browse files Browse the repository at this point in the history
…s_contribute-7.6

# Conflicts:
#	src/app/admin/admin-search-page/admin-search-results/admin-search-result-grid-element/item-search-result/item-admin-search-result-grid-element.component.ts
#	src/app/admin/admin-workflow-page/admin-workflow-search-results/admin-workflow-search-result-grid-element/workflow-item/workflow-item-search-result-admin-workflow-grid-element.component.ts
#	src/app/shared/metadata-representation/metadata-representation-loader.component.ts
#	src/app/shared/mydspace-actions/claimed-task/switcher/claimed-task-actions-loader.component.ts
#	src/app/shared/theme-support/themed.component.ts
  • Loading branch information
alexandrevryghem committed Dec 28, 2023
2 parents 404ccd9 + 480c7a6 commit b28e24f
Show file tree
Hide file tree
Showing 7 changed files with 95 additions and 58 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Component, ComponentFactoryResolver, ElementRef, OnInit, ViewChild } from '@angular/core';
import { Component, ElementRef, OnInit, ViewChild, ComponentRef, OnDestroy } from '@angular/core';
import { Item } from '../../../../../core/shared/item.model';
import { ViewMode } from '../../../../../core/shared/view-mode.model';
import {
Expand All @@ -14,6 +14,7 @@ import { GenericConstructor } from '../../../../../core/shared/generic-construct
import { ListableObjectDirective } from '../../../../../shared/object-collection/shared/listable-object/listable-object.directive';
import { ThemeService } from '../../../../../shared/theme-support/theme.service';
import { DSONameService } from '../../../../../core/breadcrumbs/dso-name.service';
import { hasValue } from '../../../../../shared/empty.util';

@listableObjectComponent(ItemSearchResult, ViewMode.GridElement, Context.AdminSearch)
@Component({
Expand All @@ -24,17 +25,18 @@ import { DSONameService } from '../../../../../core/breadcrumbs/dso-name.service
/**
* The component for displaying a list element for an item search result on the admin search page
*/
export class ItemAdminSearchResultGridElementComponent extends SearchResultGridElementComponent<ItemSearchResult, Item> implements OnInit {
export class ItemAdminSearchResultGridElementComponent extends SearchResultGridElementComponent<ItemSearchResult, Item> implements OnDestroy, OnInit {
@ViewChild(ListableObjectDirective, { static: true }) listableObjectDirective: ListableObjectDirective;
@ViewChild('badges', { static: true }) badges: ElementRef;
@ViewChild('buttons', { static: true }) buttons: ElementRef;

protected compRef: ComponentRef<Component>;

constructor(
public dsoNameService: DSONameService,
protected truncatableService: TruncatableService,
protected bitstreamDataService: BitstreamDataService,
private themeService: ThemeService,
private componentFactoryResolver: ComponentFactoryResolver,
) {
super(dsoNameService, truncatableService, bitstreamDataService);
}
Expand All @@ -44,23 +46,32 @@ export class ItemAdminSearchResultGridElementComponent extends SearchResultGridE
*/
ngOnInit(): void {
super.ngOnInit();
const componentFactory = this.componentFactoryResolver.resolveComponentFactory(this.getComponent());
const component: GenericConstructor<Component> = this.getComponent();

const viewContainerRef = this.listableObjectDirective.viewContainerRef;
viewContainerRef.clear();

const componentRef = viewContainerRef.createComponent(
componentFactory,
0,
undefined,
[
[this.badges.nativeElement],
[this.buttons.nativeElement]
]);
(componentRef.instance as any).object = this.object;
(componentRef.instance as any).index = this.index;
(componentRef.instance as any).linkType = this.linkType;
(componentRef.instance as any).listID = this.listID;
this.compRef = viewContainerRef.createComponent(
component, {
index: 0,
injector: undefined,
projectableNodes: [
[this.badges.nativeElement],
[this.buttons.nativeElement],
],
},
);
(this.compRef.instance as any).object = this.object;
(this.compRef.instance as any).index = this.index;
(this.compRef.instance as any).linkType = this.linkType;
(this.compRef.instance as any).listID = this.listID;
}

ngOnDestroy(): void {
if (hasValue(this.compRef)) {
this.compRef.destroy();
this.compRef = undefined;
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Component, ComponentFactoryResolver, ElementRef, ViewChild } from '@angular/core';
import { Component, ElementRef, ViewChild, ComponentRef, OnDestroy, OnInit } from '@angular/core';
import { Item } from '../../../../../core/shared/item.model';
import { ViewMode } from '../../../../../core/shared/view-mode.model';
import {
Expand All @@ -24,6 +24,7 @@ import { take } from 'rxjs/operators';
import { WorkflowItemSearchResult } from '../../../../../shared/object-collection/shared/workflow-item-search-result.model';
import { ThemeService } from '../../../../../shared/theme-support/theme.service';
import { DSONameService } from '../../../../../core/breadcrumbs/dso-name.service';
import { hasValue } from '../../../../../shared/empty.util';

@listableObjectComponent(WorkflowItemSearchResult, ViewMode.GridElement, Context.AdminWorkflowSearch)
@Component({
Expand All @@ -34,7 +35,7 @@ import { DSONameService } from '../../../../../core/breadcrumbs/dso-name.service
/**
* The component for displaying a grid element for an workflow item on the admin workflow search page
*/
export class WorkflowItemSearchResultAdminWorkflowGridElementComponent extends SearchResultGridElementComponent<WorkflowItemSearchResult, WorkflowItem> {
export class WorkflowItemSearchResultAdminWorkflowGridElementComponent extends SearchResultGridElementComponent<WorkflowItemSearchResult, WorkflowItem> implements OnDestroy, OnInit {
/**
* Directive used to render the dynamic component in
*/
Expand All @@ -55,9 +56,10 @@ export class WorkflowItemSearchResultAdminWorkflowGridElementComponent extends S
*/
public item$: Observable<Item>;

protected compRef: ComponentRef<Component>;

constructor(
public dsoNameService: DSONameService,
private componentFactoryResolver: ComponentFactoryResolver,
private linkService: LinkService,
protected truncatableService: TruncatableService,
private themeService: ThemeService,
Expand All @@ -75,28 +77,37 @@ export class WorkflowItemSearchResultAdminWorkflowGridElementComponent extends S
this.dso = this.linkService.resolveLink(this.dso, followLink('item'));
this.item$ = (this.dso.item as Observable<RemoteData<Item>>).pipe(getAllSucceededRemoteData(), getRemoteDataPayload());
this.item$.pipe(take(1)).subscribe((item: Item) => {
const componentFactory = this.componentFactoryResolver.resolveComponentFactory(this.getComponent(item));
const component: GenericConstructor<Component> = this.getComponent(item);

const viewContainerRef = this.listableObjectDirective.viewContainerRef;
viewContainerRef.clear();

const componentRef = viewContainerRef.createComponent(
componentFactory,
0,
undefined,
[
[this.badges.nativeElement],
[this.buttons.nativeElement]
]);
(componentRef.instance as any).object = item;
(componentRef.instance as any).index = this.index;
(componentRef.instance as any).linkType = this.linkType;
(componentRef.instance as any).listID = this.listID;
componentRef.changeDetectorRef.detectChanges();
this.compRef = viewContainerRef.createComponent(
component, {
index: 0,
injector: undefined,
projectableNodes: [
[this.badges.nativeElement],
[this.buttons.nativeElement],
],
},
);
(this.compRef.instance as any).object = item;
(this.compRef.instance as any).index = this.index;
(this.compRef.instance as any).linkType = this.linkType;
(this.compRef.instance as any).listID = this.listID;
this.compRef.changeDetectorRef.detectChanges();
}
);
}

ngOnDestroy(): void {
if (hasValue(this.compRef)) {
this.compRef.destroy();
this.compRef = undefined;
}
}

/**
* Fetch the component depending on the item's entity type, view mode and context
* @returns {GenericConstructor<Component>}
Expand Down
5 changes: 2 additions & 3 deletions src/app/shared/loading/themed-loading.component.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Component, Input, ComponentFactoryResolver, ChangeDetectorRef } from '@angular/core';
import { Component, Input, ChangeDetectorRef } from '@angular/core';
import { ThemedComponent } from '../theme-support/themed.component';
import { LoadingComponent } from './loading.component';
import { ThemeService } from '../theme-support/theme.service';
Expand All @@ -20,11 +20,10 @@ export class ThemedLoadingComponent extends ThemedComponent<LoadingComponent> {
protected inAndOutputNames: (keyof LoadingComponent & keyof this)[] = ['message', 'showMessage', 'spinner'];

constructor(
protected resolver: ComponentFactoryResolver,
protected cdr: ChangeDetectorRef,
protected themeService: ThemeService
) {
super(resolver, cdr, themeService);
super(cdr, themeService);
}

protected getComponentName(): string {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Component, ComponentFactoryResolver, Inject, Input, OnInit, ViewChild, OnChanges, SimpleChanges, ComponentRef, ViewContainerRef, ComponentFactory } from '@angular/core';
import { Component, Inject, Input, OnInit, ViewChild, OnChanges, OnDestroy, SimpleChanges, ComponentRef, ViewContainerRef } from '@angular/core';
import {
MetadataRepresentation,
MetadataRepresentationType
Expand All @@ -18,8 +18,7 @@ import { ThemeService } from '../theme-support/theme.service';
/**
* Component for determining what component to use depending on the item's entity type (dspace.entity.type), its metadata representation and, optionally, its context
*/
export class MetadataRepresentationLoaderComponent implements OnInit, OnChanges {

export class MetadataRepresentationLoaderComponent implements OnDestroy, OnInit, OnChanges {
/**
* The item or metadata to determine the component for
*/
Expand Down Expand Up @@ -55,7 +54,6 @@ export class MetadataRepresentationLoaderComponent implements OnInit, OnChanges
];

constructor(
private componentFactoryResolver: ComponentFactoryResolver,
private themeService: ThemeService,
@Inject(METADATA_REPRESENTATION_COMPONENT_FACTORY) private getMetadataRepresentationComponent: (entityType: string, mdRepresentationType: MetadataRepresentationType, context: Context, theme: string) => GenericConstructor<any>,
) {
Expand All @@ -65,7 +63,9 @@ export class MetadataRepresentationLoaderComponent implements OnInit, OnChanges
* Set up the dynamic child component
*/
ngOnInit(): void {
this.instantiateComponent();
if (hasNoValue(this.compRef)) {
this.instantiateComponent();
}
}

/**
Expand All @@ -88,12 +88,12 @@ export class MetadataRepresentationLoaderComponent implements OnInit, OnChanges
}

private instantiateComponent(changes?: SimpleChanges): void {
const componentFactory: ComponentFactory<MetadataRepresentationListElementComponent> = this.componentFactoryResolver.resolveComponentFactory(this.getComponent());
const component: GenericConstructor<MetadataRepresentationListElementComponent> = this.getComponent();

const viewContainerRef: ViewContainerRef = this.mdRepDirective.viewContainerRef;
viewContainerRef.clear();

this.compRef = viewContainerRef.createComponent(componentFactory);
this.compRef = viewContainerRef.createComponent(component);

if (hasValue(changes)) {
this.ngOnChanges(changes);
Expand All @@ -102,6 +102,13 @@ export class MetadataRepresentationLoaderComponent implements OnInit, OnChanges
}
}

ngOnDestroy(): void {
if (hasValue(this.compRef)) {
this.compRef.destroy();
this.compRef = undefined;
}
}

/**
* Fetch the component depending on the item's entity type, metadata representation type and context
* @returns {string}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import {
Component,
ComponentFactoryResolver,
EventEmitter,
Input,
OnDestroy,
OnInit,
Output,
ViewChild,
Expand All @@ -27,7 +27,7 @@ import { ClaimedTaskActionsAbstractComponent } from '../abstract/claimed-task-ac
* 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 implements OnInit, OnChanges {
export class ClaimedTaskActionsLoaderComponent implements OnInit, OnChanges, OnDestroy {
/**
* The item object that belonging to the ClaimedTask object
*/
Expand Down Expand Up @@ -73,14 +73,13 @@ export class ClaimedTaskActionsLoaderComponent implements OnInit, OnChanges {
'processCompleted',
];

constructor(private componentFactoryResolver: ComponentFactoryResolver) {
}

/**
* Fetch, create and initialize the relevant component
*/
ngOnInit(): void {
this.instantiateComponent();
if (hasNoValue(this.compRef)) {
this.instantiateComponent();
}
}

/**
Expand All @@ -105,12 +104,10 @@ export class ClaimedTaskActionsLoaderComponent implements OnInit, OnChanges {
private instantiateComponent(changes?: SimpleChanges): void {
const comp = this.getComponentByWorkflowTaskOption(this.option);
if (hasValue(comp)) {
const componentFactory = this.componentFactoryResolver.resolveComponentFactory(comp);

const viewContainerRef = this.claimedTaskActionsDirective.viewContainerRef;
viewContainerRef.clear();

this.compRef = viewContainerRef.createComponent(componentFactory);
this.compRef = viewContainerRef.createComponent(comp);

if (hasValue(changes)) {
this.ngOnChanges(changes);
Expand All @@ -135,4 +132,11 @@ export class ClaimedTaskActionsLoaderComponent implements OnInit, OnChanges {
});
}
}

ngOnDestroy(): void {
if (hasValue(this.compRef)) {
this.compRef.destroy();
this.compRef = undefined;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,9 @@ export class ListableObjectComponentLoaderComponent implements OnInit, OnChanges
* Setup the dynamic child component
*/
ngOnInit(): void {
this.instantiateComponent(this.object);
if (hasNoValue(this.compRef)) {
this.instantiateComponent(this.object);
}
}

/**
Expand All @@ -147,7 +149,11 @@ export class ListableObjectComponentLoaderComponent implements OnInit, OnChanges
}
}

ngOnDestroy() {
ngOnDestroy(): void {
if (hasValue(this.compRef)) {
this.compRef.destroy();
this.compRef = undefined;
}
this.subs
.filter((subscription) => hasValue(subscription))
.forEach((subscription) => subscription.unsubscribe());
Expand Down
7 changes: 3 additions & 4 deletions src/app/shared/theme-support/themed.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {
SimpleChanges,
OnInit,
OnDestroy,
ComponentFactoryResolver,
ChangeDetectorRef,
OnChanges,
HostBinding,
Expand Down Expand Up @@ -47,7 +46,6 @@ export abstract class ThemedComponent<T> implements OnInit, OnDestroy, OnChanges
@HostBinding('attr.data-used-theme') usedTheme: string;

constructor(
protected resolver: ComponentFactoryResolver,
protected cdr: ChangeDetectorRef,
protected themeService: ThemeService,
) {
Expand Down Expand Up @@ -120,8 +118,9 @@ export abstract class ThemedComponent<T> implements OnInit, OnDestroy, OnChanges
}

this.lazyLoadSub = this.lazyLoadObs.subscribe(([simpleChanges, constructor]: [SimpleChanges, GenericConstructor<T>]) => {
const factory = this.resolver.resolveComponentFactory(constructor);
this.compRef = this.vcr.createComponent(factory, undefined, undefined, [this.themedElementContent.nativeElement.childNodes]);
this.compRef = this.vcr.createComponent(constructor, {
projectableNodes: [this.themedElementContent.nativeElement.childNodes],
});
if (hasValue(simpleChanges)) {
this.ngOnChanges(simpleChanges);
} else {
Expand Down

0 comments on commit b28e24f

Please sign in to comment.