fix(editor): Fix memory leak in Node Detail View by correctly unsubsc…

…ribing from event buses (#6021)

cypress/e2e/14-data-transformation-expressions.cy.ts

@@ -12,11 +12,12 @@ describe('Data transformation expressions', () => {
 	beforeEach(() => {
 		wf.actions.visit();
 
-		cy.window()
-			// @ts-ignore
-			.then(
-				(win) => win.onBeforeUnloadNodeView && win.removeEventListener('beforeunload', win.onBeforeUnloadNodeView),
-			);
+		cy.window().then(
+			(win) => {
+				// @ts-ignore
+				win.preventNodeViewBeforeUnload = true;
+			},
+		);
 	});
 
 	it('$json + native string methods', () => {

cypress/e2e/14-mapping.cy.ts

@@ -17,11 +17,12 @@ describe('Data mapping', () => {
 	beforeEach(() => {
 		workflowPage.actions.visit();
 
-		cy.window()
-			// @ts-ignore
-			.then(
-				(win) => win.onBeforeUnloadNodeView && win.removeEventListener('beforeunload', win.onBeforeUnloadNodeView),
-			);
+		cy.window().then(
+			(win) => {
+				// @ts-ignore
+				win.preventNodeViewBeforeUnload = true;
+			},
+		);
 	});
 
 	it('maps expressions from table header', () => {

cypress/e2e/16-webhook-node.cy.ts

@@ -99,11 +99,12 @@ describe('Webhook Trigger node', async () => {
 	beforeEach(() => {
 		workflowPage.actions.visit();
 
-		cy.window()
-			// @ts-ignore
-			.then(
-				(win) => win.onBeforeUnloadNodeView && win.removeEventListener('beforeunload', win.onBeforeUnloadNodeView),
-			);
+		cy.window().then(
+			(win) => {
+				// @ts-ignore
+				win.preventNodeViewBeforeUnload = true;
+			},
+		);
 	});
 
 	it('should listen for a GET request', () => {

packages/editor-ui/src/components/ExpandableInput/ExpandableInputEdit.vue

@@ -37,12 +37,10 @@ export default Vue.extend({
 		if (this.autofocus && this.$refs.input) {
 			this.focus();
 		}
-
-		if (this.eventBus) {
-			this.eventBus.on('focus', () => {
-				this.focus();
-			});
-		}
+		this.eventBus?.on('focus', this.focus);
+	},
+	destroyed() {
+		this.eventBus?.off('focus', this.focus);
 	},
 	methods: {
 		focus() {

packages/editor-ui/src/components/Modal.vue

@@ -121,22 +121,17 @@ export default Vue.extend({
 	mounted() {
 		window.addEventListener('keydown', this.onWindowKeydown);
 
-		if (this.eventBus) {
-			this.eventBus.on('close', () => {
-				this.closeDialog();
-			});
-
-			this.eventBus.on('closeAll', () => {
-				this.uiStore.closeAllModals();
-			});
-		}
+		this.eventBus?.on('close', this.closeDialog);
+		this.eventBus?.on('closeAll', this.uiStore.closeAllModals);
 
 		const activeElement = document.activeElement as HTMLElement;
 		if (activeElement) {
 			activeElement.blur();
 		}
 	},
 	beforeDestroy() {
+		this.eventBus?.off('close', this.closeDialog);
+		this.eventBus?.off('closeAll', this.uiStore.closeAllModals);
 		window.removeEventListener('keydown', this.onWindowKeydown);
 	},
 	computed: {

packages/editor-ui/src/components/ModalDrawer.vue

@@ -53,19 +53,15 @@ export default Vue.extend({
 	},
 	mounted() {
 		window.addEventListener('keydown', this.onWindowKeydown);
-
-		if (this.eventBus) {
-			this.eventBus.on('close', () => {
-				this.close();
-			});
-		}
+		this.eventBus?.on('close', this.close);
 
 		const activeElement = document.activeElement as HTMLElement;
 		if (activeElement) {
 			activeElement.blur();
 		}
 	},
 	beforeDestroy() {
+		this.eventBus?.off('close', this.close);
 		window.removeEventListener('keydown', this.onWindowKeydown);
 	},
 	computed: {

packages/editor-ui/src/components/NodeDetailsView.vue

@@ -211,15 +211,10 @@ export default mixins(
 		};
 	},
 	mounted() {
-		dataPinningEventBus.on(
-			'data-pinning-discovery',
-			({ isTooltipVisible }: { isTooltipVisible: boolean }) => {
-				this.pinDataDiscoveryTooltipVisible = isTooltipVisible;
-			},
-		);
+		dataPinningEventBus.on('data-pinning-discovery', this.setIsTooltipVisible);
 	},
 	destroyed() {
-		dataPinningEventBus.off('data-pinning-discovery');
+		dataPinningEventBus.off('data-pinning-discovery', this.setIsTooltipVisible);
 	},
 	computed: {
 		...mapStores(useNodeTypesStore, useNDVStore, useUIStore, useWorkflowsStore, useSettingsStore),
@@ -480,6 +475,9 @@ export default mixins(
 		},
 	},
 	methods: {
+		setIsTooltipVisible({ isTooltipVisible }: { isTooltipVisible: boolean }) {
+			this.pinDataDiscoveryTooltipVisible = isTooltipVisible;
+		},
 		onKeyDown(e: KeyboardEvent) {
 			if (e.key === 's' && this.isCtrlKeyPressed(e)) {
 				e.stopPropagation();

packages/editor-ui/src/components/NodeSettings.vue

@@ -895,18 +895,20 @@ export default mixins(externalHooks, nodeHelpers).extend({
 		onStopExecution() {
 			this.$emit('stopExecution');
 		},
+		openSettings() {
+			this.openPanel = 'settings';
+		},
 	},
 	mounted() {
 		this.populateHiddenIssuesSet();
 		this.setNodeValues();
-		if (this.eventBus) {
-			this.eventBus.on('openSettings', () => {
-				this.openPanel = 'settings';
-			});
-		}
+		this.eventBus?.on('openSettings', this.openSettings);
 
 		this.updateNodeParameterIssues(this.node as INodeUi, this.nodeType);
 	},
+	destroyed() {
+		this.eventBus?.off('openSettings', this.openSettings);
+	},
 });
 </script>
 

packages/editor-ui/src/components/RunData.vue

@@ -585,7 +585,6 @@ export default mixins(externalHooks, genericHelpers, nodeHelpers, pinData).exten
 			currentPage: 1,
 			pageSize: 10,
 			pageSizes: [10, 25, 50, 100],
-			eventBus: dataPinningEventBus,
 
 			pinDataDiscoveryTooltipVisible: false,
 			isControlledPinDataTooltip: false,
@@ -595,8 +594,8 @@ export default mixins(externalHooks, genericHelpers, nodeHelpers, pinData).exten
 		this.init();
 
 		if (!this.isPaneTypeInput) {
-			this.eventBus.on('data-pinning-error', this.onDataPinningError);
-			this.eventBus.on('data-unpinning', this.onDataUnpinning);
+			dataPinningEventBus.on('data-pinning-error', this.onDataPinningError);
+			dataPinningEventBus.on('data-unpinning', this.onDataUnpinning);
 
 			this.showPinDataDiscoveryTooltip(this.jsonData);
 		}
@@ -609,8 +608,8 @@ export default mixins(externalHooks, genericHelpers, nodeHelpers, pinData).exten
 	},
 	destroyed() {
 		this.hidePinDataDiscoveryTooltip();
-		this.eventBus.off('data-pinning-error', this.onDataPinningError);
-		this.eventBus.off('data-unpinning', this.onDataUnpinning);
+		dataPinningEventBus.off('data-pinning-error', this.onDataPinningError);
+		dataPinningEventBus.off('data-unpinning', this.onDataUnpinning);
 	},
 	computed: {
 		...mapStores(useNodeTypesStore, useNDVStore, useWorkflowsStore),
@@ -908,15 +907,15 @@ export default mixins(externalHooks, genericHelpers, nodeHelpers, pinData).exten
 				setTimeout(() => {
 					this.isControlledPinDataTooltip = true;
 					this.pinDataDiscoveryTooltipVisible = true;
-					this.eventBus.emit('data-pinning-discovery', { isTooltipVisible: true });
+					dataPinningEventBus.emit('data-pinning-discovery', { isTooltipVisible: true });
 				}, 500); // Wait for NDV to open
 			}
 		},
 		hidePinDataDiscoveryTooltip() {
 			if (this.pinDataDiscoveryTooltipVisible) {
 				this.isControlledPinDataTooltip = false;
 				this.pinDataDiscoveryTooltipVisible = false;
-				this.eventBus.emit('data-pinning-discovery', { isTooltipVisible: false });
+				dataPinningEventBus.emit('data-pinning-discovery', { isTooltipVisible: false });
 			}
 		},
 		pinDataDiscoveryComplete() {

packages/editor-ui/src/components/SettingsLogStreaming/EventDestinationCard.ee.vue

@@ -92,15 +92,10 @@ export default mixins(showMessage).extend({
 			deepCopy(defaultMessageEventBusDestinationOptions),
 			this.destination,
 		);
-		this.eventBus.on('destinationWasSaved', () => {
-			const updatedDestination = this.logStreamingStore.getDestination(this.destination.id);
-			if (updatedDestination) {
-				this.nodeParameters = Object.assign(
-					deepCopy(defaultMessageEventBusDestinationOptions),
-					this.destination,
-				);
-			}
-		});
+		this.eventBus?.on('destinationWasSaved', this.onDestinationWasSaved);
+	},
+	destroyed() {
+		this.eventBus?.off('destinationWasSaved', this.onDestinationWasSaved);
 	},
 	computed: {
 		...mapStores(useLogStreamingStore),
@@ -124,6 +119,15 @@ export default mixins(showMessage).extend({
 		},
 	},
 	methods: {
+		onDestinationWasSaved() {
+			const updatedDestination = this.logStreamingStore.getDestination(this.destination.id);
+			if (updatedDestination) {
+				this.nodeParameters = Object.assign(
+					deepCopy(defaultMessageEventBusDestinationOptions),
+					this.destination,
+				);
+			}
+		},
 		async onClick(event?: PointerEvent) {
 			if (
 				event &&

packages/editor-ui/src/components/TagsDropdown.vue

@@ -117,15 +117,13 @@ export default mixins(showMessage).extend({
 			}
 		}
 
-		if (this.eventBus) {
-			this.eventBus.on('focus', () => {
-				this.focusOnInput();
-				this.focusOnTopOption();
-			});
-		}
+		this.eventBus?.on('focus', this.onBusFocus);
 
 		this.tagsStore.fetchAll();
 	},
+	destroyed() {
+		this.eventBus?.off('focus', this.onBusFocus);
+	},
 	computed: {
 		...mapStores(useTagsStore, useUIStore),
 		allTags(): ITag[] {
@@ -144,6 +142,10 @@ export default mixins(showMessage).extend({
 		},
 	},
 	methods: {
+		onBusFocus() {
+			this.focusOnInput();
+			this.focusOnTopOption();
+		},
 		filterOptions(filter = '') {
 			this.$data.filter = filter.trim();
 			this.$nextTick(() => this.focusOnTopOption());

packages/editor-ui/src/views/NodeView.vue

@@ -2516,6 +2516,20 @@ export default mixins(
 				}
 			}
 		},
+		onBeforeUnload(e) {
+			if (this.isDemo || window.preventNodeViewBeforeUnload) {
+				return;
+			} else if (this.uiStore.stateIsDirty) {
+				const confirmationMessage = this.$locale.baseText(
+					'nodeView.itLooksLikeYouHaveBeenEditingSomething',
+				);
+				(e || window.event).returnValue = confirmationMessage; //Gecko + IE
+				return confirmationMessage; //Gecko + Webkit, Safari, Chrome etc.
+			} else {
+				this.startLoading(this.$locale.baseText('nodeView.redirecting'));
+				return;
+			}
+		},
 		async newWorkflow(): Promise<void> {
 			this.startLoading();
 			await this.resetWorkspace();
@@ -2597,23 +2611,7 @@ export default mixins(
 			document.addEventListener('keydown', this.keyDown);
 			document.addEventListener('keyup', this.keyUp);
 
-			// allow to be overriden in e2e tests
-			// @ts-ignore
-			window.onBeforeUnloadNodeView = (e) => {
-				if (this.isDemo) {
-					return;
-				} else if (this.uiStore.stateIsDirty) {
-					const confirmationMessage = this.$locale.baseText(
-						'nodeView.itLooksLikeYouHaveBeenEditingSomething',
-					);
-					(e || window.event).returnValue = confirmationMessage; //Gecko + IE
-					return confirmationMessage; //Gecko + Webkit, Safari, Chrome etc.
-				} else {
-					this.startLoading(this.$locale.baseText('nodeView.redirecting'));
-					return;
-				}
-			};
-			window.addEventListener('beforeunload', window.onBeforeUnloadNodeView);
+			window.addEventListener('beforeunload', this.onBeforeUnload);
 		},
 		getOutputEndpointUUID(nodeName: string, index: number): string | null {
 			const node = this.workflowsStore.getNodeByName(nodeName);
@@ -3972,6 +3970,7 @@ export default mixins(
 		document.removeEventListener('keydown', this.keyDown);
 		document.removeEventListener('keyup', this.keyUp);
 		window.removeEventListener('message', this.onPostMessageReceived);
+		window.removeEventListener('beforeunload', this.onBeforeUnload);
 
 		this.$root.$off('newWorkflow', this.newWorkflow);
 		this.$root.$off('importWorkflowData', this.onImportWorkflowDataEvent);

packages/editor-ui/src/views/SettingsLogStreamingView.vue

@@ -135,18 +135,16 @@ export default mixins().extend({
 			}
 		});
 		// refresh when a modal closes
-		this.eventBus.on('destinationWasSaved', async () => {
-			this.$forceUpdate();
-		});
+		this.eventBus.on('destinationWasSaved', this.onDestinationWasSaved);
 		// listen to remove emission
-		this.eventBus.on('remove', async (destinationId: string) => {
-			await this.onRemove(destinationId);
-		});
+		this.eventBus.on('remove', this.onRemove);
 		// listen to modal closing and remove nodes from store
-		this.eventBus.on('closing', async (destinationId: string) => {
-			this.workflowsStore.removeAllNodes({ setStateDirty: false, removePinData: true });
-			this.uiStore.stateIsDirty = false;
-		});
+		this.eventBus.on('closing', this.onBusClosing);
+	},
+	destroyed() {
+		this.eventBus.off('destinationWasSaved', this.onDestinationWasSaved);
+		this.eventBus.off('remove', this.onRemove);
+		this.eventBus.off('closing', this.onBusClosing);
 	},
 	computed: {
 		...mapStores(
@@ -173,6 +171,13 @@ export default mixins().extend({
 		},
 	},
 	methods: {
+		onDestinationWasSaved() {
+			this.$forceUpdate();
+		},
+		onBusClosing() {
+			this.workflowsStore.removeAllNodes({ setStateDirty: false, removePinData: true });
+			this.uiStore.stateIsDirty = false;
+		},
 		async getDestinationDataFromBackend(): Promise<void> {
 			this.logStreamingStore.clearEventNames();
 			this.logStreamingStore.clearDestinationItemTrees();