fix(editor): Fix memory leak in Node Detail View by correctly unsubscribing from event buses (#6021)

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

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

cypress/e2e/14-mapping.cy.ts

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

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

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

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

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

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

@@ -121,15 +121,8 @@ export default Vue.extend({
 	mounted() {
 		window.addEventListener('keydown', this.onWindowKeydown);
 
-		if (this.eventBus) {
+		this.eventBus?.on('close', this.closeDialog);
-			this.eventBus.on('close', () => {
+		this.eventBus?.on('closeAll', this.uiStore.closeAllModals);
-				this.closeDialog();
-			});
-
-			this.eventBus.on('closeAll', () => {
-				this.uiStore.closeAllModals();
-			});
-		}
 
 		const activeElement = document.activeElement as HTMLElement;
 		if (activeElement) {
@@ -137,6 +130,8 @@ export default Vue.extend({
 		}
 	},
 	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,12 +53,7 @@ export default Vue.extend({
 	},
 	mounted() {
 		window.addEventListener('keydown', this.onWindowKeydown);
-
+		this.eventBus?.on('close', this.close);
-		if (this.eventBus) {
-			this.eventBus.on('close', () => {
-				this.close();
-			});
-		}
 
 		const activeElement = document.activeElement as HTMLElement;
 		if (activeElement) {
@@ -66,6 +61,7 @@ export default Vue.extend({
 		}
 	},
 	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(
+		dataPinningEventBus.on('data-pinning-discovery', this.setIsTooltipVisible);
-			'data-pinning-discovery',
-			({ isTooltipVisible }: { isTooltipVisible: boolean }) => {
-				this.pinDataDiscoveryTooltipVisible = isTooltipVisible;
-			},
-		);
 	},
 	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.openSettings);
-			this.eventBus.on('openSettings', () => {
-				this.openPanel = 'settings';
-			});
-		}
 
 		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);
+			dataPinningEventBus.on('data-pinning-error', this.onDataPinningError);
-			this.eventBus.on('data-unpinning', this.onDataUnpinning);
+			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);
+		dataPinningEventBus.off('data-pinning-error', this.onDataPinningError);
-		this.eventBus.off('data-unpinning', this.onDataUnpinning);
+		dataPinningEventBus.off('data-unpinning', this.onDataUnpinning);
 	},
 	computed: {
 		...mapStores(useNodeTypesStore, useNDVStore, useWorkflowsStore),
@@ -908,7 +907,7 @@ 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
 			}
 		},
@@ -916,7 +915,7 @@ export default mixins(externalHooks, genericHelpers, nodeHelpers, pinData).exten
 			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', () => {
+		this.eventBus?.on('destinationWasSaved', this.onDestinationWasSaved);
-			const updatedDestination = this.logStreamingStore.getDestination(this.destination.id);
+	},
-			if (updatedDestination) {
+	destroyed() {
-				this.nodeParameters = Object.assign(
+		this.eventBus?.off('destinationWasSaved', this.onDestinationWasSaved);
-					deepCopy(defaultMessageEventBusDestinationOptions),
-					this.destination,
-				);
-			}
-		});
 	},
 	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.onBusFocus);
-			this.eventBus.on('focus', () => {
-				this.focusOnInput();
-				this.focusOnTopOption();
-			});
-		}
 
 		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
+			window.addEventListener('beforeunload', this.onBeforeUnload);
-			// @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);
 		},
 		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.eventBus.on('destinationWasSaved', this.onDestinationWasSaved);
-			this.$forceUpdate();
-		});
 		// listen to remove emission
-		this.eventBus.on('remove', async (destinationId: string) => {
+		this.eventBus.on('remove', this.onRemove);
-			await this.onRemove(destinationId);
-		});
 		// listen to modal closing and remove nodes from store
-		this.eventBus.on('closing', async (destinationId: string) => {
+		this.eventBus.on('closing', this.onBusClosing);
-			this.workflowsStore.removeAllNodes({ setStateDirty: false, removePinData: true });
+	},
-			this.uiStore.stateIsDirty = false;
+	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();