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):
This commit is contained in:
Mutasem Aldmour 2023-11-07 10:06:08 +01:00 committed by GitHub
parent 78b84af8d1
commit 151e60f829
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 97 additions and 42 deletions

View file

@ -50,6 +50,7 @@ import { getActivatableTriggerNodes, getTriggerNodeServiceName } from '@/utils';
import { useUIStore } from '@/stores/ui.store'; import { useUIStore } from '@/stores/ui.store';
import { useWorkflowsStore } from '@/stores/workflows.store'; import { useWorkflowsStore } from '@/stores/workflows.store';
import { useNodeTypesStore } from '@/stores/nodeTypes.store'; import { useNodeTypesStore } from '@/stores/nodeTypes.store';
import { useStorage } from '@/composables/useStorage';
export default defineComponent({ export default defineComponent({
name: 'ActivationModal', name: 'ActivationModal',
@ -88,7 +89,7 @@ export default defineComponent({
}, },
handleCheckboxChange(checkboxValue: boolean) { handleCheckboxChange(checkboxValue: boolean) {
this.checked = checkboxValue; this.checked = checkboxValue;
window.localStorage.setItem(LOCAL_STORAGE_ACTIVATION_FLAG, checkboxValue.toString()); useStorage(LOCAL_STORAGE_ACTIVATION_FLAG).value = checkboxValue.toString();
}, },
}, },
computed: { computed: {

View file

@ -41,7 +41,7 @@ import { defineComponent } from 'vue';
import type { PropType } from 'vue'; import type { PropType } from 'vue';
import { mapStores } from 'pinia'; import { mapStores } from 'pinia';
import { get } from 'lodash-es'; import { get } from 'lodash-es';
import { useStorage } from '@vueuse/core'; import { useStorage } from '@/composables/useStorage';
import type { INodeTypeDescription } from 'n8n-workflow'; import type { INodeTypeDescription } from 'n8n-workflow';
import PanelDragButton from './PanelDragButton.vue'; import PanelDragButton from './PanelDragButton.vue';
@ -348,7 +348,6 @@ export default defineComponent({
restorePositionData() { restorePositionData() {
const storedPanelWidthData = useStorage( const storedPanelWidthData = useStorage(
`${LOCAL_STORAGE_MAIN_PANEL_RELATIVE_WIDTH}_${this.currentNodePaneType}`, `${LOCAL_STORAGE_MAIN_PANEL_RELATIVE_WIDTH}_${this.currentNodePaneType}`,
undefined,
).value; ).value;
if (storedPanelWidthData) { if (storedPanelWidthData) {
@ -362,10 +361,8 @@ export default defineComponent({
return false; return false;
}, },
storePositionData() { storePositionData() {
window.localStorage.setItem( useStorage(`${LOCAL_STORAGE_MAIN_PANEL_RELATIVE_WIDTH}_${this.currentNodePaneType}`).value =
`${LOCAL_STORAGE_MAIN_PANEL_RELATIVE_WIDTH}_${this.currentNodePaneType}`, this.mainPanelDimensions.relativeWidth.toString();
this.mainPanelDimensions.relativeWidth.toString(),
);
}, },
onDragStart() { onDragStart() {
this.isDragging = true; this.isDragging = true;

View file

@ -170,7 +170,7 @@
<script lang="ts"> <script lang="ts">
import { defineComponent } from 'vue'; import { defineComponent } from 'vue';
import { mapStores } from 'pinia'; import { mapStores } from 'pinia';
import { useStorage } from '@vueuse/core'; import { useStorage } from '@/composables/useStorage';
import { import {
CUSTOM_API_CALL_KEY, CUSTOM_API_CALL_KEY,
LOCAL_STORAGE_PIN_DATA_DISCOVERY_CANVAS_FLAG, LOCAL_STORAGE_PIN_DATA_DISCOVERY_CANVAS_FLAG,
@ -582,10 +582,7 @@ export default defineComponent({
}, },
}, },
created() { created() {
const hasSeenPinDataTooltip = useStorage( const hasSeenPinDataTooltip = useStorage(LOCAL_STORAGE_PIN_DATA_DISCOVERY_CANVAS_FLAG).value;
LOCAL_STORAGE_PIN_DATA_DISCOVERY_CANVAS_FLAG,
undefined,
).value;
if (!hasSeenPinDataTooltip) { if (!hasSeenPinDataTooltip) {
this.unwatchWorkflowDataItems = this.$watch('workflowDataItems', (dataItemsCount: number) => { this.unwatchWorkflowDataItems = this.$watch('workflowDataItems', (dataItemsCount: number) => {
this.showPinDataDiscoveryTooltip(dataItemsCount); this.showPinDataDiscoveryTooltip(dataItemsCount);
@ -626,7 +623,7 @@ export default defineComponent({
) )
return; 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.pinDataDiscoveryTooltipVisible = true;
this.unwatchWorkflowDataItems(); this.unwatchWorkflowDataItems();

View file

@ -496,7 +496,7 @@
import { defineAsyncComponent, defineComponent } from 'vue'; import { defineAsyncComponent, defineComponent } from 'vue';
import type { PropType } from 'vue'; import type { PropType } from 'vue';
import { mapStores } from 'pinia'; import { mapStores } from 'pinia';
import { useStorage } from '@vueuse/core'; import { useStorage } from '@/composables/useStorage';
import { saveAs } from 'file-saver'; import { saveAs } from 'file-saver';
import type { import type {
ConnectionTypes, ConnectionTypes,
@ -940,10 +940,7 @@ export default defineComponent({
return; return;
} }
const pinDataDiscoveryFlag = useStorage( const pinDataDiscoveryFlag = useStorage(LOCAL_STORAGE_PIN_DATA_DISCOVERY_NDV_FLAG).value;
LOCAL_STORAGE_PIN_DATA_DISCOVERY_NDV_FLAG,
undefined,
).value;
if (value && value.length > 0 && !this.isReadOnlyRoute && !pinDataDiscoveryFlag) { if (value && value.length > 0 && !this.isReadOnlyRoute && !pinDataDiscoveryFlag) {
this.pinDataDiscoveryComplete(); this.pinDataDiscoveryComplete();
@ -963,8 +960,8 @@ export default defineComponent({
} }
}, },
pinDataDiscoveryComplete() { pinDataDiscoveryComplete() {
localStorage.setItem(LOCAL_STORAGE_PIN_DATA_DISCOVERY_NDV_FLAG, 'true'); useStorage(LOCAL_STORAGE_PIN_DATA_DISCOVERY_NDV_FLAG).value = 'true';
localStorage.setItem(LOCAL_STORAGE_PIN_DATA_DISCOVERY_CANVAS_FLAG, 'true'); useStorage(LOCAL_STORAGE_PIN_DATA_DISCOVERY_CANVAS_FLAG).value = 'true';
}, },
enterEditMode({ origin }: EnterEditModeArgs) { enterEditMode({ origin }: EnterEditModeArgs) {
const inputData = this.pinData const inputData = this.pinData

View file

@ -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();
});
});

View file

@ -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;
}

View file

@ -1,6 +1,6 @@
import { defineComponent } from 'vue'; import { defineComponent } from 'vue';
import { mapStores } from 'pinia'; import { mapStores } from 'pinia';
import { useStorage } from '@vueuse/core'; import { useStorage } from '@/composables/useStorage';
import { externalHooks } from '@/mixins/externalHooks'; import { externalHooks } from '@/mixins/externalHooks';
import { workflowHelpers } from '@/mixins/workflowHelpers'; import { workflowHelpers } from '@/mixins/workflowHelpers';
@ -120,10 +120,7 @@ export const workflowActivate = defineComponent({
this.updatingWorkflowActivation = false; this.updatingWorkflowActivation = false;
if (isCurrentWorkflow) { if (isCurrentWorkflow) {
if ( if (newActiveState && useStorage(LOCAL_STORAGE_ACTIVATION_FLAG).value !== 'true') {
newActiveState &&
useStorage(LOCAL_STORAGE_ACTIVATION_FLAG, undefined).value !== 'true'
) {
this.uiStore.openModal(WORKFLOW_ACTIVE_MODAL_KEY); this.uiStore.openModal(WORKFLOW_ACTIVE_MODAL_KEY);
} else { } else {
await this.settingsStore.fetchPromptsData(); await this.settingsStore.fetchPromptsData();

View file

@ -1,4 +1,5 @@
import { useStorage } from '@vueuse/core'; import { useStorage } from '@/composables/useStorage';
import type { RouteLocation, RouteRecordRaw } from 'vue-router'; import type { RouteLocation, RouteRecordRaw } from 'vue-router';
import { createRouter, createWebHistory } from 'vue-router'; import { createRouter, createWebHistory } from 'vue-router';
import type { IPermissions } from './Interface'; import type { IPermissions } from './Interface';
@ -802,7 +803,7 @@ export const routes = [
role: [ROLE.Owner], role: [ROLE.Owner],
}, },
deny: { deny: {
shouldDeny: () => !useStorage('audit-logs', undefined).value, shouldDeny: () => !useStorage('audit-logs').value,
}, },
}, },
}, },

View file

@ -5,6 +5,8 @@ import { useSettingsStore } from '@/stores/settings.store';
import { useRootStore } from '@/stores/n8nRoot.store'; import { useRootStore } from '@/stores/n8nRoot.store';
import { useTelemetryStore } from '@/stores/telemetry.store'; import { useTelemetryStore } from '@/stores/telemetry.store';
import type { IN8nUISettings } from 'n8n-workflow'; import type { IN8nUISettings } from 'n8n-workflow';
import { LOCAL_STORAGE_EXPERIMENT_OVERRIDES } from '@/constants';
import { nextTick } from 'vue';
const DEFAULT_POSTHOG_SETTINGS: IN8nUISettings['posthog'] = { const DEFAULT_POSTHOG_SETTINGS: IN8nUISettings['posthog'] = {
enabled: true, enabled: true,
@ -55,7 +57,6 @@ function setup() {
vi.spyOn(window.posthog, 'init'); vi.spyOn(window.posthog, 'init');
vi.spyOn(window.posthog, 'identify'); vi.spyOn(window.posthog, 'identify');
vi.spyOn(window.Storage.prototype, 'setItem');
vi.spyOn(telemetryStore, 'track'); 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 TEST = 'test';
const flags = { const flags = {
[TEST]: 'variant', [TEST]: 'variant',
@ -126,17 +127,17 @@ describe('Posthog store', () => {
posthog.init(flags); posthog.init(flags);
window.featureFlags?.override(TEST, 'override'); window.featureFlags?.override(TEST, 'override');
await nextTick();
expect(posthog.getVariant('test')).toEqual('override'); expect(posthog.getVariant('test')).toEqual('override');
expect(window.posthog?.init).toHaveBeenCalled(); expect(window.posthog?.init).toHaveBeenCalled();
expect(window.localStorage.setItem).toHaveBeenCalledWith( expect(window.localStorage.getItem(LOCAL_STORAGE_EXPERIMENT_OVERRIDES)).toEqual(
'N8N_EXPERIMENT_OVERRIDES',
JSON.stringify({ test: 'override' }), JSON.stringify({ test: 'override' }),
); );
window.featureFlags?.override('other_test', 'override'); window.featureFlags?.override('other_test', 'override');
expect(window.localStorage.setItem).toHaveBeenCalledWith( await nextTick();
'N8N_EXPERIMENT_OVERRIDES', expect(window.localStorage.getItem(LOCAL_STORAGE_EXPERIMENT_OVERRIDES)).toEqual(
JSON.stringify({ test: 'override', other_test: 'override' }), JSON.stringify({ test: 'override', other_test: 'override' }),
); );
}); });

View file

@ -1,4 +1,4 @@
import { useStorage } from '@vueuse/core'; import { useStorage } from '@/composables/useStorage';
import { LOCAL_STORAGE_MAPPING_IS_ONBOARDED, STORES } from '@/constants'; import { LOCAL_STORAGE_MAPPING_IS_ONBOARDED, STORES } from '@/constants';
import type { import type {
INodeUi, INodeUi,
@ -49,7 +49,7 @@ export const useNDVStore = defineStore(STORES.NDV, {
canDrop: false, canDrop: false,
stickyPosition: null, stickyPosition: null,
}, },
isMappingOnboarded: useStorage(LOCAL_STORAGE_MAPPING_IS_ONBOARDED, undefined).value === 'true', isMappingOnboarded: useStorage(LOCAL_STORAGE_MAPPING_IS_ONBOARDED).value === 'true',
}), }),
getters: { getters: {
activeNode(): INodeUi | null { activeNode(): INodeUi | null {
@ -228,7 +228,7 @@ export const useNDVStore = defineStore(STORES.NDV, {
disableMappingHint(store = true) { disableMappingHint(store = true) {
this.isMappingOnboarded = true; this.isMappingOnboarded = true;
if (store) { if (store) {
window.localStorage.setItem(LOCAL_STORAGE_MAPPING_IS_ONBOARDED, 'true'); useStorage(LOCAL_STORAGE_MAPPING_IS_ONBOARDED).value = 'true';
} }
}, },
updateNodeParameterIssues(issues: INodeIssues): void { updateNodeParameterIssues(issues: INodeIssues): void {

View file

@ -1,7 +1,7 @@
import type { Ref } from 'vue'; import type { Ref } from 'vue';
import { ref } from 'vue'; import { ref } from 'vue';
import { defineStore } from 'pinia'; import { defineStore } from 'pinia';
import { useStorage } from '@vueuse/core'; import { useStorage } from '@/composables/useStorage';
import { useUsersStore } from '@/stores/users.store'; import { useUsersStore } from '@/stores/users.store';
import { useRootStore } from '@/stores/n8nRoot.store'; import { useRootStore } from '@/stores/n8nRoot.store';
import { useSettingsStore } from '@/stores/settings.store'; import { useSettingsStore } from '@/stores/settings.store';
@ -41,7 +41,7 @@ export const usePostHog = defineStore('posthog', () => {
if (!window.featureFlags) { if (!window.featureFlags) {
// for testing // for testing
const cachedOverrides = useStorage(LOCAL_STORAGE_EXPERIMENT_OVERRIDES, undefined).value; const cachedOverrides = useStorage(LOCAL_STORAGE_EXPERIMENT_OVERRIDES).value;
if (cachedOverrides) { if (cachedOverrides) {
try { try {
console.log('Overriding feature flags', cachedOverrides); console.log('Overriding feature flags', cachedOverrides);
@ -60,7 +60,7 @@ export const usePostHog = defineStore('posthog', () => {
[name]: value, [name]: value,
}; };
try { try {
localStorage.setItem(LOCAL_STORAGE_EXPERIMENT_OVERRIDES, JSON.stringify(overrides.value)); useStorage(LOCAL_STORAGE_EXPERIMENT_OVERRIDES).value = JSON.stringify(overrides.value);
} catch (e) {} } catch (e) {}
}, },

View file

@ -1,6 +1,9 @@
import type { AppliedThemeOption, ThemeOption } from '@/Interface'; import type { AppliedThemeOption, ThemeOption } from '@/Interface';
import { useStorage } from '@/composables/useStorage';
import { LOCAL_STORAGE_THEME } from '@/constants'; import { LOCAL_STORAGE_THEME } from '@/constants';
const themeRef = useStorage(LOCAL_STORAGE_THEME);
export function addThemeToBody(theme: AppliedThemeOption) { export function addThemeToBody(theme: AppliedThemeOption) {
window.document.body.setAttribute('data-theme', theme); 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 // query param allows overriding theme for demo view in preview iframe without flickering
export function getThemeOverride() { export function getThemeOverride() {
return getQueryParam('theme') || localStorage.getItem(LOCAL_STORAGE_THEME); return getQueryParam('theme') || themeRef.value;
} }
function getQueryParam(paramName: string): string | null { function getQueryParam(paramName: string): string | null {
@ -21,10 +24,10 @@ function getQueryParam(paramName: string): string | null {
export function updateTheme(theme: ThemeOption) { export function updateTheme(theme: ThemeOption) {
if (theme === 'system') { if (theme === 'system') {
window.document.body.removeAttribute('data-theme'); window.document.body.removeAttribute('data-theme');
localStorage.removeItem(LOCAL_STORAGE_THEME); themeRef.value = null;
} else { } else {
addThemeToBody(theme); addThemeToBody(theme);
localStorage.setItem(LOCAL_STORAGE_THEME, theme); themeRef.value = theme;
} }
} }