From add0fc9f1037baebee6820b38dce1b1a9b59043c Mon Sep 17 00:00:00 2001 From: Lukas Weiss Date: Thu, 27 Jun 2024 14:42:01 +0200 Subject: [PATCH 1/4] make use-query handle array params correctly --- src/engine/query.ts | 8 ++++-- src/routers/__test__/inMemory.test.ts | 22 ++++++++++++++++ src/routers/__test__/nextRouter.test.ts | 24 +++++++++++++++++ src/routers/__test__/reactRouter.test.tsx | 24 +++++++++++++++++ src/routers/reactRouter.ts | 32 ++++++++++------------- 5 files changed, 90 insertions(+), 20 deletions(-) diff --git a/src/engine/query.ts b/src/engine/query.ts index 3912f12..3958bcf 100644 --- a/src/engine/query.ts +++ b/src/engine/query.ts @@ -68,8 +68,12 @@ const DEFAULT_ABSTRACT_QUERY_OPTIONS: AbstractQueryOptions = { convertToQuery: (abstractQuery) => { const query: Query = {} for (const [key, value] of Object.entries(abstractQuery)) { - if (value !== undefined) { - query[key] = value.toString() + if (Array.isArray(value)) { + query[key] = value.map((v) => v.toString()) + } else { + if (value !== undefined) { + query[key] = value.toString() + } } } return query diff --git a/src/routers/__test__/inMemory.test.ts b/src/routers/__test__/inMemory.test.ts index df0ccb6..509f70d 100644 --- a/src/routers/__test__/inMemory.test.ts +++ b/src/routers/__test__/inMemory.test.ts @@ -186,4 +186,26 @@ describe('InMemory', () => { expect(result.current.query.search).toBe(search) }) + + test('should handle array query parameters with single and multiple options', () => { + const optionsSchema = z.object({ + options: z.array(z.string()), + }) + + const { result } = renderHook(() => + useQuery(optionsSchema, { options: [] }), + ) + + act(() => { + result.current.setQuery({ options: ['A'] }) + }) + + expect(result.current.query.options).toStrictEqual(['A']) + + act(() => { + result.current.setQuery({ options: ['A', 'B'] }) + }) + + expect(result.current.query.options).toStrictEqual(['A', 'B']) + }) }) diff --git a/src/routers/__test__/nextRouter.test.ts b/src/routers/__test__/nextRouter.test.ts index 4f9a5b6..8638a3f 100644 --- a/src/routers/__test__/nextRouter.test.ts +++ b/src/routers/__test__/nextRouter.test.ts @@ -345,4 +345,28 @@ describe('NextRouter', () => { expect(result.current.query.department).toBeUndefined() expect(result.current.query.role).toBe(defaultRole) }) + + test('should handle array query parameters with single and multiple options', () => { + const optionsSchema = z.object({ + options: z.array(z.string()), + }) + + const { result } = renderHook(() => + useQuery(optionsSchema, { options: [] }), + ) + + act(() => { + result.current.setQuery({ options: ['A'] }) + }) + + expect(result.current.query.options).toStrictEqual(['A']) + expect(router.query.options).toStrictEqual(['A']) + + act(() => { + result.current.setQuery({ options: ['A', 'B'] }) + }) + + expect(result.current.query.options).toStrictEqual(['A', 'B']) + expect(router.query.options).toStrictEqual(['A', 'B']) + }) }) diff --git a/src/routers/__test__/reactRouter.test.tsx b/src/routers/__test__/reactRouter.test.tsx index 7d3c5da..2eb7218 100644 --- a/src/routers/__test__/reactRouter.test.tsx +++ b/src/routers/__test__/reactRouter.test.tsx @@ -256,4 +256,28 @@ describe('ReactRouter', () => { expect(result.current.query.search).toBe(search) expect(window.location.search).toBe(`?search=`) }) + + test('should handle array query parameters with single and multiple options', () => { + const optionsSchema = z.object({ + options: z.string().or(z.array(z.string())), + }) + + const { result } = renderHookWithContext(() => + useQuery(optionsSchema, { options: [] }), + ) + + act(() => { + result.current.setQuery({ options: ['A'] }) + }) + + expect(result.current.query.options).toStrictEqual('A') + expect(window.location.search).toBe(`?options=A`) + + act(() => { + result.current.setQuery({ options: ['A', 'B'] }) + }) + + expect(result.current.query.options).toStrictEqual(['A', 'B']) + expect(window.location.search).toBe(`?options=A&options=B`) + }) }) diff --git a/src/routers/reactRouter.ts b/src/routers/reactRouter.ts index 744e2ce..bd61864 100644 --- a/src/routers/reactRouter.ts +++ b/src/routers/reactRouter.ts @@ -10,7 +10,7 @@ import { useAbstractQueryAndPagination, Router, } from '../engine' -import { QUERY_ARRAY_SEPARATOR, RouterWithHistoryOptions } from './shared' +import { RouterWithHistoryOptions } from './shared' const DEFAULT_REACT_ROUTER_OPTIONS: RouterWithHistoryOptions = { setQueryMethod: 'replace', @@ -30,14 +30,9 @@ const useReactRouter = ( const getQuery = useCallback( (defaultQuery: Query) => { const query: Query = {} - for (const [key, value] of new URLSearchParams(search).entries()) { - const decodedValues = value - .split(QUERY_ARRAY_SEPARATOR) - .map((v) => decodeURIComponent(v)) - query[key] = - decodedValues.length === 1 - ? (decodedValues[0] as string) - : decodedValues + for (const key of new URLSearchParams(search).keys()) { + const value = new URLSearchParams(search).getAll(key) + query[key] = value.length === 1 ? (value[0] as string) : value } return { ...defaultQuery, ...query } }, @@ -51,16 +46,17 @@ const useReactRouter = ( if (value === defaultQuery[key]) { urlSearchParams.delete(key) } else if (value !== undefined) { - let values: string[] - if (Array.isArray(value)) { - values = value - } else { - values = [value] + const [firstValue, ...restValues] = Array.isArray(value) + ? value + : [value] + + if (firstValue !== undefined) { + urlSearchParams.set(key, firstValue) + } + + for (const restValue of restValues) { + urlSearchParams.append(key, restValue) } - const encodedValues = values - .map((v) => encodeURIComponent(v)) - .join(QUERY_ARRAY_SEPARATOR) - urlSearchParams.set(key, encodedValues) } } navigate( From 0c9ec13fde51201aeb76d50795ccdc86d0fda427 Mon Sep 17 00:00:00 2001 From: Lukas Weiss Date: Thu, 27 Jun 2024 15:03:03 +0200 Subject: [PATCH 2/4] remove array values correctly from url if equal to default value --- src/routers/__test__/nextRouter.test.ts | 39 ++++++++++++++++++++--- src/routers/__test__/reactRouter.test.tsx | 39 ++++++++++++++++++++--- src/routers/reactRouter.ts | 29 ++++++++++------- 3 files changed, 88 insertions(+), 19 deletions(-) diff --git a/src/routers/__test__/nextRouter.test.ts b/src/routers/__test__/nextRouter.test.ts index 8638a3f..eebfa81 100644 --- a/src/routers/__test__/nextRouter.test.ts +++ b/src/routers/__test__/nextRouter.test.ts @@ -15,6 +15,7 @@ describe('NextRouter', () => { const searchSchema = z.object({ search: z.string().optional().catch(undefined), + options: z.string().or(z.array(z.string())).optional().catch(undefined), }) const useNextRouterQueryWithSearch = ( @@ -289,13 +290,17 @@ describe('NextRouter', () => { expect(router.query.greeting).toBe(greeting) }) - test('query keys with default value should not be stored in the url', () => { - const defaultSearch = '' + test('query keys with default value by reset should not be stored in the url', () => { + const defaultSearch = 'Default' + const defaultOptions = ['A', 'B'] - router.query = { search: 'Max' } + router.query = { search: 'Max', options: ['X', 'Y'] } const { result } = renderHook(() => - useQuery(searchSchema, { search: defaultSearch }), + useQuery(searchSchema, { + search: defaultSearch, + options: defaultOptions, + }), ) act(() => { @@ -303,6 +308,32 @@ describe('NextRouter', () => { }) expect(result.current.query.search).toBe(defaultSearch) + expect(result.current.query.options).toStrictEqual(defaultOptions) + expect(router.query.search).toBeUndefined() + }) + + test('query keys with default value by set should not be stored in the url', () => { + const defaultSearch = 'Default' + const defaultOptions = ['A', 'B'] + + router.query = { search: 'Max', options: ['X', 'Y'] } + + const { result } = renderHook(() => + useQuery(searchSchema, { + search: defaultSearch, + options: defaultOptions, + }), + ) + + act(() => { + result.current.setQuery({ + search: defaultSearch, + options: defaultOptions, + }) + }) + + expect(result.current.query.search).toBe(defaultSearch) + expect(result.current.query.options).toStrictEqual(defaultOptions) expect(router.query.search).toBeUndefined() }) diff --git a/src/routers/__test__/reactRouter.test.tsx b/src/routers/__test__/reactRouter.test.tsx index 2eb7218..88a1cbd 100644 --- a/src/routers/__test__/reactRouter.test.tsx +++ b/src/routers/__test__/reactRouter.test.tsx @@ -20,6 +20,7 @@ describe('ReactRouter', () => { const searchSchema = z.object({ search: z.string().optional().catch(undefined), + options: z.string().or(z.array(z.string())).optional().catch(undefined), }) const useReactRouterQueryWithSearch = ( @@ -225,13 +226,17 @@ describe('ReactRouter', () => { ) }) - test('query keys with default value should not be stored in the url', () => { - const defaultSearch = '' + test('query keys with default value by reset should not be stored in the url', () => { + const defaultSearch = 'Default' + const defaultOptions = ['A', 'B'] - window.history.pushState({}, '', `/?search=Max`) + window.history.pushState({}, '', `/?search=Max&options=X&options=Y`) const { result } = renderHookWithContext(() => - useQuery(searchSchema, { search: defaultSearch }), + useQuery(searchSchema, { + search: defaultSearch, + options: defaultOptions, + }), ) act(() => { @@ -239,6 +244,32 @@ describe('ReactRouter', () => { }) expect(result.current.query.search).toBe(defaultSearch) + expect(result.current.query.options).toStrictEqual(defaultOptions) + expect(window.location.search).toBe('') + }) + + test('query keys with default value by set should not be stored in the url', () => { + const defaultSearch = 'Default' + const defaultOptions = ['A', 'B'] + + window.history.pushState({}, '', `/?search=Max&options=X&options=Y`) + + const { result } = renderHookWithContext(() => + useQuery(searchSchema, { + search: defaultSearch, + options: defaultOptions, + }), + ) + + act(() => { + result.current.setQuery({ + search: defaultSearch, + options: defaultOptions, + }) + }) + + expect(result.current.query.search).toBe(defaultSearch) + expect(result.current.query.options).toStrictEqual(defaultOptions) expect(window.location.search).toBe('') }) diff --git a/src/routers/reactRouter.ts b/src/routers/reactRouter.ts index bd61864..c8fb1a5 100644 --- a/src/routers/reactRouter.ts +++ b/src/routers/reactRouter.ts @@ -43,19 +43,26 @@ const useReactRouter = ( (query: Partial, defaultQuery: Query) => { const urlSearchParams = new URLSearchParams(search) for (const [key, value] of Object.entries(query)) { - if (value === defaultQuery[key]) { - urlSearchParams.delete(key) - } else if (value !== undefined) { - const [firstValue, ...restValues] = Array.isArray(value) - ? value - : [value] + if (value !== undefined) { + const defaultValue = defaultQuery[key] - if (firstValue !== undefined) { - urlSearchParams.set(key, firstValue) - } + if ( + defaultValue !== undefined && + value.toString() === defaultValue.toString() + ) { + urlSearchParams.delete(key) + } else { + const [firstValue, ...restValues] = Array.isArray(value) + ? value + : [value] + + if (firstValue !== undefined) { + urlSearchParams.set(key, firstValue) + } - for (const restValue of restValues) { - urlSearchParams.append(key, restValue) + for (const restValue of restValues) { + urlSearchParams.append(key, restValue) + } } } } From e5e05091d0dc7ed3bd0c9f2079813a7c29a50756 Mon Sep 17 00:00:00 2001 From: Lukas Weiss Date: Thu, 27 Jun 2024 15:06:34 +0200 Subject: [PATCH 3/4] remove obsolete const query-array-separator --- src/routers/shared.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/routers/shared.ts b/src/routers/shared.ts index 13971dd..8064e31 100644 --- a/src/routers/shared.ts +++ b/src/routers/shared.ts @@ -1,5 +1,3 @@ -export const QUERY_ARRAY_SEPARATOR = ',' - export type RouterWithHistoryOptions = { /** * Whether the router should push a new URL to the browser history or replace the current URL. From 5a9363df8c6f3f6f140714013c49fdf1d3723ee5 Mon Sep 17 00:00:00 2001 From: Lukas Weiss Date: Thu, 27 Jun 2024 16:31:48 +0200 Subject: [PATCH 4/4] reuse url-search-params in loop --- src/routers/reactRouter.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/routers/reactRouter.ts b/src/routers/reactRouter.ts index c8fb1a5..f522102 100644 --- a/src/routers/reactRouter.ts +++ b/src/routers/reactRouter.ts @@ -30,8 +30,9 @@ const useReactRouter = ( const getQuery = useCallback( (defaultQuery: Query) => { const query: Query = {} - for (const key of new URLSearchParams(search).keys()) { - const value = new URLSearchParams(search).getAll(key) + const urlSearchParams = new URLSearchParams(search) + for (const key of urlSearchParams.keys()) { + const value = urlSearchParams.getAll(key) query[key] = value.length === 1 ? (value[0] as string) : value } return { ...defaultQuery, ...query }