Skip to content

Commit

Permalink
Add useQueryAndLocalStoragePersistedState hook for persisting state t…
Browse files Browse the repository at this point in the history
…o both localStorage and queryStrings (dagster-io#18868)

## Summary & Motivation

We want to be able to persist state to both localStorage and
queryStrings while relying on the queryString as the source of truth if
both localStorage and the query string are present

## How I Tested These Changes

I wrote Jest tests + tested in the asset graph that the behavior works
as expected.
  • Loading branch information
salazarm authored Jan 3, 2024
1 parent ff56543 commit 818700b
Show file tree
Hide file tree
Showing 6 changed files with 216 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import {DEFAULT_MAX_ZOOM, SVGViewport} from '../graph/SVGViewport';
import {useAssetLayout} from '../graph/asyncGraphLayout';
import {closestNodeInDirection, isNodeOffscreen} from '../graph/common';
import {AssetGroupSelector} from '../graphql/types';
import {useQueryPersistedState} from '../hooks/useQueryPersistedState';
import {useQueryAndLocalStoragePersistedState} from '../hooks/useQueryAndLocalStoragePersistedState';
import {useStartTrace} from '../performance';
import {
GraphExplorerOptions,
Expand Down Expand Up @@ -222,9 +222,11 @@ const AssetGraphExplorerWithData = ({
return {allGroups: Object.keys(groupedAssets), allGroupCounts: counts, groupedAssets};
}, [assetGraphData]);

const [expandedGroups, setExpandedGroups] = useQueryPersistedState<string[]>({
const [expandedGroups, setExpandedGroups] = useQueryAndLocalStoragePersistedState<string[]>({
localStorageKey: `asset-graph-open-graph-nodes-${isGlobalGraph}-${explorerPath.pipelineName}`,
encode: (arr) => ({expanded: arr.length ? arr.join(',') : undefined}),
decode: (qs) => (qs.expanded || '').split(',').filter(Boolean),
isEmptyState: (val) => val.length === 0,
});
const focusGroupIdAfterLayoutRef = React.useRef('');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import React from 'react';

import {LayoutContext} from '../../app/LayoutProvider';
import {AssetKey} from '../../assets/types';
import {useQueryAndLocalStoragePersistedState} from '../../hooks/useQueryAndLocalStoragePersistedState';
import {ExplorerPath} from '../../pipelines/PipelinePathUtils';
import {Container, Inner, Row} from '../../ui/VirtualizedTable';
import {buildRepoPathForHuman} from '../../workspace/buildRepoAddress';
Expand Down Expand Up @@ -80,7 +81,18 @@ export const AssetGraphExplorerSidebar = React.memo(
}
}
};
const [openNodes, setOpenNodes] = React.useState<Set<string>>(new Set());
const [openNodes, setOpenNodes] = useQueryAndLocalStoragePersistedState<Set<string>>({
// include pathname so that theres separate storage entries for graphs at different URLs
// eg. independent group graph should persist open nodes separately
localStorageKey: `asset-graph-open-sidebar-nodes-${isGlobalGraph}-${explorerPath.pipelineName}`,
encode: (val) => {
return {'open-nodes': Array.from(val)};
},
decode: (qs) => {
return new Set(qs['open-nodes']);
},
isEmptyState: (val) => val.size === 0,
});
const [selectedNode, setSelectedNode] = React.useState<
null | {id: string; path: string} | {id: string}
>(null);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
import {act, renderHook, waitFor} from '@testing-library/react';
import React from 'react';
import {MemoryRouter, Route} from 'react-router-dom';

import {useQueryAndLocalStoragePersistedState} from '../useQueryAndLocalStoragePersistedState';

// Mock local storage
const localStorageMock = (() => {
let store: Record<string, string> = {};

return {
getItem: (key: string) => store[key] || null,
setItem: (key: string, value: string) => {
store[key] = value.toString();
},
removeItem: (key: string) => {
delete store[key];
},
clear: () => {
store = {};
},
};
})();

Object.defineProperty(window, 'localStorage', {
value: localStorageMock,
});

describe('useQueryAndLocalStoragePersistedState', () => {
afterEach(() => {
localStorageMock.clear();
});

test('persists state to localStorage and loads initial state from local storage', async () => {
let querySearch: string | undefined;

const localStorageKey = 'asset-graph-open-nodes';

localStorageMock.setItem(localStorageKey, JSON.stringify({'open-nodes': ['test']}));

const hookResult = renderHook(
() =>
useQueryAndLocalStoragePersistedState<Set<string>>({
localStorageKey: 'asset-graph-open-nodes',
encode: (val) => {
return {'open-nodes': Array.from(val)};
},
decode: (qs) => {
return new Set(qs['open-nodes']);
},
isEmptyState: (val) => val.size === 0,
}),
{
wrapper: ({children}: {children?: React.ReactNode}) => {
return (
<MemoryRouter initialEntries={['/foo/hello']}>
{children}
<Route
path="*"
render={({location}) => (querySearch = location.search) && <span />}
/>
</MemoryRouter>
);
},
},
);

let state, setter: any;

[state, setter] = hookResult.result.current;

// Assert that the state was retrieved from local storage
expect(localStorageMock.getItem(localStorageKey)).toEqual(
JSON.stringify({'open-nodes': ['test']}),
);

expect(state).toEqual(new Set(['test']));

act(() => {
setter(new Set(['test', 'test2']));
});

[state, setter] = hookResult.result.current;

expect(localStorageMock.getItem(localStorageKey)).toEqual(
JSON.stringify({'open-nodes': ['test', 'test2']}),
);

expect(state).toEqual(new Set(['test', 'test2']));

await waitFor(() => {
expect(querySearch).toEqual('?open-nodes%5B%5D=test&open-nodes%5B%5D=test2');
});
});

test('uses queryString as source of truth if query string is present and localStorage data is also present', async () => {
const localStorageKey = 'asset-graph-open-nodes';

localStorageMock.setItem(localStorageKey, JSON.stringify({'open-nodes': ['test']}));

const hookResult = renderHook(
() =>
useQueryAndLocalStoragePersistedState<Set<string>>({
localStorageKey: 'asset-graph-open-nodes',
encode: (val) => {
return {'open-nodes': Array.from(val)};
},
decode: (qs) => {
return new Set(qs['open-nodes']);
},
isEmptyState: (val) => val.size === 0,
}),
{
wrapper: ({children}: {children?: React.ReactNode}) => {
return (
<MemoryRouter
initialEntries={[
'/foo/hello?open-nodes%5B%5D=basic_assets_repository%40toys%3Abasic_assets',
]}
>
{children}
</MemoryRouter>
);
},
},
);

const [state] = hookResult.result.current;

// Assert that the state was retrieved from local storage
expect(localStorageMock.getItem(localStorageKey)).toEqual(
JSON.stringify({'open-nodes': ['test']}),
);

expect(state).toEqual(new Set(['basic_assets_repository@toys:basic_assets']));
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import React from 'react';

import {QueryPersistedDataType, useQueryPersistedState} from './useQueryPersistedState';
import {useSetStateUpdateCallback} from './useSetStateUpdateCallback';

/**
*
* Use URL query string as main source of truth with localStorage as the backup if no state is found in the query string
* Syncs changes back to localStorage but relies solely on queryString after the initial render
* @returns
*/
export const useQueryAndLocalStoragePersistedState = <T extends QueryPersistedDataType>(
props: Parameters<typeof useQueryPersistedState<T>>[0] & {
localStorageKey: string;
isEmptyState: (state: T) => boolean;
},
): [T, (setterOrState: React.SetStateAction<T>) => void] => {
// Grab state from localStorage as "initialState"
const initialState = React.useMemo(() => {
try {
const value = localStorage.getItem(props.localStorageKey);
if (value) {
return props.decode?.(JSON.parse(value));
}
} catch {}
return undefined;
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [props.localStorageKey]);

const [state, setter] = useQueryPersistedState(props);

const isFirstRender = React.useRef(true);
React.useEffect(() => {
if (initialState && props.isEmptyState(state)) {
setter(initialState);
}
isFirstRender.current = false;
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

return [
isFirstRender.current && initialState && props.isEmptyState(state) ? initialState : state,
useSetStateUpdateCallback(state, (nextState) => {
setter(nextState);

// Persist state updates to localStorage
window.localStorage.setItem(
props.localStorageKey,
JSON.stringify(props.encode ? props.encode(nextState) : nextState),
);
}),
];
};
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ import qs from 'qs';
import React from 'react';
import {useHistory, useLocation} from 'react-router-dom';

type QueryPersistedDataType =
import {useSetStateUpdateCallback} from './useSetStateUpdateCallback';

export type QueryPersistedDataType =
| {[key: string]: any}
| Array<any>
| (string | undefined | number)
Expand Down Expand Up @@ -107,7 +109,7 @@ export function useQueryPersistedState<T extends QueryPersistedDataType>(
if (!isEqual(valueRef.current, qsDecoded)) {
valueRef.current = qsDecoded;
}
return [valueRef.current, onChangeRef];
return [valueRef.current, useSetStateUpdateCallback(valueRef.current, onChangeRef)];
}

function inferTypeOfQueryParam<T>(q: any): T {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,16 @@ export interface ExplorerPath {
opNames: string[];
}

export const explorerPathSeparator = '~';

export function explorerPathToString(path: ExplorerPath) {
const root = [
path.pipelineName,
path.snapshotId ? `@${path.snapshotId}` : ``,
path.opsQuery
? `~${path.explodeComposites ? '!' : ''}${encodeURIComponent(path.opsQuery)}`
? `${explorerPathSeparator}${path.explodeComposites ? '!' : ''}${encodeURIComponent(
path.opsQuery,
)}`
: ``,
].join('');

Expand Down

0 comments on commit 818700b

Please sign in to comment.