From 151e60f829663e79982aae6ac1cd8489f3083224 Mon Sep 17 00:00:00 2001
From: Mutasem Aldmour <4711238+mutdmour@users.noreply.github.com>
Date: Tue, 7 Nov 2023 10:06:08 +0100
Subject: [PATCH] fix(editor): Fix local storage flags defaulting to undefined
 string (#7603)

useStorage takes the default value `undefined` and sets it in local
storage.. also returns "undefined" as string which breaks onboarding
flows

Github issue / Community forum post (link here to close automatically):
---
 .../src/components/ActivationModal.vue        |  3 +-
 .../src/components/NDVDraggablePanels.vue     |  9 ++--
 packages/editor-ui/src/components/Node.vue    |  9 ++--
 packages/editor-ui/src/components/RunData.vue | 11 ++---
 .../src/composables/useStorage.test.ts        | 48 +++++++++++++++++++
 .../editor-ui/src/composables/useStorage.ts   | 13 +++++
 .../editor-ui/src/mixins/workflowActivate.ts  |  7 +--
 packages/editor-ui/src/router.ts              |  5 +-
 .../src/stores/__tests__/posthog.test.ts      | 13 ++---
 packages/editor-ui/src/stores/ndv.store.ts    |  6 +--
 .../editor-ui/src/stores/posthog.store.ts     |  6 +--
 packages/editor-ui/src/stores/ui.utils.ts     |  9 ++--
 12 files changed, 97 insertions(+), 42 deletions(-)
 create mode 100644 packages/editor-ui/src/composables/useStorage.test.ts
 create mode 100644 packages/editor-ui/src/composables/useStorage.ts

diff --git a/packages/editor-ui/src/components/ActivationModal.vue b/packages/editor-ui/src/components/ActivationModal.vue
index 6b2e7274bc..a8c119c2e2 100644
--- a/packages/editor-ui/src/components/ActivationModal.vue
+++ b/packages/editor-ui/src/components/ActivationModal.vue
@@ -50,6 +50,7 @@ import { getActivatableTriggerNodes, getTriggerNodeServiceName } from '@/utils';
 import { useUIStore } from '@/stores/ui.store';
 import { useWorkflowsStore } from '@/stores/workflows.store';
 import { useNodeTypesStore } from '@/stores/nodeTypes.store';
+import { useStorage } from '@/composables/useStorage';
 
 export default defineComponent({
 	name: 'ActivationModal',
@@ -88,7 +89,7 @@ export default defineComponent({
 		},
 		handleCheckboxChange(checkboxValue: boolean) {
 			this.checked = checkboxValue;
-			window.localStorage.setItem(LOCAL_STORAGE_ACTIVATION_FLAG, checkboxValue.toString());
+			useStorage(LOCAL_STORAGE_ACTIVATION_FLAG).value = checkboxValue.toString();
 		},
 	},
 	computed: {
diff --git a/packages/editor-ui/src/components/NDVDraggablePanels.vue b/packages/editor-ui/src/components/NDVDraggablePanels.vue
index 8c96ebf154..5851aa85b1 100644
--- a/packages/editor-ui/src/components/NDVDraggablePanels.vue
+++ b/packages/editor-ui/src/components/NDVDraggablePanels.vue
@@ -41,7 +41,7 @@ import { defineComponent } from 'vue';
 import type { PropType } from 'vue';
 import { mapStores } from 'pinia';
 import { get } from 'lodash-es';
-import { useStorage } from '@vueuse/core';
+import { useStorage } from '@/composables/useStorage';
 
 import type { INodeTypeDescription } from 'n8n-workflow';
 import PanelDragButton from './PanelDragButton.vue';
@@ -348,7 +348,6 @@ export default defineComponent({
 		restorePositionData() {
 			const storedPanelWidthData = useStorage(
 				`${LOCAL_STORAGE_MAIN_PANEL_RELATIVE_WIDTH}_${this.currentNodePaneType}`,
-				undefined,
 			).value;
 
 			if (storedPanelWidthData) {
@@ -362,10 +361,8 @@ export default defineComponent({
 			return false;
 		},
 		storePositionData() {
-			window.localStorage.setItem(
-				`${LOCAL_STORAGE_MAIN_PANEL_RELATIVE_WIDTH}_${this.currentNodePaneType}`,
-				this.mainPanelDimensions.relativeWidth.toString(),
-			);
+			useStorage(`${LOCAL_STORAGE_MAIN_PANEL_RELATIVE_WIDTH}_${this.currentNodePaneType}`).value =
+				this.mainPanelDimensions.relativeWidth.toString();
 		},
 		onDragStart() {
 			this.isDragging = true;
diff --git a/packages/editor-ui/src/components/Node.vue b/packages/editor-ui/src/components/Node.vue
index da67476079..615aab0c5f 100644
--- a/packages/editor-ui/src/components/Node.vue
+++ b/packages/editor-ui/src/components/Node.vue
@@ -170,7 +170,7 @@
 <script lang="ts">
 import { defineComponent } from 'vue';
 import { mapStores } from 'pinia';
-import { useStorage } from '@vueuse/core';
+import { useStorage } from '@/composables/useStorage';
 import {
 	CUSTOM_API_CALL_KEY,
 	LOCAL_STORAGE_PIN_DATA_DISCOVERY_CANVAS_FLAG,
@@ -582,10 +582,7 @@ export default defineComponent({
 		},
 	},
 	created() {
-		const hasSeenPinDataTooltip = useStorage(
-			LOCAL_STORAGE_PIN_DATA_DISCOVERY_CANVAS_FLAG,
-			undefined,
-		).value;
+		const hasSeenPinDataTooltip = useStorage(LOCAL_STORAGE_PIN_DATA_DISCOVERY_CANVAS_FLAG).value;
 		if (!hasSeenPinDataTooltip) {
 			this.unwatchWorkflowDataItems = this.$watch('workflowDataItems', (dataItemsCount: number) => {
 				this.showPinDataDiscoveryTooltip(dataItemsCount);
@@ -626,7 +623,7 @@ export default defineComponent({
 			)
 				return;
 
-			localStorage.setItem(LOCAL_STORAGE_PIN_DATA_DISCOVERY_CANVAS_FLAG, 'true');
+			useStorage(LOCAL_STORAGE_PIN_DATA_DISCOVERY_CANVAS_FLAG).value = 'true';
 
 			this.pinDataDiscoveryTooltipVisible = true;
 			this.unwatchWorkflowDataItems();
diff --git a/packages/editor-ui/src/components/RunData.vue b/packages/editor-ui/src/components/RunData.vue
index e659474b20..44a4b4d1fe 100644
--- a/packages/editor-ui/src/components/RunData.vue
+++ b/packages/editor-ui/src/components/RunData.vue
@@ -496,7 +496,7 @@
 import { defineAsyncComponent, defineComponent } from 'vue';
 import type { PropType } from 'vue';
 import { mapStores } from 'pinia';
-import { useStorage } from '@vueuse/core';
+import { useStorage } from '@/composables/useStorage';
 import { saveAs } from 'file-saver';
 import type {
 	ConnectionTypes,
@@ -940,10 +940,7 @@ export default defineComponent({
 				return;
 			}
 
-			const pinDataDiscoveryFlag = useStorage(
-				LOCAL_STORAGE_PIN_DATA_DISCOVERY_NDV_FLAG,
-				undefined,
-			).value;
+			const pinDataDiscoveryFlag = useStorage(LOCAL_STORAGE_PIN_DATA_DISCOVERY_NDV_FLAG).value;
 
 			if (value && value.length > 0 && !this.isReadOnlyRoute && !pinDataDiscoveryFlag) {
 				this.pinDataDiscoveryComplete();
@@ -963,8 +960,8 @@ export default defineComponent({
 			}
 		},
 		pinDataDiscoveryComplete() {
-			localStorage.setItem(LOCAL_STORAGE_PIN_DATA_DISCOVERY_NDV_FLAG, 'true');
-			localStorage.setItem(LOCAL_STORAGE_PIN_DATA_DISCOVERY_CANVAS_FLAG, 'true');
+			useStorage(LOCAL_STORAGE_PIN_DATA_DISCOVERY_NDV_FLAG).value = 'true';
+			useStorage(LOCAL_STORAGE_PIN_DATA_DISCOVERY_CANVAS_FLAG).value = 'true';
 		},
 		enterEditMode({ origin }: EnterEditModeArgs) {
 			const inputData = this.pinData
diff --git a/packages/editor-ui/src/composables/useStorage.test.ts b/packages/editor-ui/src/composables/useStorage.test.ts
new file mode 100644
index 0000000000..cbe58864d1
--- /dev/null
+++ b/packages/editor-ui/src/composables/useStorage.test.ts
@@ -0,0 +1,48 @@
+import { nextTick } from 'vue';
+import { useStorage } from './useStorage';
+
+describe('useStorage', () => {
+	beforeEach(() => {
+		localStorage.clear();
+	});
+
+	it('should initialize with null if no value is stored in localStorage', () => {
+		const key = 'test-key';
+		const data = useStorage(key);
+
+		expect(data.value).toBeNull();
+	});
+
+	it('should initialize with the stored value if it exists in localStorage', () => {
+		const key = 'test-key';
+		const value = 'test-value';
+		localStorage.setItem(key, value);
+
+		const data = useStorage(key);
+		expect(data.value).toBe(value);
+	});
+
+	it('should update localStorage when the data ref is updated', async () => {
+		const key = 'test-key';
+		const value = 'test-value';
+		const data = useStorage(key);
+
+		data.value = value;
+		await nextTick();
+
+		expect(localStorage.getItem(key)).toBe(value);
+	});
+
+	it('should remove the key from localStorage when the data ref is set to null', async () => {
+		const key = 'test-key';
+		const value = 'test-value';
+		localStorage.setItem(key, value);
+
+		const data = useStorage(key);
+
+		data.value = null;
+		await nextTick();
+
+		expect(localStorage.getItem(key)).toBeNull();
+	});
+});
diff --git a/packages/editor-ui/src/composables/useStorage.ts b/packages/editor-ui/src/composables/useStorage.ts
new file mode 100644
index 0000000000..2feeafe1f4
--- /dev/null
+++ b/packages/editor-ui/src/composables/useStorage.ts
@@ -0,0 +1,13 @@
+import { useStorage as useStorageComposable } from '@vueuse/core';
+import type { Ref } from 'vue';
+
+export function useStorage(key: string): Ref<string | null> {
+	const data = useStorageComposable(key, null, undefined, { writeDefaults: false });
+
+	// bug in 1.15.1
+	if (data.value === 'undefined') {
+		data.value = null;
+	}
+
+	return data;
+}
diff --git a/packages/editor-ui/src/mixins/workflowActivate.ts b/packages/editor-ui/src/mixins/workflowActivate.ts
index 5290387172..18183cd1e5 100644
--- a/packages/editor-ui/src/mixins/workflowActivate.ts
+++ b/packages/editor-ui/src/mixins/workflowActivate.ts
@@ -1,6 +1,6 @@
 import { defineComponent } from 'vue';
 import { mapStores } from 'pinia';
-import { useStorage } from '@vueuse/core';
+import { useStorage } from '@/composables/useStorage';
 
 import { externalHooks } from '@/mixins/externalHooks';
 import { workflowHelpers } from '@/mixins/workflowHelpers';
@@ -120,10 +120,7 @@ export const workflowActivate = defineComponent({
 			this.updatingWorkflowActivation = false;
 
 			if (isCurrentWorkflow) {
-				if (
-					newActiveState &&
-					useStorage(LOCAL_STORAGE_ACTIVATION_FLAG, undefined).value !== 'true'
-				) {
+				if (newActiveState && useStorage(LOCAL_STORAGE_ACTIVATION_FLAG).value !== 'true') {
 					this.uiStore.openModal(WORKFLOW_ACTIVE_MODAL_KEY);
 				} else {
 					await this.settingsStore.fetchPromptsData();
diff --git a/packages/editor-ui/src/router.ts b/packages/editor-ui/src/router.ts
index 8c04564271..d96cb8dfc2 100644
--- a/packages/editor-ui/src/router.ts
+++ b/packages/editor-ui/src/router.ts
@@ -1,4 +1,5 @@
-import { useStorage } from '@vueuse/core';
+import { useStorage } from '@/composables/useStorage';
+
 import type { RouteLocation, RouteRecordRaw } from 'vue-router';
 import { createRouter, createWebHistory } from 'vue-router';
 import type { IPermissions } from './Interface';
@@ -802,7 +803,7 @@ export const routes = [
 							role: [ROLE.Owner],
 						},
 						deny: {
-							shouldDeny: () => !useStorage('audit-logs', undefined).value,
+							shouldDeny: () => !useStorage('audit-logs').value,
 						},
 					},
 				},
diff --git a/packages/editor-ui/src/stores/__tests__/posthog.test.ts b/packages/editor-ui/src/stores/__tests__/posthog.test.ts
index 398ba4ef46..d485dc5af6 100644
--- a/packages/editor-ui/src/stores/__tests__/posthog.test.ts
+++ b/packages/editor-ui/src/stores/__tests__/posthog.test.ts
@@ -5,6 +5,8 @@ import { useSettingsStore } from '@/stores/settings.store';
 import { useRootStore } from '@/stores/n8nRoot.store';
 import { useTelemetryStore } from '@/stores/telemetry.store';
 import type { IN8nUISettings } from 'n8n-workflow';
+import { LOCAL_STORAGE_EXPERIMENT_OVERRIDES } from '@/constants';
+import { nextTick } from 'vue';
 
 const DEFAULT_POSTHOG_SETTINGS: IN8nUISettings['posthog'] = {
 	enabled: true,
@@ -55,7 +57,6 @@ function setup() {
 
 	vi.spyOn(window.posthog, 'init');
 	vi.spyOn(window.posthog, 'identify');
-	vi.spyOn(window.Storage.prototype, 'setItem');
 	vi.spyOn(telemetryStore, 'track');
 }
 
@@ -117,7 +118,7 @@ describe('Posthog store', () => {
 			});
 		});
 
-		it('sets override feature flags', () => {
+		it('sets override feature flags', async () => {
 			const TEST = 'test';
 			const flags = {
 				[TEST]: 'variant',
@@ -126,17 +127,17 @@ describe('Posthog store', () => {
 			posthog.init(flags);
 
 			window.featureFlags?.override(TEST, 'override');
+			await nextTick();
 
 			expect(posthog.getVariant('test')).toEqual('override');
 			expect(window.posthog?.init).toHaveBeenCalled();
-			expect(window.localStorage.setItem).toHaveBeenCalledWith(
-				'N8N_EXPERIMENT_OVERRIDES',
+			expect(window.localStorage.getItem(LOCAL_STORAGE_EXPERIMENT_OVERRIDES)).toEqual(
 				JSON.stringify({ test: 'override' }),
 			);
 
 			window.featureFlags?.override('other_test', 'override');
-			expect(window.localStorage.setItem).toHaveBeenCalledWith(
-				'N8N_EXPERIMENT_OVERRIDES',
+			await nextTick();
+			expect(window.localStorage.getItem(LOCAL_STORAGE_EXPERIMENT_OVERRIDES)).toEqual(
 				JSON.stringify({ test: 'override', other_test: 'override' }),
 			);
 		});
diff --git a/packages/editor-ui/src/stores/ndv.store.ts b/packages/editor-ui/src/stores/ndv.store.ts
index 668e3b3a32..058b3fc405 100644
--- a/packages/editor-ui/src/stores/ndv.store.ts
+++ b/packages/editor-ui/src/stores/ndv.store.ts
@@ -1,4 +1,4 @@
-import { useStorage } from '@vueuse/core';
+import { useStorage } from '@/composables/useStorage';
 import { LOCAL_STORAGE_MAPPING_IS_ONBOARDED, STORES } from '@/constants';
 import type {
 	INodeUi,
@@ -49,7 +49,7 @@ export const useNDVStore = defineStore(STORES.NDV, {
 			canDrop: false,
 			stickyPosition: null,
 		},
-		isMappingOnboarded: useStorage(LOCAL_STORAGE_MAPPING_IS_ONBOARDED, undefined).value === 'true',
+		isMappingOnboarded: useStorage(LOCAL_STORAGE_MAPPING_IS_ONBOARDED).value === 'true',
 	}),
 	getters: {
 		activeNode(): INodeUi | null {
@@ -228,7 +228,7 @@ export const useNDVStore = defineStore(STORES.NDV, {
 		disableMappingHint(store = true) {
 			this.isMappingOnboarded = true;
 			if (store) {
-				window.localStorage.setItem(LOCAL_STORAGE_MAPPING_IS_ONBOARDED, 'true');
+				useStorage(LOCAL_STORAGE_MAPPING_IS_ONBOARDED).value = 'true';
 			}
 		},
 		updateNodeParameterIssues(issues: INodeIssues): void {
diff --git a/packages/editor-ui/src/stores/posthog.store.ts b/packages/editor-ui/src/stores/posthog.store.ts
index 5a3953275e..c6ae7b48a3 100644
--- a/packages/editor-ui/src/stores/posthog.store.ts
+++ b/packages/editor-ui/src/stores/posthog.store.ts
@@ -1,7 +1,7 @@
 import type { Ref } from 'vue';
 import { ref } from 'vue';
 import { defineStore } from 'pinia';
-import { useStorage } from '@vueuse/core';
+import { useStorage } from '@/composables/useStorage';
 import { useUsersStore } from '@/stores/users.store';
 import { useRootStore } from '@/stores/n8nRoot.store';
 import { useSettingsStore } from '@/stores/settings.store';
@@ -41,7 +41,7 @@ export const usePostHog = defineStore('posthog', () => {
 
 	if (!window.featureFlags) {
 		// for testing
-		const cachedOverrides = useStorage(LOCAL_STORAGE_EXPERIMENT_OVERRIDES, undefined).value;
+		const cachedOverrides = useStorage(LOCAL_STORAGE_EXPERIMENT_OVERRIDES).value;
 		if (cachedOverrides) {
 			try {
 				console.log('Overriding feature flags', cachedOverrides);
@@ -60,7 +60,7 @@ export const usePostHog = defineStore('posthog', () => {
 					[name]: value,
 				};
 				try {
-					localStorage.setItem(LOCAL_STORAGE_EXPERIMENT_OVERRIDES, JSON.stringify(overrides.value));
+					useStorage(LOCAL_STORAGE_EXPERIMENT_OVERRIDES).value = JSON.stringify(overrides.value);
 				} catch (e) {}
 			},
 
diff --git a/packages/editor-ui/src/stores/ui.utils.ts b/packages/editor-ui/src/stores/ui.utils.ts
index 4accdfba67..9245bc549a 100644
--- a/packages/editor-ui/src/stores/ui.utils.ts
+++ b/packages/editor-ui/src/stores/ui.utils.ts
@@ -1,6 +1,9 @@
 import type { AppliedThemeOption, ThemeOption } from '@/Interface';
+import { useStorage } from '@/composables/useStorage';
 import { LOCAL_STORAGE_THEME } from '@/constants';
 
+const themeRef = useStorage(LOCAL_STORAGE_THEME);
+
 export function addThemeToBody(theme: AppliedThemeOption) {
 	window.document.body.setAttribute('data-theme', theme);
 }
@@ -11,7 +14,7 @@ export function isValidTheme(theme: string | null): theme is AppliedThemeOption
 
 // query param allows overriding theme for demo view in preview iframe without flickering
 export function getThemeOverride() {
-	return getQueryParam('theme') || localStorage.getItem(LOCAL_STORAGE_THEME);
+	return getQueryParam('theme') || themeRef.value;
 }
 
 function getQueryParam(paramName: string): string | null {
@@ -21,10 +24,10 @@ function getQueryParam(paramName: string): string | null {
 export function updateTheme(theme: ThemeOption) {
 	if (theme === 'system') {
 		window.document.body.removeAttribute('data-theme');
-		localStorage.removeItem(LOCAL_STORAGE_THEME);
+		themeRef.value = null;
 	} else {
 		addThemeToBody(theme);
-		localStorage.setItem(LOCAL_STORAGE_THEME, theme);
+		themeRef.value = theme;
 	}
 }