From c456537784e52b28ce804eb2db97409f884e3dcf Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Thu, 6 Mar 2025 15:36:43 -0500 Subject: [PATCH 1/5] add behavior --- .../ui-core/src/hooks/useQueryPersistedState.tsx | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/js_modules/dagster-ui/packages/ui-core/src/hooks/useQueryPersistedState.tsx b/js_modules/dagster-ui/packages/ui-core/src/hooks/useQueryPersistedState.tsx index e540d3acd32c8..ecfb4711d6b3a 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/hooks/useQueryPersistedState.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/hooks/useQueryPersistedState.tsx @@ -21,6 +21,7 @@ export type QueryPersistedStateConfig = { defaults?: {[key: string]: any}; decode?: (raw: {[key: string]: any}) => T; encode?: (raw: T) => {[key: string]: any}; + behavior?: 'push' | 'replace'; }; const defaultEncode = memoize((queryKey: string) => (raw: T) => ({[queryKey]: raw})); @@ -63,7 +64,7 @@ const defaultDecode = memoize( export function useQueryPersistedState( options: QueryPersistedStateConfig, ): [T, React.Dispatch>] { - const {queryKey, defaults} = options; + const {queryKey, defaults, behavior = 'replace'} = options; let {encode, decode} = options; if (queryKey) { @@ -113,9 +114,15 @@ export function useQueryPersistedState( // the `replace` so that we surface any unwanted loops during development. if (process.env.NODE_ENV !== 'production' || !areQueriesEqual(currentQueryString, next)) { currentQueryString = next; - history.replace( - `${history.location.pathname}?${qs.stringify(next, {arrayFormat: 'brackets'})}`, - ); + if (behavior === 'replace') { + history.replace( + `${history.location.pathname}?${qs.stringify(next, {arrayFormat: 'brackets'})}`, + ); + } else { + history.push( + `${history.location.pathname}?${qs.stringify(next, {arrayFormat: 'brackets'})}`, + ); + } } }, [history, encode, options], From 86f13449117e4abed78149454ee2a5814f2e3971 Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Thu, 6 Mar 2025 15:46:43 -0500 Subject: [PATCH 2/5] add push behavior to useQueryPersistedState --- .../__tests__/useQueryPersistedState.test.tsx | 112 +++++++++++++++++- 1 file changed, 110 insertions(+), 2 deletions(-) diff --git a/js_modules/dagster-ui/packages/ui-core/src/hooks/__tests__/useQueryPersistedState.test.tsx b/js_modules/dagster-ui/packages/ui-core/src/hooks/__tests__/useQueryPersistedState.test.tsx index 890991cced98a..6f571a4bc58ec 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/hooks/__tests__/useQueryPersistedState.test.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/hooks/__tests__/useQueryPersistedState.test.tsx @@ -1,7 +1,7 @@ -import {render, screen, waitFor} from '@testing-library/react'; +import {act, render, screen, waitFor} from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import {useCallback, useMemo} from 'react'; -import {MemoryRouter} from 'react-router-dom'; +import {MemoryRouter, useHistory} from 'react-router-dom'; import {Route} from '../../app/Route'; import {useQueryPersistedState} from '../useQueryPersistedState'; @@ -348,4 +348,112 @@ describe('useQueryPersistedState', () => { ); }); }); + + it('supports push behavior', async () => { + let querySearch: string | undefined; + + let goback: () => void; + + const Test = ({options}: {options: Parameters[0]}) => { + const [_, setQuery] = useQueryPersistedState(options); + const history = useHistory(); + goback = () => history.goBack(); + return ( + <> +
setQuery('one')}>one
+
setQuery('two')}>two
+
setQuery('three')}>three
+ + ); + }; + + render( + + + (querySearch = location.search) && } /> + , + ); + + await userEvent.click(screen.getByText(`one`)); + await waitFor(() => { + expect(querySearch).toEqual('?q=one'); + }); + + await userEvent.click(screen.getByText(`two`)); + await waitFor(() => { + expect(querySearch).toEqual('?q=two'); + }); + + await userEvent.click(screen.getByText(`three`)); + await waitFor(() => { + expect(querySearch).toEqual('?q=three'); + }); + + await act(() => { + goback(); + }); + + await waitFor(() => { + expect(querySearch).toEqual('?q=two'); + }); + + await act(() => { + goback(); + }); + + await waitFor(() => { + expect(querySearch).toEqual('?q=one'); + }); + }); + + it('supports replace behavior', async () => { + let querySearch: string | undefined; + + let goback: () => void; + let push: (...args: Parameters['push']>) => void; + + const Test = ({options}: {options: Parameters[0]}) => { + const [_, setQuery] = useQueryPersistedState(options); + const history = useHistory(); + goback = () => history.goBack(); + push = history.push.bind(history); + return ( + <> +
setQuery('one')}>one
+
setQuery('two')}>two
+
setQuery('three')}>three
+ + ); + }; + + render( + + + (querySearch = location.search) && } /> + , + ); + + push!('/page?'); + + await userEvent.click(screen.getByText(`one`)); + await waitFor(() => { + expect(querySearch).toEqual('?q=one'); + }); + + await userEvent.click(screen.getByText(`two`)); + await waitFor(() => { + expect(querySearch).toEqual('?q=two'); + }); + + await userEvent.click(screen.getByText(`three`)); + await waitFor(() => { + expect(querySearch).toEqual('?q=three'); + }); + + await act(() => { + goback(); + }); + + expect(querySearch).toEqual('?q=B'); // end up back on initial route + }); }); From 22175cecf3a8aae21a44b9d01ca4b0ae5c7ee12b Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Thu, 6 Mar 2025 15:51:02 -0500 Subject: [PATCH 3/5] push --- .../ui-core/src/asset-selection/useAssetSelectionState.oss.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/js_modules/dagster-ui/packages/ui-core/src/asset-selection/useAssetSelectionState.oss.tsx b/js_modules/dagster-ui/packages/ui-core/src/asset-selection/useAssetSelectionState.oss.tsx index 080cc61ff0b20..501871204a37b 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/asset-selection/useAssetSelectionState.oss.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/asset-selection/useAssetSelectionState.oss.tsx @@ -4,5 +4,6 @@ export function useAssetSelectionState() { return useQueryPersistedState({ queryKey: 'asset-selection', defaults: {['asset-selection']: ''}, + behavior: 'push', }); } From 31b6347351f39813050bf92c2ac7f5dcad45032e Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Fri, 7 Mar 2025 13:08:58 -0500 Subject: [PATCH 4/5] add dep --- .../packages/ui-core/src/hooks/useQueryPersistedState.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js_modules/dagster-ui/packages/ui-core/src/hooks/useQueryPersistedState.tsx b/js_modules/dagster-ui/packages/ui-core/src/hooks/useQueryPersistedState.tsx index ecfb4711d6b3a..2485aa06c8cc5 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/hooks/useQueryPersistedState.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/hooks/useQueryPersistedState.tsx @@ -125,7 +125,7 @@ export function useQueryPersistedState( } } }, - [history, encode, options], + [encode, options.defaults, behavior, history], ); if (!isEqual(valueRef.current, qsDecoded)) { From 69f537544ef48b3f94ac571c90396bd1e5b0704a Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Fri, 7 Mar 2025 13:10:49 -0500 Subject: [PATCH 5/5] feedback --- .../ui-core/src/hooks/useQueryPersistedState.tsx | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/js_modules/dagster-ui/packages/ui-core/src/hooks/useQueryPersistedState.tsx b/js_modules/dagster-ui/packages/ui-core/src/hooks/useQueryPersistedState.tsx index 2485aa06c8cc5..63cccfe5a3f1d 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/hooks/useQueryPersistedState.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/hooks/useQueryPersistedState.tsx @@ -114,14 +114,11 @@ export function useQueryPersistedState( // the `replace` so that we surface any unwanted loops during development. if (process.env.NODE_ENV !== 'production' || !areQueriesEqual(currentQueryString, next)) { currentQueryString = next; + const nextPath = `${history.location.pathname}?${qs.stringify(next, {arrayFormat: 'brackets'})}`; if (behavior === 'replace') { - history.replace( - `${history.location.pathname}?${qs.stringify(next, {arrayFormat: 'brackets'})}`, - ); + history.replace(nextPath); } else { - history.push( - `${history.location.pathname}?${qs.stringify(next, {arrayFormat: 'brackets'})}`, - ); + history.push(nextPath); } } },