From 1e82b7796c19fb578b6f19baab5ac676bad81eee Mon Sep 17 00:00:00 2001 From: Noel De Martin Date: Tue, 15 Jun 2021 16:16:20 +0200 Subject: [PATCH] MOBILE-3320 dom: Store instances using a WeakMap --- .../components/context-menu/context-menu.ts | 8 ++----- .../navbar-buttons/navbar-buttons.ts | 11 ++++------ .../components/tabs-outlet/tabs-outlet.ts | 2 +- src/core/components/tabs/tab.ts | 2 +- src/core/services/utils/dom.ts | 22 +++++-------------- 5 files changed, 13 insertions(+), 32 deletions(-) diff --git a/src/core/components/context-menu/context-menu.ts b/src/core/components/context-menu/context-menu.ts index 73e8c9393..95b084efa 100644 --- a/src/core/components/context-menu/context-menu.ts +++ b/src/core/components/context-menu/context-menu.ts @@ -40,13 +40,10 @@ export class CoreContextMenuComponent implements OnInit, OnDestroy { protected items: CoreContextMenuItemComponent[] = []; protected itemsMovedToParent: CoreContextMenuItemComponent[] = []; protected itemsChangedStream: Subject; // Stream to update the hideMenu boolean when items change. - protected instanceId: string; protected parentContextMenu?: CoreContextMenuComponent; protected expanded = false; - constructor( - elementRef: ElementRef, - ) { + constructor(elementRef: ElementRef) { // Create the stream and subscribe to it. We ignore successive changes during 250ms. this.itemsChangedStream = new Subject(); this.itemsChangedStream.pipe(auditTime(250)); @@ -61,7 +58,7 @@ export class CoreContextMenuComponent implements OnInit, OnDestroy { // Calculate the unique ID. this.uniqueId = 'core-context-menu-' + CoreUtils.getUniqueId('CoreContextMenuComponent'); - this.instanceId = CoreDomUtils.storeInstanceByElement(elementRef.nativeElement, this); + CoreDomUtils.storeInstanceByElement(elementRef.nativeElement, this); } /** @@ -202,7 +199,6 @@ export class CoreContextMenuComponent implements OnInit, OnDestroy { * Component destroyed. */ ngOnDestroy(): void { - CoreDomUtils.removeInstanceById(this.instanceId); this.removeMergedItems(); } diff --git a/src/core/components/navbar-buttons/navbar-buttons.ts b/src/core/components/navbar-buttons/navbar-buttons.ts index cb4534642..f8363b1a6 100644 --- a/src/core/components/navbar-buttons/navbar-buttons.ts +++ b/src/core/components/navbar-buttons/navbar-buttons.ts @@ -62,13 +62,13 @@ export class CoreNavBarButtonsComponent implements OnInit, OnDestroy { protected forceHidden = false; protected logger: CoreLogger; protected movedChildren?: Node[]; - protected instanceId: string; protected mergedContextMenu?: CoreContextMenuComponent; constructor(element: ElementRef) { this.element = element.nativeElement; this.logger = CoreLogger.getInstance('CoreNavBarButtonsComponent'); - this.instanceId = CoreDomUtils.storeInstanceByElement(this.element, this); + + CoreDomUtils.storeInstanceByElement(this.element, this); } /** @@ -138,9 +138,8 @@ export class CoreNavBarButtonsComponent implements OnInit, OnDestroy { } // Both containers have a context menu. Merge them to prevent having 2 menus at the same time. - const mainContextMenuInstance: CoreContextMenuComponent = CoreDomUtils.getInstanceByElement(mainContextMenu); - const secondaryContextMenuInstance: CoreContextMenuComponent = - CoreDomUtils.getInstanceByElement(secondaryContextMenu); + const mainContextMenuInstance = CoreDomUtils.getInstanceByElement(mainContextMenu); + const secondaryContextMenuInstance = CoreDomUtils.getInstanceByElement(secondaryContextMenu); // Check that both context menus belong to the same core-tab. We shouldn't merge menus from different tabs. if (mainContextMenuInstance && secondaryContextMenuInstance) { @@ -247,8 +246,6 @@ export class CoreNavBarButtonsComponent implements OnInit, OnDestroy { * Component destroyed. */ ngOnDestroy(): void { - CoreDomUtils.removeInstanceById(this.instanceId); - // This component was destroyed, remove all the buttons that were moved. // The buttons can be moved outside of the current page, that's why we need to manually destroy them. // There's no need to destroy context menu items that were merged because they weren't moved from their DOM position. diff --git a/src/core/components/tabs-outlet/tabs-outlet.ts b/src/core/components/tabs-outlet/tabs-outlet.ts index 7c82bdd88..2ccf498e3 100644 --- a/src/core/components/tabs-outlet/tabs-outlet.ts +++ b/src/core/components/tabs-outlet/tabs-outlet.ts @@ -150,7 +150,7 @@ export class CoreTabsOutletComponent extends CoreTabsBaseComponent { - const instance: CoreNavBarButtonsComponent = domUtils.getInstanceByElement(element); + const instance = domUtils.getInstanceByElement(element); if (instance) { const pagetagName = element.closest('.ion-page')?.tagName; diff --git a/src/core/components/tabs/tab.ts b/src/core/components/tabs/tab.ts index 8f11dd03e..0fb0865fb 100644 --- a/src/core/components/tabs/tab.ts +++ b/src/core/components/tabs/tab.ts @@ -144,7 +144,7 @@ export class CoreTabComponent implements OnInit, OnDestroy, CoreTabBase { protected showHideNavBarButtons(show: boolean): void { const elements = this.element.querySelectorAll('core-navbar-buttons'); elements.forEach((element) => { - const instance: CoreNavBarButtonsComponent = CoreDomUtils.getInstanceByElement(element); + const instance = CoreDomUtils.getInstanceByElement(element); if (instance) { instance.forceHide(!show); diff --git a/src/core/services/utils/dom.ts b/src/core/services/utils/dom.ts index a5fefb235..5895170ad 100644 --- a/src/core/services/utils/dom.ts +++ b/src/core/services/utils/dom.ts @@ -53,9 +53,7 @@ export class CoreDomUtilsProvider { protected template: HTMLTemplateElement = document.createElement('template'); // A template element to convert HTML to element. protected matchesFunctionName?: string; // Name of the "matches" function to use when simulating a closest call. - // eslint-disable-next-line @typescript-eslint/no-explicit-any - protected instances: {[id: string]: any} = {}; // Store component/directive instances by id. - protected lastInstanceId = 0; + protected instances: WeakMap = new WeakMap(); // Store component/directive instances indexed by element. protected debugDisplay = false; // Whether to display debug messages. Store it in a variable to make it synchronous. protected displayedAlerts: Record = {}; // To prevent duplicated alerts. protected activeLoadingModals: CoreIonLoadingElement[] = []; @@ -706,11 +704,8 @@ export class CoreDomUtilsProvider { * @param element The root element of the component/directive. * @return The instance, undefined if not found. */ - // eslint-disable-next-line @typescript-eslint/no-explicit-any - getInstanceByElement(element: Element): any { - const id = element.getAttribute(this.INSTANCE_ID_ATTR_NAME); - - return id && this.instances[id]; + getInstanceByElement(element: Element): T | undefined { + return this.instances.get(element) as T; } /** @@ -1629,16 +1624,9 @@ export class CoreDomUtilsProvider { * * @param element The root element of the component/directive. * @param instance The instance to store. - * @return ID to identify the instance. */ - // eslint-disable-next-line @typescript-eslint/no-explicit-any - storeInstanceByElement(element: Element, instance: any): string { - const id = String(this.lastInstanceId++); - - element.setAttribute(this.INSTANCE_ID_ATTR_NAME, id); - this.instances[id] = instance; - - return id; + storeInstanceByElement(element: Element, instance: unknown): void { + this.instances.set(element, instance); } /**