Mirrors/uppy
1
0
Fork 0
mirror of https://github.com/transloadit/uppy.git synced 2026-07-17 16:50:22 +00:00
Code Issues Projects Releases Wiki Activity

Fixed #6265: Memory leak in 3 angualr related classes

Browse source 
Memory leak in 3 classes that instantiate Uppy (& document listeners)
but fail to destroy it - derived from UppyAngularWrapper.

See issue description for more info.
This commit is contained in:
Andrei Costescu 2026-07-09 23:37:58 +03:00
parent 0b1f79c90d
commit 973a4d83b3
No known key found for this signature in database
GPG key ID: 3A6C1D7AA91CBE58
4 changed files with 56 additions and 14 deletions
Download patch file Download diff file Expand all files Collapse all files

2
packages/@uppy/angular/projects/uppy/angular/src/lib/components/dashboard-modal/dashboard-modal.component.ts
View file

@ -26,7 +26,7 @@ export class DashboardModalComponent<M extends Meta, B extends Body>
{
el = inject(ElementRef);
@Input() uppy: Uppy<M, B> = new Uppy();
@Input() uppy: Uppy<M, B> = this.createInitialUppy();
@Input() props: DashboardOptions<M, B> = {};
@Input() open: boolean = false;

2
packages/@uppy/angular/projects/uppy/angular/src/lib/components/dashboard/dashboard.component.ts
View file

@ -26,7 +26,7 @@ export class DashboardComponent<M extends Meta, B extends Body>
{
el = inject(ElementRef);
@Input() uppy: Uppy<M, B> = new Uppy();
@Input() uppy: Uppy<M, B> = this.createInitialUppy();
@Input() props: DashboardOptions<M, B> = {};
/** Inserted by Angular inject() migration for backwards compatibility */

2
packages/@uppy/angular/projects/uppy/angular/src/lib/components/status-bar/status-bar.component.ts
View file

@ -26,7 +26,7 @@ export class StatusBarComponent<M extends Meta, B extends Body>
{
el = inject(ElementRef);
@Input() uppy: Uppy<M, B> = new Uppy();
@Input() uppy: Uppy<M, B> = this.createInitialUppy();
@Input() props: StatusBarOptions = {};
/** Inserted by Angular inject() migration for backwards compatibility */

64
packages/@uppy/angular/projects/uppy/angular/src/lib/utils/wrapper.ts
View file

@ -1,6 +1,7 @@
import type { ElementRef, SimpleChanges } from "@angular/core";
import type { UIPlugin, UIPluginOptions, Uppy, Body, Meta } from "@uppy/core";
import type { UIPlugin, UIPluginOptions, Body, Meta } from "@uppy/core";
import type { DashboardOptions } from "@uppy/dashboard";
import { Uppy } from "@uppy/core";
export abstract class UppyAngularWrapper<
M extends Meta,
@ -13,6 +14,19 @@ export abstract class UppyAngularWrapper<
abstract uppy: Uppy<M, B>;
private options: any;
plugin: PluginType | undefined;
private uppyInstanceManagedByThisClass?: Uppy<M, B>;
/**
* If extending classes need an initial instance of Uppy - use this one; it will be destroyed when needed to avoid memory leaks.
*/
protected createInitialUppy(): Uppy<M, B> {
if (!this.uppyInstanceManagedByThisClass)
// the instance of Uppy that is created here - we are in charge of destroying it when it's no longer used
this.uppyInstanceManagedByThisClass = new Uppy();
return this.uppyInstanceManagedByThisClass;
}
onMount(
defaultOptions: Partial<Opts>,
@ -28,18 +42,33 @@ export abstract class UppyAngularWrapper<
}
handleChanges(changes: SimpleChanges, plugin: any): void {
// Without the last part of this conditional, it tries to uninstall before the plugin is mounted
if (
changes["uppy"] &&
this.uppy !== changes["uppy"].previousValue &&
changes["uppy"].previousValue !== undefined
) {
this.uninstall(changes["uppy"].previousValue);
// @ts-expect-error The options correspond to the plugin, I swear
this.uppy.use(plugin, this.options);
if (changes["uppy"])
{
// "uppy" should never get set to null/undefined - that is not supported
// Without the undefined check below, initially it would try to uninstall before the plugin is mounted (ngOnChanges
// that calls this gets called before ngOnInit that calls onMount if uppy is an @Input in derived classes; so the
// "uppyInstanceManagedByThisClass" didn't even get to be used; and changes["uppy"].previousValue doesn't give
// that anyway initially, but undefined, as that value was a 'default')
if (this.uppy !== changes["uppy"].previousValue && changes["uppy"].previousValue !== undefined) {
// we had an actively used Uppy that now changed
this.uninstall(changes["uppy"].previousValue); // this also destroys old Uppy if needed
// @ts-expect-error The options correspond to the plugin, I swear
this.uppy.use(plugin, this.options);
} else {
// can get here from derived classes on the initial ngOnChanges call of @Input uppy for example; previousValue
// is undefined then, even if we did have a this.uppy from constructor (now replaced) that was not yet making
// use of the plugin; so we don't uninstall (that also does destroy) here, just destroy if needed
let previousValueIncludingDefault = changes["uppy"].previousValue ? changes["uppy"].previousValue
: this.uppyInstanceManagedByThisClass;
if (this.uppy !== previousValueIncludingDefault)
this.destroyUppyIfNeeed(previousValueIncludingDefault);
}
}
this.options = { ...this.options, ...this.props };
this.plugin = this.uppy.getPlugin(this.options.id) as PluginType;
if (
changes["props"] &&
this.props !== changes["props"].previousValue &&
@ -49,7 +78,20 @@ export abstract class UppyAngularWrapper<
}
}
uninstall(uppy = this.uppy): void {
uninstall(uppy: Uppy<M, B> = this.uppy): void {
uppy.removePlugin(this.plugin!);
this.destroyUppyIfNeeed(uppy);
}
private destroyUppyIfNeeed(uppyThatWillNoLongerBeUsed: Uppy<M, B>): void {
// "uppyThatWillNoLongerBeUsed" is either the "uppyInstanceManagedByThisClass" or it's received via derived classes
// from the outside (as an "@input" for example); if the Uppy that this instance created is no longer used,
// we need to destroy it in order to avoid memory leaks
if (uppyThatWillNoLongerBeUsed === this.uppyInstanceManagedByThisClass) {
this.uppyInstanceManagedByThisClass.destroy();
delete this.uppyInstanceManagedByThisClass;
} // else whoever provided the Uppy instance (as @Input maybe) is in charge of destroying it
}
}