chore: Refactor localstorage into typesafe version (#17832)

superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/controlPanel.tsx

@@ -237,14 +237,14 @@ const config: ControlPanelConfig = {
               ],
               renderTrigger: true,
               description: (
-                <React.Fragment>
+                <>
                   <div>{t('Change order of rows.')}</div>
                   <div>{t('Available sorting modes:')}</div>
                   <ul>
                     <li>{t('By key: use row names as sorting key')}</li>
                     <li>{t('By value: use metric values as sorting key')}</li>
                   </ul>
-                </React.Fragment>
+                </>
               ),
             },
           },
@@ -265,14 +265,14 @@ const config: ControlPanelConfig = {
               ],
               renderTrigger: true,
               description: (
-                <React.Fragment>
+                <>
                   <div>{t('Change order of columns.')}</div>
                   <div>{t('Available sorting modes:')}</div>
                   <ul>
                     <li>{t('By key: use column names as sorting key')}</li>
                     <li>{t('By value: use metric values as sorting key')}</li>
                   </ul>
-                </React.Fragment>
+                </>
               ),
             },
           },

superset-frontend/src/dashboard/containers/DashboardPage.tsx

@@ -36,13 +36,13 @@ import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
 import { addWarningToast } from 'src/components/MessageToasts/actions';
 
 import {
-  getFromLocalStorage,
-  setInLocalStorage,
+  getItem,
+  setItem,
+  LocalStorageKeys,
 } from 'src/utils/localStorageHelpers';
 import {
   FILTER_BOX_MIGRATION_STATES,
   FILTER_BOX_TRANSITION_SNOOZE_DURATION,
-  FILTER_BOX_TRANSITION_SNOOZED_AT,
 } from 'src/explore/constants';
 import { URL_PARAMS } from 'src/constants';
 import { getUrlParam } from 'src/utils/urlUtils';
@@ -126,8 +126,10 @@ const DashboardPage: FC = () => {
           }
 
           // has cookie?
