From e1ab1b12a73b303a13419a460e2e49952fcbc893 Mon Sep 17 00:00:00 2001 From: Patrick Browne Date: Mon, 22 Aug 2022 17:31:51 +0200 Subject: [PATCH 1/2] fix: Fix docs/ route infinite loop Prevent infinite loop in updateRouterQuery --- app/lib/use-route-state.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/lib/use-route-state.ts b/app/lib/use-route-state.ts index 956e15f6d..5b9b6fb3b 100644 --- a/app/lib/use-route-state.ts +++ b/app/lib/use-route-state.ts @@ -7,6 +7,9 @@ export const updateRouterQuery = ( router: NextRouter, values: { [k: string]: string } ) => { + if (Object.entries(values).every(([k, v]) => router.query[k] === v)) { + return; + } router.replace( { pathname: router.pathname, From e6e710a3027cb33e01e60aa9f8facf0211cc11b4 Mon Sep 17 00:00:00 2001 From: Patrick Browne Date: Mon, 22 Aug 2022 17:32:42 +0200 Subject: [PATCH 2/2] fix: Do not save in localStorage nor in URL the dataSource if it's the same as the default one --- app/domain/datasource/index.spec.ts | 26 ++++++++++++++++++++++---- app/domain/datasource/index.ts | 5 +++++ app/lib/use-route-state.ts | 22 +++++++++++++++++----- 3 files changed, 44 insertions(+), 9 deletions(-) diff --git a/app/domain/datasource/index.spec.ts b/app/domain/datasource/index.spec.ts index 818b8b93c..98840e720 100644 --- a/app/domain/datasource/index.spec.ts +++ b/app/domain/datasource/index.spec.ts @@ -10,7 +10,7 @@ jest.mock("next/router", () => ({ jest.mock("../env", () => ({ WHITELISTED_DATA_SOURCES: ["Test", "Prod", "Int"], - ENDPOINT: "sparql+https://lindas.admin.ch/query", + ENDPOINT: "sparql+https://lindas.admin.ch/query", // Default is Prod in tests })); describe("datasource state hook", () => { @@ -68,13 +68,14 @@ describe("datasource state hook", () => { }); it("should have the correct default state from local storage", () => { - const { getState } = setup({ - localStorageValue: "sparql+https://lindas.admin.ch/query", + const { getState, router } = setup({ + localStorageValue: "sparql+https://int.lindas.admin.ch/query", }); expect(getState()).toEqual({ type: "sparql", - url: "https://lindas.admin.ch/query", + url: "https://int.lindas.admin.ch/query", }); + expect(router.query.dataSource).toBe("Int"); }); it("should have the correct default state from URL in priority", () => { @@ -101,4 +102,21 @@ describe("datasource state hook", () => { "sparql+https://lindas.admin.ch/query" ); }); + + it("should not update router when default value is used", () => { + const { router } = setup({ + initialURL: "https://visualize.admin.ch/", + localStorageValue: "", + }); + expect(router.query.dataSource).toBeFalsy(); + expect(localStorage.getItem("dataSource")).toBeFalsy(); + }); + + it("should update router when default value is used and another value is present", () => { + const { router } = setup({ + initialURL: "https://visualize.admin.ch/?dataSource=Int", + localStorageValue: "", + }); + expect(router.query.dataSource).toBe("Int"); + }); }); diff --git a/app/domain/datasource/index.ts b/app/domain/datasource/index.ts index 01c658c2c..651660ed3 100644 --- a/app/domain/datasource/index.ts +++ b/app/domain/datasource/index.ts @@ -90,6 +90,11 @@ export const useDataSourceState = () => { onValueChange: (newSource) => { saveDataSourceToLocalStorage(newSource); }, + shouldValueBeSaved: (item) => + !( + item.type === DEFAULT_DATA_SOURCE.type && + item.url === DEFAULT_DATA_SOURCE.url + ), } ); diff --git a/app/lib/use-route-state.ts b/app/lib/use-route-state.ts index 5b9b6fb3b..e14578ebb 100644 --- a/app/lib/use-route-state.ts +++ b/app/lib/use-route-state.ts @@ -30,11 +30,13 @@ export const useRouteState = ( onValueChange, deserialize, serialize, + shouldValueBeSaved, }: { param: string; onValueChange: (val: T) => void; deserialize: (itemS: string) => T; serialize: (item: T) => string; + shouldValueBeSaved?: (item: T) => boolean; } ) => { const router = useRouter(); @@ -42,16 +44,27 @@ export const useRouteState = ( const [val, rawSetVal] = useState(initialState); const setVal = useEvent((newVal: T) => { + if (val === newVal) { + return; + } rawSetVal(newVal); - updateRouterQuery(router, { [param]: serialize(newVal) }); + if ( + !shouldValueBeSaved || + shouldValueBeSaved?.(newVal) || + router.query[param] + ) { + updateRouterQuery(router, { [param]: serialize(newVal) }); + } onValueChange(newVal); }); const handleRouteChange = useEvent(() => { const routerVal = router.query[param] as string; if (routerVal === undefined) { - // Update router to reflect local state - updateRouterQuery(router, { [param]: serialize(val) }); + if (!shouldValueBeSaved || shouldValueBeSaved?.(val)) { + // Update router to reflect local state + updateRouterQuery(router, { [param]: serialize(val) }); + } } else { // Update local state to reflect router if (routerVal !== serialize(val)) { @@ -67,12 +80,11 @@ export const useRouteState = ( return () => router.events.off("routeChangeComplete", handleRouteChange); }, [handleRouteChange, router.events]); - // Make sure router state is synchronized initially useEffect(() => { if (!router.isReady) { return; } - updateRouterQuery(router, { [param]: serialize(val) }); + handleRouteChange(); // eslint-disable-next-line react-hooks/exhaustive-deps }, [router.isReady]);