-          const snoozeDash =
-            getFromLocalStorage(FILTER_BOX_TRANSITION_SNOOZED_AT, 0) || {};
+          const snoozeDash = getItem(
+            LocalStorageKeys.filter_box_transition_snoozed_at,
+            {},
+          );
           if (
             Date.now() - (snoozeDash[id] || 0) <
             FILTER_BOX_TRANSITION_SNOOZE_DURATION
@@ -210,9 +212,11 @@ const DashboardPage: FC = () => {
           setFilterboxMigrationState(FILTER_BOX_MIGRATION_STATES.REVIEWING);
         }}
         onClickSnooze={() => {
-          const snoozedDash =
-            getFromLocalStorage(FILTER_BOX_TRANSITION_SNOOZED_AT, 0) || {};
-          setInLocalStorage(FILTER_BOX_TRANSITION_SNOOZED_AT, {
+          const snoozedDash = getItem(
+            LocalStorageKeys.filter_box_transition_snoozed_at,
+            {},
+          );
+          setItem(LocalStorageKeys.filter_box_transition_snoozed_at, {
             ...snoozedDash,
             [id]: Date.now(),
           });

superset-frontend/src/explore/components/DataTablesPane/index.tsx

@@ -25,8 +25,9 @@ import TableView, { EmptyWrapperType } from 'src/components/TableView';
 import { getChartDataRequest } from 'src/chart/chartAction';
 import { getClientErrorObject } from 'src/utils/getClientErrorObject';
 import {
-  getFromLocalStorage,
-  setInLocalStorage,
+  getItem,
+  setItem,
+  LocalStorageKeys,
 } from 'src/utils/localStorageHelpers';
 import {
   CopyToClipboardButton,
@@ -49,10 +50,6 @@ const NULLISH_RESULTS_STATE = {
 
 const DATA_TABLE_PAGE_SIZE = 50;
 
-const STORAGE_KEYS = {
-  isOpen: 'is_datapanel_open',
-};
-
 const DATAPANEL_KEY = 'data';
 
 const TableControlsWrapper = styled.div`
@@ -200,7 +197,7 @@ export const DataTablesPane = ({
     [RESULT_TYPES.samples]?: boolean;
   }>(NULLISH_RESULTS_STATE);
   const [panelOpen, setPanelOpen] = useState(
-    getFromLocalStorage(STORAGE_KEYS.isOpen, false),
+    getItem(LocalStorageKeys.is_datapanel_open, false),
   );
 
   const formattedData = useMemo(
@@ -283,7 +280,7 @@ export const DataTablesPane = ({
   );
 
   useEffect(() => {
-    setInLocalStorage(STORAGE_KEYS.isOpen, panelOpen);
+    setItem(LocalStorageKeys.is_datapanel_open, panelOpen);
   }, [panelOpen]);
 
   useEffect(() => {

superset-frontend/src/explore/components/ExploreChartPanel.jsx

@@ -24,8 +24,9 @@ import { useResizeDetector } from 'react-resize-detector';
 import { chartPropShape } from 'src/dashboard/util/propShapes';
 import ChartContainer from 'src/chart/ChartContainer';
 import {
-  getFromLocalStorage,
-  setInLocalStorage,
+  getItem,
+  setItem,
+  LocalStorageKeys,
 } from 'src/utils/localStorageHelpers';
 import ConnectedExploreChartHeader from './ExploreChartHeader';
 import { DataTablesPane } from './DataTablesPane';
@@ -64,10 +65,6 @@ const CHART_PANEL_PADDING_HORIZ = 30;
 const CHART_PANEL_PADDING_VERTICAL = 15;
 const HEADER_PADDING = 15;
 
-const STORAGE_KEYS = {
-  sizes: 'chart_split_sizes',
-};
-
 const INITIAL_SIZES = [90, 10];
 const MIN_SIZES = [300, 50];
 const DEFAULT_SOUTH_PANE_HEIGHT_PERCENT = 40;
@@ -126,7 +123,7 @@ const ExploreChartPanel = props => {
     refreshRate: 300,
   });
   const [splitSizes, setSplitSizes] = useState(
-    getFromLocalStorage(STORAGE_KEYS.sizes, INITIAL_SIZES),
+    getItem(LocalStorageKeys.chart_split_sizes, INITIAL_SIZES),
   );
   const { slice } = props;
   const updateQueryContext = useCallback(
@@ -192,7 +189,7 @@ const ExploreChartPanel = props => {
   }, [recalcPanelSizes, splitSizes]);
 
   useEffect(() => {
-    setInLocalStorage(STORAGE_KEYS.sizes, splitSizes);
+    setItem(LocalStorageKeys.chart_split_sizes, splitSizes);
   }, [splitSizes]);
 
   const onDragEnd = sizes => {

superset-frontend/src/explore/components/ExploreViewContainer.jsx

@@ -32,8 +32,9 @@ import { usePrevious } from 'src/common/hooks/usePrevious';
 import { useComponentDidMount } from 'src/common/hooks/useComponentDidMount';
 import Icons from 'src/components/Icons';
 import {
-  getFromLocalStorage,
-  setInLocalStorage,
+  getItem,
+  setItem,
+  LocalStorageKeys,
 } from 'src/utils/localStorageHelpers';
 import { URL_PARAMS } from 'src/constants';
 import cx from 'classnames';
@@ -183,11 +184,6 @@ function ExploreViewContainer(props) {
     ? `${props.forcedHeight}px`
     : `${windowSize.height - navHeight}px`;
 
-  const storageKeys = {
-    controlsWidth: 'controls_width',
-    dataSourceWidth: 'datasource_width',
-  };
-
   const defaultSidebarsWidth = {
     controls_width: 320,
     datasource_width: 300,
@@ -455,12 +451,12 @@ function ExploreViewContainer(props) {
   }
 
   function getSidebarWidths(key) {
-    return getFromLocalStorage(key, defaultSidebarsWidth[key]);
+    return getItem(key, defaultSidebarsWidth[key]);
   }
 
   function setSidebarWidths(key, dimension) {
     const newDimension = Number(getSidebarWidths(key)) + dimension.width;
-    setInLocalStorage(key, newDimension);
+    setItem(key, newDimension);
   }
 
   if (props.standalone) {
@@ -504,13 +500,13 @@ function ExploreViewContainer(props) {
       )}
       <Resizable
         onResizeStop={(evt, direction, ref, d) =>
-          setSidebarWidths(storageKeys.dataSourceWidth, d)
+          setSidebarWidths(LocalStorageKeys.datasource_width, d)
         }
         defaultSize={{
-          width: getSidebarWidths(storageKeys.dataSourceWidth),
+          width: getSidebarWidths(LocalStorageKeys.datasource_width),
           height: '100%',
         }}
-        minWidth={defaultSidebarsWidth[storageKeys.dataSourceWidth]}
+        minWidth={defaultSidebarsWidth[LocalStorageKeys.datasource_width]}
         maxWidth="33%"
         enable={{ right: true }}
         className={
@@ -564,13 +560,13 @@ function ExploreViewContainer(props) {
       ) : null}
       <Resizable
         onResizeStop={(evt, direction, ref, d) =>
-          setSidebarWidths(storageKeys.controlsWidth, d)
+          setSidebarWidths(LocalStorageKeys.controls_width, d)
         }
         defaultSize={{
-          width: getSidebarWidths(storageKeys.controlsWidth),
+          width: getSidebarWidths(LocalStorageKeys.controls_width),
           height: '100%',
         }}
-        minWidth={defaultSidebarsWidth[storageKeys.controlsWidth]}
+        minWidth={defaultSidebarsWidth[LocalStorageKeys.controls_width]}
         maxWidth="33%"
         enable={{ right: true }}
         className="col-sm-3 explore-column controls-column"

superset-frontend/src/explore/constants.ts

@@ -151,8 +151,6 @@ export enum FILTER_BOX_MIGRATION_STATES {
   UNDECIDED = 'UNDECIDED',
 }
 
-export const FILTER_BOX_TRANSITION_SNOOZED_AT =
-  'filter_box_transition_snoozed_at';
 export const FILTER_BOX_TRANSITION_SNOOZE_DURATION = 24 * 60 * 60 * 1000; // 24 hours
 
 export const POPOVER_INITIAL_HEIGHT = 240;

superset-frontend/src/views/CRUD/storageKeys.ts → superset-frontend/src/utils/localStorageHelpers.test.ts

@@ -16,9 +16,28 @@
  * specific language governing permissions and limitations
  * under the License.
  */
+import {
+  getItem,
+  setItem,
+  LocalStorageKeys,
+} from 'src/utils/localStorageHelpers';
 
-// storage keys for welcome page sticky tabs and tables
-export const HOMEPAGE_CHART_FILTER = 'homepage_chart_filter';
-export const HOMEPAGE_ACTIVITY_FILTER = 'homepage_activity_filter';
-export const HOMEPAGE_DASHBOARD_FILTER = 'homepage_dashboard_filter';
-export const HOMEPAGE_COLLAPSE_STATE = 'homepage_collapse_state';
+describe('localStorageHelpers', () => {
+  beforeEach(() => {
+    localStorage.clear();
+  });
+
+  afterAll(() => {
+    localStorage.clear();
+  });
+
+  it('gets a value that was set', () => {
+    setItem(LocalStorageKeys.is_datapanel_open, false);
+
+    expect(getItem(LocalStorageKeys.is_datapanel_open, true)).toBe(false);
+  });
+
+  it('returns the default value for an unset value', () => {
+    expect(getItem(LocalStorageKeys.is_datapanel_open, true)).toBe(true);
+  });
+});

superset-frontend/src/utils/localStorageHelpers.ts

@@ -17,19 +17,98 @@
  * under the License.
  */
 
-export const getFromLocalStorage = (key: string, defaultValue: any) => {
+import { TableTabTypes } from 'src/views/CRUD/types';
+import { SetTabType } from 'src/views/CRUD/welcome/ActivityTable';
+
+export enum LocalStorageKeys {
+  /**
+   * START LEGACY LOCAL STORAGE KEYS
+   *
+   * Do not follow the patterns here for key names. Keys should instead be namespaced to avoid
+   * collisions.
+   *
+   * TODO: Update all local storage keys to follow the new pattern. This is a breaking change,
+   * and therefore should be done in a major release.
+   */
+  filter_box_transition_snoozed_at = 'filter_box_transition_snoozed_at',
+  chart_split_sizes = 'chart_split_sizes',
+  controls_width = 'controls_width',
+  datasource_width = 'datasource_width',
+  is_datapanel_open = 'is_datapanel_open',
+  homepage_chart_filter = 'homepage_chart_filter',
+  homepage_dashboard_filter = 'homepage_dashboard_filter',
+  homepage_collapse_state = 'homepage_collapse_state',
+  homepage_activity_filter = 'homepage_activity_filter',
+  /** END LEGACY LOCAL STORAGE KEYS */
+
+  /**
+   * New local storage keys should be namespaced to avoid collisions. Keys should be named in the
+   * form [namespace]__[key].
+   *
+   * Example:
+   * sqllab__is_autocomplete_enabled
+   */
+}
+
+export type LocalStorageValues = {
+  filter_box_transition_snoozed_at: Record<number, number>;
+  chart_split_sizes: [number, number];
+  controls_width: number;
+  datasource_width: number;
+  is_datapanel_open: boolean;
+  homepage_chart_filter: TableTabTypes;
+  homepage_dashboard_filter: TableTabTypes;
+  homepage_collapse_state: string[];
+  homepage_activity_filter: SetTabType | null;
+};
+
+export function getItem<K extends LocalStorageKeys>(
+  key: K,
+  defaultValue: LocalStorageValues[K],
+): LocalStorageValues[K] {
+  return dangerouslyGetItemDoNotUse(key, defaultValue);
+}
+
+export function setItem<K extends LocalStorageKeys>(
+  key: K,
+  value: LocalStorageValues[K],
+): void {
+  dangerouslySetItemDoNotUse(key, value);
+}
+
+/*
+ * This function should not be used directly, as it doesn't provide any type safety or any
+ * guarantees that the globally namespaced localstorage key is correct.
+ *
+ * Instead, use getItem and setItem. Any legacy uses should be updated/migrated in future
+ * Superset versions (as they may require breaking changes).
+ * */
+export function dangerouslyGetItemDoNotUse(
+  key: string,
+  defaultValue: any,
+): any {
   try {
     const value = localStorage.getItem(key);
-    return JSON.parse(value || 'null') || defaultValue;
+    if (value === null) {
+      return defaultValue;
+    }
+    return JSON.parse(value);
   } catch {
     return defaultValue;
   }
-};
+}
 
-export const setInLocalStorage = (key: string, value: any) => {
+/*
+ * This function should not be used directly, as it doesn't provide any type safety or any
+ * guarantees that the globally namespaced localstorage key is correct.
+ *
+ * Instead, use getItem and setItem. Any legacy uses should be updated/migrated in future
+ * Superset versions (as they may require breaking changes).
+ * */
+export function dangerouslySetItemDoNotUse(key: string, value: any): void {
   try {
     localStorage.setItem(key, JSON.stringify(value));
   } catch {
     // Catch in case localStorage is unavailable
   }
-};
+}