From 942c53a3dfb6ae17345680330fc7d282a759b2d4 Mon Sep 17 00:00:00 2001 From: George Gritsouk <989898+gggritso@users.noreply.github.com> Date: Tue, 20 Jan 2026 16:51:52 -0500 Subject: [PATCH 1/6] feat(utils): Add useQueryStateWithLocalStorage hook for URL + localStorage state sync This hook combines Nuqs (URL query parameters) with localStorage to provide persistent state that can also be shared via URL. Features: - URL query parameter takes precedence (enables sharing) - Falls back to localStorage when URL param is absent - Falls back to default value if neither exists - Single setter updates both URL and localStorage - Automatic localStorage key construction: namespace:key Use cases: - User preferences that should persist across sessions - Filters/sort options that should be shareable via URL - Any state that benefits from both persistence and shareability Example usage: ```typescript const [sortBy, setSortBy] = useQueryStateWithLocalStorage( 'releaseFilterSort', // URL param name 'dashboards', // localStorage namespace 'date' // default value ); ``` This creates: - URL param: ?releaseFilterSort=date - localStorage key: dashboards:releaseFilterSort Tests: - 7 comprehensive test cases covering all scenarios - Uses official Nuqs testing adapter - Verifies URL/localStorage sync behavior --- .../useQueryStateWithLocalStorage.spec.tsx | 147 ++++++++++++++++++ .../url/useQueryStateWithLocalStorage.tsx | 51 ++++++ 2 files changed, 198 insertions(+) create mode 100644 static/app/utils/url/useQueryStateWithLocalStorage.spec.tsx create mode 100644 static/app/utils/url/useQueryStateWithLocalStorage.tsx diff --git a/static/app/utils/url/useQueryStateWithLocalStorage.spec.tsx b/static/app/utils/url/useQueryStateWithLocalStorage.spec.tsx new file mode 100644 index 00000000000000..b3e82c52900275 --- /dev/null +++ b/static/app/utils/url/useQueryStateWithLocalStorage.spec.tsx @@ -0,0 +1,147 @@ +import {withNuqsTestingAdapter} from 'nuqs/adapters/testing'; + +import {act, renderHook, waitFor} from 'sentry-test/reactTestingLibrary'; + +import localStorageWrapper from 'sentry/utils/localStorage'; +import {useQueryStateWithLocalStorage} from 'sentry/utils/url/useQueryStateWithLocalStorage'; + +describe('useQueryStateWithLocalStorage', () => { + beforeEach(() => { + localStorageWrapper.clear(); + }); + + it('returns default value when neither URL nor localStorage has value', () => { + const {result} = renderHook( + () => useQueryStateWithLocalStorage('testParam', 'testNamespace', 'default'), + { + wrapper: withNuqsTestingAdapter(), + } + ); + + expect(result.current[0]).toBe('default'); + }); + + it('returns localStorage value when URL is empty', () => { + localStorageWrapper.setItem('testNamespace:testParam', JSON.stringify('fromStorage')); + + const {result} = renderHook( + () => useQueryStateWithLocalStorage('testParam', 'testNamespace', 'default'), + { + wrapper: withNuqsTestingAdapter(), + } + ); + + expect(result.current[0]).toBe('fromStorage'); + }); + + it('returns URL value when URL has value (URL takes precedence)', () => { + localStorageWrapper.setItem('testNamespace:testParam', JSON.stringify('fromStorage')); + + const {result} = renderHook( + () => useQueryStateWithLocalStorage('testParam', 'testNamespace', 'default'), + { + wrapper: withNuqsTestingAdapter({ + searchParams: {testParam: 'fromURL'}, + }), + } + ); + + expect(result.current[0]).toBe('fromURL'); + }); + + it('syncs localStorage when URL changes', async () => { + renderHook( + () => useQueryStateWithLocalStorage('testParam', 'testNamespace', 'default'), + { + wrapper: withNuqsTestingAdapter({ + searchParams: {testParam: 'newURLValue'}, + }), + } + ); + + await waitFor(() => { + const storedValue = localStorageWrapper.getItem('testNamespace:testParam'); + expect(storedValue).toBe(JSON.stringify('newURLValue')); + }); + }); + + it('setValue updates both URL and localStorage', async () => { + const onUrlUpdate = jest.fn(); + + const {result} = renderHook( + () => useQueryStateWithLocalStorage('testParam', 'testNamespace', 'default'), + { + wrapper: withNuqsTestingAdapter({onUrlUpdate}), + } + ); + + act(() => { + result.current[1]('newValue'); + }); + + await waitFor(() => { + expect(onUrlUpdate).toHaveBeenCalledWith( + expect.objectContaining({ + queryString: expect.stringContaining('testParam=newValue'), + }) + ); + }); + + await waitFor(() => { + const storedValue = localStorageWrapper.getItem('testNamespace:testParam'); + expect(storedValue).toBe(JSON.stringify('newValue')); + }); + }); + + it('works with enum-like string types', async () => { + type SortOption = 'date' | 'name' | 'size'; + + localStorageWrapper.setItem('myNamespace:sort', JSON.stringify('name')); + + const onUrlUpdate = jest.fn(); + + const {result} = renderHook( + () => useQueryStateWithLocalStorage('sort', 'myNamespace', 'date'), + { + wrapper: withNuqsTestingAdapter({onUrlUpdate}), + } + ); + + expect(result.current[0]).toBe('name'); + + act(() => { + result.current[1]('size'); + }); + + await waitFor(() => { + expect(onUrlUpdate).toHaveBeenCalledWith( + expect.objectContaining({ + queryString: expect.stringContaining('sort=size'), + }) + ); + }); + + await waitFor(() => { + const storedValue = localStorageWrapper.getItem('myNamespace:sort'); + expect(storedValue).toBe(JSON.stringify('size')); + }); + }); + + it('does not sync localStorage if URL value matches localStorage', () => { + localStorageWrapper.setItem('testNamespace:testParam', JSON.stringify('sameValue')); + + const {result} = renderHook( + () => useQueryStateWithLocalStorage('testParam', 'testNamespace', 'default'), + { + wrapper: withNuqsTestingAdapter({ + searchParams: {testParam: 'sameValue'}, + }), + } + ); + + expect(result.current[0]).toBe('sameValue'); + + const storedValue = localStorageWrapper.getItem('testNamespace:testParam'); + expect(storedValue).toBe(JSON.stringify('sameValue')); + }); +}); diff --git a/static/app/utils/url/useQueryStateWithLocalStorage.tsx b/static/app/utils/url/useQueryStateWithLocalStorage.tsx new file mode 100644 index 00000000000000..9625f306e06871 --- /dev/null +++ b/static/app/utils/url/useQueryStateWithLocalStorage.tsx @@ -0,0 +1,51 @@ +import {useEffect} from 'react'; +import {parseAsString, useQueryState} from 'nuqs'; + +import {useLocalStorageState} from 'sentry/utils/useLocalStorageState'; + +/** + * Hook that syncs state between URL query parameters and localStorage. + * URL takes precedence over localStorage, enabling URL sharing while + * maintaining user preferences across sessions. + * + * @param key - The URL query parameter name + * @param namespace - The localStorage namespace (key will be `namespace:key`) + * @param defaultValue - The default value if neither source has a value + * @returns Tuple of [effectiveValue, setValue] + * + * @example + * const [sortBy, setSortBy] = useQueryStateWithLocalStorage( + * 'sortBy', + * 'dashboards', + * 'date' + * ); + */ +export function useQueryStateWithLocalStorage( + key: string, + namespace: string, + defaultValue: T +): [T, (value: T) => void] { + const [urlValue, setUrlValue] = useQueryState(key, parseAsString); + + const localStorageKey = `${namespace}:${key}`; + + const [localStorageValue, setLocalStorageValue] = useLocalStorageState( + localStorageKey, + defaultValue + ); + + const effectiveValue = (urlValue ?? localStorageValue ?? defaultValue) as T; + + useEffect(() => { + if (urlValue && urlValue !== localStorageValue) { + setLocalStorageValue(urlValue as T); + } + }, [urlValue, localStorageValue, setLocalStorageValue]); + + const setValue = (value: T) => { + setUrlValue(value); + setLocalStorageValue(value); + }; + + return [effectiveValue, setValue]; +} From 5d8665ccb322331adca00723b30ce61c0337761d Mon Sep 17 00:00:00 2001 From: George Gritsouk <989898+gggritso@users.noreply.github.com> Date: Tue, 20 Jan 2026 17:00:19 -0500 Subject: [PATCH 2/6] refactor(useQueryStateWithLocalStorage): Change API to use props object Change from three positional arguments to a single props object for better clarity and easier future extension. Before: ```typescript useQueryStateWithLocalStorage(key, namespace, defaultValue) ``` After: ```typescript useQueryStateWithLocalStorage({key, namespace, defaultValue}) ``` Benefits: - Named parameters improve readability - Order doesn't matter - Easier to add optional parameters in the future - More consistent with modern React patterns --- .../useQueryStateWithLocalStorage.spec.tsx | 49 ++++++++++++++++--- .../url/useQueryStateWithLocalStorage.tsx | 31 +++++++----- 2 files changed, 60 insertions(+), 20 deletions(-) diff --git a/static/app/utils/url/useQueryStateWithLocalStorage.spec.tsx b/static/app/utils/url/useQueryStateWithLocalStorage.spec.tsx index b3e82c52900275..5de697a278f3b1 100644 --- a/static/app/utils/url/useQueryStateWithLocalStorage.spec.tsx +++ b/static/app/utils/url/useQueryStateWithLocalStorage.spec.tsx @@ -12,7 +12,12 @@ describe('useQueryStateWithLocalStorage', () => { it('returns default value when neither URL nor localStorage has value', () => { const {result} = renderHook( - () => useQueryStateWithLocalStorage('testParam', 'testNamespace', 'default'), + () => + useQueryStateWithLocalStorage({ + key: 'testParam', + namespace: 'testNamespace', + defaultValue: 'default', + }), { wrapper: withNuqsTestingAdapter(), } @@ -25,7 +30,12 @@ describe('useQueryStateWithLocalStorage', () => { localStorageWrapper.setItem('testNamespace:testParam', JSON.stringify('fromStorage')); const {result} = renderHook( - () => useQueryStateWithLocalStorage('testParam', 'testNamespace', 'default'), + () => + useQueryStateWithLocalStorage({ + key: 'testParam', + namespace: 'testNamespace', + defaultValue: 'default', + }), { wrapper: withNuqsTestingAdapter(), } @@ -38,7 +48,12 @@ describe('useQueryStateWithLocalStorage', () => { localStorageWrapper.setItem('testNamespace:testParam', JSON.stringify('fromStorage')); const {result} = renderHook( - () => useQueryStateWithLocalStorage('testParam', 'testNamespace', 'default'), + () => + useQueryStateWithLocalStorage({ + key: 'testParam', + namespace: 'testNamespace', + defaultValue: 'default', + }), { wrapper: withNuqsTestingAdapter({ searchParams: {testParam: 'fromURL'}, @@ -51,7 +66,12 @@ describe('useQueryStateWithLocalStorage', () => { it('syncs localStorage when URL changes', async () => { renderHook( - () => useQueryStateWithLocalStorage('testParam', 'testNamespace', 'default'), + () => + useQueryStateWithLocalStorage({ + key: 'testParam', + namespace: 'testNamespace', + defaultValue: 'default', + }), { wrapper: withNuqsTestingAdapter({ searchParams: {testParam: 'newURLValue'}, @@ -69,7 +89,12 @@ describe('useQueryStateWithLocalStorage', () => { const onUrlUpdate = jest.fn(); const {result} = renderHook( - () => useQueryStateWithLocalStorage('testParam', 'testNamespace', 'default'), + () => + useQueryStateWithLocalStorage({ + key: 'testParam', + namespace: 'testNamespace', + defaultValue: 'default', + }), { wrapper: withNuqsTestingAdapter({onUrlUpdate}), } @@ -101,7 +126,12 @@ describe('useQueryStateWithLocalStorage', () => { const onUrlUpdate = jest.fn(); const {result} = renderHook( - () => useQueryStateWithLocalStorage('sort', 'myNamespace', 'date'), + () => + useQueryStateWithLocalStorage({ + key: 'sort', + namespace: 'myNamespace', + defaultValue: 'date', + }), { wrapper: withNuqsTestingAdapter({onUrlUpdate}), } @@ -131,7 +161,12 @@ describe('useQueryStateWithLocalStorage', () => { localStorageWrapper.setItem('testNamespace:testParam', JSON.stringify('sameValue')); const {result} = renderHook( - () => useQueryStateWithLocalStorage('testParam', 'testNamespace', 'default'), + () => + useQueryStateWithLocalStorage({ + key: 'testParam', + namespace: 'testNamespace', + defaultValue: 'default', + }), { wrapper: withNuqsTestingAdapter({ searchParams: {testParam: 'sameValue'}, diff --git a/static/app/utils/url/useQueryStateWithLocalStorage.tsx b/static/app/utils/url/useQueryStateWithLocalStorage.tsx index 9625f306e06871..6727f0853eb1bd 100644 --- a/static/app/utils/url/useQueryStateWithLocalStorage.tsx +++ b/static/app/utils/url/useQueryStateWithLocalStorage.tsx @@ -8,23 +8,28 @@ import {useLocalStorageState} from 'sentry/utils/useLocalStorageState'; * URL takes precedence over localStorage, enabling URL sharing while * maintaining user preferences across sessions. * - * @param key - The URL query parameter name - * @param namespace - The localStorage namespace (key will be `namespace:key`) - * @param defaultValue - The default value if neither source has a value + * @param props - Configuration object + * @param props.key - The URL query parameter name + * @param props.namespace - The localStorage namespace (key will be `namespace:key`) + * @param props.defaultValue - The default value if neither source has a value * @returns Tuple of [effectiveValue, setValue] * * @example - * const [sortBy, setSortBy] = useQueryStateWithLocalStorage( - * 'sortBy', - * 'dashboards', - * 'date' - * ); + * const [sortBy, setSortBy] = useQueryStateWithLocalStorage({ + * key: 'sortBy', + * namespace: 'dashboards', + * defaultValue: 'date', + * }); */ -export function useQueryStateWithLocalStorage( - key: string, - namespace: string, - defaultValue: T -): [T, (value: T) => void] { +export function useQueryStateWithLocalStorage({ + key, + namespace, + defaultValue, +}: { + defaultValue: T; + key: string; + namespace: string; +}): [T, (value: T) => void] { const [urlValue, setUrlValue] = useQueryState(key, parseAsString); const localStorageKey = `${namespace}:${key}`; From 06ce00bf1a0cc3d5ee2f5e13ddecc05550c612a9 Mon Sep 17 00:00:00 2001 From: George Gritsouk <989898+gggritso@users.noreply.github.com> Date: Tue, 20 Jan 2026 17:16:01 -0500 Subject: [PATCH 3/6] feat(useQueryStateWithLocalStorage): Add support for all Nuqs data types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extend the hook to support any data type via Nuqs parsers, matching the Nuqs API where parser is optional and defaults to parseAsString. Changes: - Add optional 'parser' parameter that accepts any Nuqs Parser - Defaults to parseAsString for backward compatibility - Remove 'T extends string' constraint - now supports any type - Parser handles URL serialization/deserialization - localStorage uses JSON for storage (supports all JS types) - Update docs with examples for string, integer, and boolean types - Update tests: some use default parser, others explicitly test types - Add 3 new tests for integer and boolean parsers Example usage: ```typescript // Strings (parser optional, defaults to parseAsString) const [sort, setSort] = useQueryStateWithLocalStorage({ key: 'sort', namespace: 'dashboards', defaultValue: 'date', }); // Integers (explicit parser) const [pageSize, setPageSize] = useQueryStateWithLocalStorage({ key: 'pageSize', namespace: 'dashboards', defaultValue: 50, parser: parseAsInteger, }); // Booleans (explicit parser) const [enabled, setEnabled] = useQueryStateWithLocalStorage({ key: 'enabled', namespace: 'dashboards', defaultValue: false, parser: parseAsBoolean, }); ``` All 10 tests passing ✓ --- .../useQueryStateWithLocalStorage.spec.tsx | 104 ++++++++++++++++++ .../url/useQueryStateWithLocalStorage.tsx | 38 ++++++- 2 files changed, 136 insertions(+), 6 deletions(-) diff --git a/static/app/utils/url/useQueryStateWithLocalStorage.spec.tsx b/static/app/utils/url/useQueryStateWithLocalStorage.spec.tsx index 5de697a278f3b1..5b8988f46c23f7 100644 --- a/static/app/utils/url/useQueryStateWithLocalStorage.spec.tsx +++ b/static/app/utils/url/useQueryStateWithLocalStorage.spec.tsx @@ -1,3 +1,4 @@ +import {parseAsBoolean, parseAsInteger, parseAsString} from 'nuqs'; import {withNuqsTestingAdapter} from 'nuqs/adapters/testing'; import {act, renderHook, waitFor} from 'sentry-test/reactTestingLibrary'; @@ -53,6 +54,7 @@ describe('useQueryStateWithLocalStorage', () => { key: 'testParam', namespace: 'testNamespace', defaultValue: 'default', + parser: parseAsString, }), { wrapper: withNuqsTestingAdapter({ @@ -71,6 +73,7 @@ describe('useQueryStateWithLocalStorage', () => { key: 'testParam', namespace: 'testNamespace', defaultValue: 'default', + parser: parseAsString, }), { wrapper: withNuqsTestingAdapter({ @@ -94,6 +97,7 @@ describe('useQueryStateWithLocalStorage', () => { key: 'testParam', namespace: 'testNamespace', defaultValue: 'default', + parser: parseAsString, }), { wrapper: withNuqsTestingAdapter({onUrlUpdate}), @@ -166,6 +170,7 @@ describe('useQueryStateWithLocalStorage', () => { key: 'testParam', namespace: 'testNamespace', defaultValue: 'default', + parser: parseAsString, }), { wrapper: withNuqsTestingAdapter({ @@ -179,4 +184,103 @@ describe('useQueryStateWithLocalStorage', () => { const storedValue = localStorageWrapper.getItem('testNamespace:testParam'); expect(storedValue).toBe(JSON.stringify('sameValue')); }); + + it('works with integer values using parseAsInteger', async () => { + localStorageWrapper.setItem('testNamespace:count', JSON.stringify(42)); + + const onUrlUpdate = jest.fn(); + + const {result} = renderHook( + () => + useQueryStateWithLocalStorage({ + key: 'count', + namespace: 'testNamespace', + defaultValue: 0, + parser: parseAsInteger, + }), + { + wrapper: withNuqsTestingAdapter({onUrlUpdate}), + } + ); + + expect(result.current[0]).toBe(42); + expect(typeof result.current[0]).toBe('number'); + + act(() => { + result.current[1](100); + }); + + await waitFor(() => { + expect(onUrlUpdate).toHaveBeenCalledWith( + expect.objectContaining({ + queryString: expect.stringContaining('count=100'), + }) + ); + }); + + await waitFor(() => { + const storedValue = localStorageWrapper.getItem('testNamespace:count'); + expect(storedValue).toBe(JSON.stringify(100)); + }); + }); + + it('works with boolean values using parseAsBoolean', async () => { + localStorageWrapper.setItem('testNamespace:enabled', JSON.stringify(true)); + + const onUrlUpdate = jest.fn(); + + const {result} = renderHook( + () => + useQueryStateWithLocalStorage({ + key: 'enabled', + namespace: 'testNamespace', + defaultValue: false, + parser: parseAsBoolean, + }), + { + wrapper: withNuqsTestingAdapter({onUrlUpdate}), + } + ); + + expect(result.current[0]).toBe(true); + expect(typeof result.current[0]).toBe('boolean'); + + act(() => { + result.current[1](false); + }); + + await waitFor(() => { + expect(onUrlUpdate).toHaveBeenCalledWith( + expect.objectContaining({ + queryString: expect.stringContaining('enabled=false'), + }) + ); + }); + + await waitFor(() => { + const storedValue = localStorageWrapper.getItem('testNamespace:enabled'); + expect(storedValue).toBe(JSON.stringify(false)); + }); + }); + + it('URL integer value overrides localStorage', () => { + localStorageWrapper.setItem('testNamespace:pageSize', JSON.stringify(25)); + + const {result} = renderHook( + () => + useQueryStateWithLocalStorage({ + key: 'pageSize', + namespace: 'testNamespace', + defaultValue: 10, + parser: parseAsInteger, + }), + { + wrapper: withNuqsTestingAdapter({ + searchParams: {pageSize: '50'}, + }), + } + ); + + expect(result.current[0]).toBe(50); + }); }); diff --git a/static/app/utils/url/useQueryStateWithLocalStorage.tsx b/static/app/utils/url/useQueryStateWithLocalStorage.tsx index 6727f0853eb1bd..62a36e691916f0 100644 --- a/static/app/utils/url/useQueryStateWithLocalStorage.tsx +++ b/static/app/utils/url/useQueryStateWithLocalStorage.tsx @@ -1,5 +1,5 @@ import {useEffect} from 'react'; -import {parseAsString, useQueryState} from 'nuqs'; +import {parseAsString, useQueryState, type Parser} from 'nuqs'; import {useLocalStorageState} from 'sentry/utils/useLocalStorageState'; @@ -8,29 +8,55 @@ import {useLocalStorageState} from 'sentry/utils/useLocalStorageState'; * URL takes precedence over localStorage, enabling URL sharing while * maintaining user preferences across sessions. * + * Supports any data type via Nuqs parsers (parseAsString, parseAsInteger, + * parseAsBoolean, parseAsArrayOf, etc.). The parser handles URL serialization + * while localStorage uses JSON for storage. + * * @param props - Configuration object * @param props.key - The URL query parameter name * @param props.namespace - The localStorage namespace (key will be `namespace:key`) * @param props.defaultValue - The default value if neither source has a value + * @param props.parser - Nuqs parser for the value type (defaults to parseAsString) * @returns Tuple of [effectiveValue, setValue] * * @example + * // String values (parser optional, defaults to parseAsString) * const [sortBy, setSortBy] = useQueryStateWithLocalStorage({ * key: 'sortBy', * namespace: 'dashboards', * defaultValue: 'date', * }); + * + * @example + * // Integer values + * const [pageSize, setPageSize] = useQueryStateWithLocalStorage({ + * key: 'pageSize', + * namespace: 'dashboards', + * defaultValue: 50, + * parser: parseAsInteger, + * }); + * + * @example + * // Boolean values + * const [enabled, setEnabled] = useQueryStateWithLocalStorage({ + * key: 'enabled', + * namespace: 'dashboards', + * defaultValue: false, + * parser: parseAsBoolean, + * }); */ -export function useQueryStateWithLocalStorage({ +export function useQueryStateWithLocalStorage({ key, namespace, defaultValue, + parser = parseAsString as Parser, }: { defaultValue: T; key: string; namespace: string; + parser?: Parser; }): [T, (value: T) => void] { - const [urlValue, setUrlValue] = useQueryState(key, parseAsString); + const [urlValue, setUrlValue] = useQueryState(key, parser); const localStorageKey = `${namespace}:${key}`; @@ -39,11 +65,11 @@ export function useQueryStateWithLocalStorage({ defaultValue ); - const effectiveValue = (urlValue ?? localStorageValue ?? defaultValue) as T; + const effectiveValue = urlValue ?? localStorageValue ?? defaultValue; useEffect(() => { - if (urlValue && urlValue !== localStorageValue) { - setLocalStorageValue(urlValue as T); + if (urlValue !== null && urlValue !== undefined && urlValue !== localStorageValue) { + setLocalStorageValue(urlValue); } }, [urlValue, localStorageValue, setLocalStorageValue]); From 6271136c504b6619d974f3f578f8936ecbc6a05e Mon Sep 17 00:00:00 2001 From: George Gritsouk <989898+gggritso@users.noreply.github.com> Date: Wed, 21 Jan 2026 11:44:48 -0500 Subject: [PATCH 4/6] refactor(useQueryStateWithLocalStorage): Use parser.eq for equality comparison Use parser's equality check method when available for more accurate comparison of complex values like arrays and objects. Falls back to strict equality when parser.eq is not defined. Also adds Knip exclusion since this hook is intentionally only used in test files. --- knip.config.ts | 2 + .../useQueryStateWithLocalStorage.spec.tsx | 161 ++++++++++-------- .../url/useQueryStateWithLocalStorage.tsx | 136 ++++++++------- 3 files changed, 168 insertions(+), 131 deletions(-) diff --git a/knip.config.ts b/knip.config.ts index de578287056e92..7222eda184039a 100644 --- a/knip.config.ts +++ b/knip.config.ts @@ -59,6 +59,8 @@ const config: KnipConfig = { '!static/**/{fixtures,__fixtures__}/**!', // helper files for tests - it's fine that they are only used in tests '!static/**/*{t,T}estUtils*.{js,mjs,ts,tsx}!', + // utility hook only used in tests (intentionally) + '!static/app/utils/url/useQueryStateWithLocalStorage.tsx!', // helper files for stories - it's fine that they are only used in tests '!static/app/**/__stories__/*.{js,mjs,ts,tsx}!', '!static/app/stories/**/*.{js,mjs,ts,tsx}!', diff --git a/static/app/utils/url/useQueryStateWithLocalStorage.spec.tsx b/static/app/utils/url/useQueryStateWithLocalStorage.spec.tsx index 5b8988f46c23f7..04507e1c4c1205 100644 --- a/static/app/utils/url/useQueryStateWithLocalStorage.spec.tsx +++ b/static/app/utils/url/useQueryStateWithLocalStorage.spec.tsx @@ -14,29 +14,31 @@ describe('useQueryStateWithLocalStorage', () => { it('returns default value when neither URL nor localStorage has value', () => { const {result} = renderHook( () => - useQueryStateWithLocalStorage({ - key: 'testParam', - namespace: 'testNamespace', - defaultValue: 'default', - }), + useQueryStateWithLocalStorage( + 'testParam', + 'testNamespace:testParam', + parseAsString, + 'fallback' + ), { wrapper: withNuqsTestingAdapter(), } ); - expect(result.current[0]).toBe('default'); + expect(result.current[0]).toBe('fallback'); }); it('returns localStorage value when URL is empty', () => { - localStorageWrapper.setItem('testNamespace:testParam', JSON.stringify('fromStorage')); + localStorageWrapper.setItem('testNamespace:testParam', 'fromStorage'); const {result} = renderHook( () => - useQueryStateWithLocalStorage({ - key: 'testParam', - namespace: 'testNamespace', - defaultValue: 'default', - }), + useQueryStateWithLocalStorage( + 'testParam', + 'testNamespace:testParam', + parseAsString, + 'notUsed' + ), { wrapper: withNuqsTestingAdapter(), } @@ -46,16 +48,16 @@ describe('useQueryStateWithLocalStorage', () => { }); it('returns URL value when URL has value (URL takes precedence)', () => { - localStorageWrapper.setItem('testNamespace:testParam', JSON.stringify('fromStorage')); + localStorageWrapper.setItem('testNamespace:testParam', 'fromStorage'); const {result} = renderHook( () => - useQueryStateWithLocalStorage({ - key: 'testParam', - namespace: 'testNamespace', - defaultValue: 'default', - parser: parseAsString, - }), + useQueryStateWithLocalStorage( + 'testParam', + 'testNamespace:testParam', + parseAsString, + 'unused' + ), { wrapper: withNuqsTestingAdapter({ searchParams: {testParam: 'fromURL'}, @@ -69,12 +71,12 @@ describe('useQueryStateWithLocalStorage', () => { it('syncs localStorage when URL changes', async () => { renderHook( () => - useQueryStateWithLocalStorage({ - key: 'testParam', - namespace: 'testNamespace', - defaultValue: 'default', - parser: parseAsString, - }), + useQueryStateWithLocalStorage( + 'testParam', + 'testNamespace:testParam', + parseAsString, + 'initial' + ), { wrapper: withNuqsTestingAdapter({ searchParams: {testParam: 'newURLValue'}, @@ -84,7 +86,7 @@ describe('useQueryStateWithLocalStorage', () => { await waitFor(() => { const storedValue = localStorageWrapper.getItem('testNamespace:testParam'); - expect(storedValue).toBe(JSON.stringify('newURLValue')); + expect(storedValue).toBe('newURLValue'); }); }); @@ -93,12 +95,12 @@ describe('useQueryStateWithLocalStorage', () => { const {result} = renderHook( () => - useQueryStateWithLocalStorage({ - key: 'testParam', - namespace: 'testNamespace', - defaultValue: 'default', - parser: parseAsString, - }), + useQueryStateWithLocalStorage( + 'testParam', + 'testNamespace:testParam', + parseAsString, + 'starting' + ), { wrapper: withNuqsTestingAdapter({onUrlUpdate}), } @@ -118,24 +120,18 @@ describe('useQueryStateWithLocalStorage', () => { await waitFor(() => { const storedValue = localStorageWrapper.getItem('testNamespace:testParam'); - expect(storedValue).toBe(JSON.stringify('newValue')); + expect(storedValue).toBe('newValue'); }); }); it('works with enum-like string types', async () => { - type SortOption = 'date' | 'name' | 'size'; - - localStorageWrapper.setItem('myNamespace:sort', JSON.stringify('name')); + localStorageWrapper.setItem('myNamespace:sort', 'name'); const onUrlUpdate = jest.fn(); const {result} = renderHook( () => - useQueryStateWithLocalStorage({ - key: 'sort', - namespace: 'myNamespace', - defaultValue: 'date', - }), + useQueryStateWithLocalStorage('sort', 'myNamespace:sort', parseAsString, 'date'), { wrapper: withNuqsTestingAdapter({onUrlUpdate}), } @@ -157,21 +153,21 @@ describe('useQueryStateWithLocalStorage', () => { await waitFor(() => { const storedValue = localStorageWrapper.getItem('myNamespace:sort'); - expect(storedValue).toBe(JSON.stringify('size')); + expect(storedValue).toBe('size'); }); }); it('does not sync localStorage if URL value matches localStorage', () => { - localStorageWrapper.setItem('testNamespace:testParam', JSON.stringify('sameValue')); + localStorageWrapper.setItem('testNamespace:testParam', 'sameValue'); const {result} = renderHook( () => - useQueryStateWithLocalStorage({ - key: 'testParam', - namespace: 'testNamespace', - defaultValue: 'default', - parser: parseAsString, - }), + useQueryStateWithLocalStorage( + 'testParam', + 'testNamespace:testParam', + parseAsString, + 'baseline' + ), { wrapper: withNuqsTestingAdapter({ searchParams: {testParam: 'sameValue'}, @@ -182,22 +178,22 @@ describe('useQueryStateWithLocalStorage', () => { expect(result.current[0]).toBe('sameValue'); const storedValue = localStorageWrapper.getItem('testNamespace:testParam'); - expect(storedValue).toBe(JSON.stringify('sameValue')); + expect(storedValue).toBe('sameValue'); }); it('works with integer values using parseAsInteger', async () => { - localStorageWrapper.setItem('testNamespace:count', JSON.stringify(42)); + localStorageWrapper.setItem('testNamespace:count', '42'); const onUrlUpdate = jest.fn(); const {result} = renderHook( () => - useQueryStateWithLocalStorage({ - key: 'count', - namespace: 'testNamespace', - defaultValue: 0, - parser: parseAsInteger, - }), + useQueryStateWithLocalStorage( + 'count', + 'testNamespace:count', + parseAsInteger, + 999 + ), { wrapper: withNuqsTestingAdapter({onUrlUpdate}), } @@ -220,23 +216,23 @@ describe('useQueryStateWithLocalStorage', () => { await waitFor(() => { const storedValue = localStorageWrapper.getItem('testNamespace:count'); - expect(storedValue).toBe(JSON.stringify(100)); + expect(storedValue).toBe('100'); }); }); it('works with boolean values using parseAsBoolean', async () => { - localStorageWrapper.setItem('testNamespace:enabled', JSON.stringify(true)); + localStorageWrapper.setItem('testNamespace:enabled', 'true'); const onUrlUpdate = jest.fn(); const {result} = renderHook( () => - useQueryStateWithLocalStorage({ - key: 'enabled', - namespace: 'testNamespace', - defaultValue: false, - parser: parseAsBoolean, - }), + useQueryStateWithLocalStorage( + 'enabled', + 'testNamespace:enabled', + parseAsBoolean, + false + ), { wrapper: withNuqsTestingAdapter({onUrlUpdate}), } @@ -259,21 +255,21 @@ describe('useQueryStateWithLocalStorage', () => { await waitFor(() => { const storedValue = localStorageWrapper.getItem('testNamespace:enabled'); - expect(storedValue).toBe(JSON.stringify(false)); + expect(storedValue).toBe('false'); }); }); it('URL integer value overrides localStorage', () => { - localStorageWrapper.setItem('testNamespace:pageSize', JSON.stringify(25)); + localStorageWrapper.setItem('testNamespace:pageSize', '25'); const {result} = renderHook( () => - useQueryStateWithLocalStorage({ - key: 'pageSize', - namespace: 'testNamespace', - defaultValue: 10, - parser: parseAsInteger, - }), + useQueryStateWithLocalStorage( + 'pageSize', + 'testNamespace:pageSize', + parseAsInteger, + 5 + ), { wrapper: withNuqsTestingAdapter({ searchParams: {pageSize: '50'}, @@ -283,4 +279,23 @@ describe('useQueryStateWithLocalStorage', () => { expect(result.current[0]).toBe(50); }); + + it('throws error when parser has .withDefault() configured', () => { + expect(() => { + renderHook( + () => + useQueryStateWithLocalStorage( + 'testParam', + 'testNamespace:testParam', + parseAsString.withDefault('shouldThrow'), + 'ignored' + ), + { + wrapper: withNuqsTestingAdapter(), + } + ); + }).toThrow( + 'useQueryStateWithLocalStorage: parser should not have .withDefault() configured' + ); + }); }); diff --git a/static/app/utils/url/useQueryStateWithLocalStorage.tsx b/static/app/utils/url/useQueryStateWithLocalStorage.tsx index 62a36e691916f0..fe51280fb6a46e 100644 --- a/static/app/utils/url/useQueryStateWithLocalStorage.tsx +++ b/static/app/utils/url/useQueryStateWithLocalStorage.tsx @@ -1,7 +1,8 @@ -import {useEffect} from 'react'; -import {parseAsString, useQueryState, type Parser} from 'nuqs'; +import {useCallback, useEffect} from 'react'; +import {useQueryState, type SingleParserBuilder} from 'nuqs'; -import {useLocalStorageState} from 'sentry/utils/useLocalStorageState'; +import {defined} from 'sentry/utils'; +import localStorageWrapper from 'sentry/utils/localStorage'; /** * Hook that syncs state between URL query parameters and localStorage. @@ -9,74 +10,93 @@ import {useLocalStorageState} from 'sentry/utils/useLocalStorageState'; * maintaining user preferences across sessions. * * Supports any data type via Nuqs parsers (parseAsString, parseAsInteger, - * parseAsBoolean, parseAsArrayOf, etc.). The parser handles URL serialization - * while localStorage uses JSON for storage. + * parseAsBoolean, parseAsArrayOf, etc.). The parser handles both URL and + * localStorage serialization/deserialization. * - * @param props - Configuration object - * @param props.key - The URL query parameter name - * @param props.namespace - The localStorage namespace (key will be `namespace:key`) - * @param props.defaultValue - The default value if neither source has a value - * @param props.parser - Nuqs parser for the value type (defaults to parseAsString) - * @returns Tuple of [effectiveValue, setValue] - * - * @example - * // String values (parser optional, defaults to parseAsString) - * const [sortBy, setSortBy] = useQueryStateWithLocalStorage({ - * key: 'sortBy', - * namespace: 'dashboards', - * defaultValue: 'date', - * }); + * **Important**: Pass the parser WITHOUT `.withDefault()` - use the separate + * `defaultValue` parameter instead. This ensures localStorage values are + * properly prioritized over defaults. * - * @example - * // Integer values - * const [pageSize, setPageSize] = useQueryStateWithLocalStorage({ - * key: 'pageSize', - * namespace: 'dashboards', - * defaultValue: 50, - * parser: parseAsInteger, - * }); + * @param queryKey - The URL query parameter name + * @param localStorageKey - The localStorage key + * @param parser - Nuqs parser WITHOUT .withDefault(). Pass the base parser (parseAsString, parseAsInteger, etc.) + * @param defaultValue - The default value when neither URL nor localStorage has a value + * @returns Tuple of [effectiveValue, setValue] * * @example - * // Boolean values - * const [enabled, setEnabled] = useQueryStateWithLocalStorage({ - * key: 'enabled', - * namespace: 'dashboards', - * defaultValue: false, - * parser: parseAsBoolean, - * }); + * // String values with default + * const [sortBy, setSortBy] = useQueryStateWithLocalStorage( + * 'sortBy', + * 'dashboards:sortBy', + * parseAsString, + * 'date' + * ); */ -export function useQueryStateWithLocalStorage({ - key, - namespace, - defaultValue, - parser = parseAsString as Parser, -}: { - defaultValue: T; - key: string; - namespace: string; - parser?: Parser; -}): [T, (value: T) => void] { - const [urlValue, setUrlValue] = useQueryState(key, parser); +export function useQueryStateWithLocalStorage( + queryKey: string, + localStorageKey: string, + parser: SingleParserBuilder, + defaultValue: T +): [T, (value: T & {}) => void] { + // Detect if parser has a default value configured (runtime check) + // Parsers with .withDefault() have a 'defaultValue' property + // + // If the parser has `.withDefault()`, it will _always_ return that default + // instead of `null` when there's no URL param. This breaks our priority order + // (URL > localStorage > default) because we can't distinguish between: + // 1. No URL param exists (should fall back to localStorage) + // 2. URL param explicitly set to the default value (should use that) + // Both would return the same value, making localStorage never take effect. + if ('defaultValue' in parser && defined(parser.defaultValue)) { + throw new Error( + `useQueryStateWithLocalStorage: parser should not have .withDefault() configured. ` + + `Pass the base parser and use the separate defaultValue parameter instead.` + ); + } - const localStorageKey = `${namespace}:${key}`; + // The authoritative state is from the URL parameter + const [urlValue, setUrlValue] = useQueryState(queryKey, parser); - const [localStorageValue, setLocalStorageValue] = useLocalStorageState( - localStorageKey, - defaultValue - ); + // The fallback state is read from `localStorage` directly. We are not using + // `useLocalStorageState` because we want to do the deserialization ourselves + // according to Nuqs configuration. + const stored = localStorageWrapper.getItem(localStorageKey); + const localStorageValue = stored ? parser.parse(stored) : null; const effectiveValue = urlValue ?? localStorageValue ?? defaultValue; + // Synchronize the URL state and the `localStorage` state by writing the URL + // state into `localStorage`. This can happen on hook initialization, if the + // URL query state has been updated by the user by some means other than + // `setValue`. useEffect(() => { - if (urlValue !== null && urlValue !== undefined && urlValue !== localStorageValue) { - setLocalStorageValue(urlValue); + if (defined(urlValue)) { + // Use parser's equality check if available (for arrays, objects, etc.) + // Otherwise fall back to strict equality + const areEqual = + defined(localStorageValue) && 'eq' in parser && typeof parser.eq === 'function' + ? parser.eq(urlValue, localStorageValue) + : urlValue === localStorageValue; + + if (!areEqual) { + const serializedUrlValue = parser.serialize(urlValue); + localStorageWrapper.setItem(localStorageKey, serializedUrlValue); + } } - }, [urlValue, localStorageValue, setLocalStorageValue]); + }, [parser, urlValue, localStorageValue, localStorageKey]); - const setValue = (value: T) => { - setUrlValue(value); - setLocalStorageValue(value); - }; + const setValue = useCallback( + (value: T & {}) => { + // `setURLValue` is async in Nuqs. Maybe it's because of throttling. + // Normally we might want to `await` the Promise from `setUrlValue` but it'd + // be bad for us to have the Nuqs state and the `localStorage` state be out + // of sync because the `useEffect` above might re-run. + setUrlValue(value); + const serializedValue = parser.serialize(value); + localStorageWrapper.setItem(localStorageKey, serializedValue); + }, + [setUrlValue, parser, localStorageKey] + ); return [effectiveValue, setValue]; } From 39674c22418b484eecb2ad99ad0cc5491d19c93e Mon Sep 17 00:00:00 2001 From: George Gritsouk <989898+gggritso@users.noreply.github.com> Date: Wed, 21 Jan 2026 13:19:42 -0500 Subject: [PATCH 5/6] fix(useQueryStateWithLocalStorage): Handle empty string values correctly Fix bug where empty strings in localStorage were incorrectly treated as missing values due to truthy check. Use defined() helper to properly distinguish between null (key doesn't exist) and empty string (key exists with empty value). Also add tests to verify empty string handling from both URL and localStorage sources. --- .../useQueryStateWithLocalStorage.spec.tsx | 46 +++++++++++++++++++ .../url/useQueryStateWithLocalStorage.tsx | 2 +- 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/static/app/utils/url/useQueryStateWithLocalStorage.spec.tsx b/static/app/utils/url/useQueryStateWithLocalStorage.spec.tsx index 04507e1c4c1205..dad7e889906a4c 100644 --- a/static/app/utils/url/useQueryStateWithLocalStorage.spec.tsx +++ b/static/app/utils/url/useQueryStateWithLocalStorage.spec.tsx @@ -298,4 +298,50 @@ describe('useQueryStateWithLocalStorage', () => { 'useQueryStateWithLocalStorage: parser should not have .withDefault() configured' ); }); + + it('handles empty string values correctly', () => { + // Set empty string in localStorage + localStorageWrapper.setItem('testNamespace:testParam', ''); + + const {result} = renderHook( + () => + useQueryStateWithLocalStorage( + 'testParam', + 'testNamespace:testParam', + parseAsString, + 'defaultValue' + ), + { + wrapper: withNuqsTestingAdapter(), + } + ); + + // Empty string from localStorage should be returned, not the default + expect(result.current[0]).toBe(''); + }); + + it('syncs empty string URL values to localStorage', async () => { + const {result} = renderHook( + () => + useQueryStateWithLocalStorage( + 'testParam', + 'testNamespace:testParam', + parseAsString, + 'defaultValue' + ), + { + wrapper: withNuqsTestingAdapter({ + searchParams: {testParam: ''}, + }), + } + ); + + expect(result.current[0]).toBe(''); + + // Empty string from URL should be synced to localStorage + await waitFor(() => { + const storedValue = localStorageWrapper.getItem('testNamespace:testParam'); + expect(storedValue).toBe(''); + }); + }); }); diff --git a/static/app/utils/url/useQueryStateWithLocalStorage.tsx b/static/app/utils/url/useQueryStateWithLocalStorage.tsx index fe51280fb6a46e..bacb44142d391c 100644 --- a/static/app/utils/url/useQueryStateWithLocalStorage.tsx +++ b/static/app/utils/url/useQueryStateWithLocalStorage.tsx @@ -61,7 +61,7 @@ export function useQueryStateWithLocalStorage( // `useLocalStorageState` because we want to do the deserialization ourselves // according to Nuqs configuration. const stored = localStorageWrapper.getItem(localStorageKey); - const localStorageValue = stored ? parser.parse(stored) : null; + const localStorageValue = defined(stored) ? parser.parse(stored) : null; const effectiveValue = urlValue ?? localStorageValue ?? defaultValue; From 4de1f3a808cc4ecd935bf2fd229166b6004a3186 Mon Sep 17 00:00:00 2001 From: George Gritsouk <989898+gggritso@users.noreply.github.com> Date: Wed, 21 Jan 2026 16:38:03 -0500 Subject: [PATCH 6/6] ref(useQueryStateWithLocalStorage): Address PR feedback and improve performance Address four review comments from TkDodo on PR #106625: 1. Array Support: Extended type signature to accept GenericParserBuilder, supporting both SingleParserBuilder (parseAsArrayOf) and MultiParserBuilder. Multi parsers store data as JSON arrays in localStorage. 2. Type-Level Safety: Updated type signature to use GenericParserBuilder with runtime validation to prevent parsers with .withDefault() configured. 3. Performance: Refactored to read localStorage once on mount instead of every render. URL becomes the single source of truth after initialization, with effective value chain simplified to: urlValue ?? defaultValue. 4. Null Behavior: Setting setValue(null) now correctly clears both URL and localStorage, falling back to defaultValue instead of stale localStorage. Added comprehensive test coverage for array support (both SingleParser and MultiParser), null behavior, and initialization logic. Co-Authored-By: Claude --- .../useQueryStateWithLocalStorage.spec.tsx | 197 +++++++++++++++++- .../url/useQueryStateWithLocalStorage.tsx | 169 +++++++++++---- 2 files changed, 320 insertions(+), 46 deletions(-) diff --git a/static/app/utils/url/useQueryStateWithLocalStorage.spec.tsx b/static/app/utils/url/useQueryStateWithLocalStorage.spec.tsx index dad7e889906a4c..b64808215ee0c0 100644 --- a/static/app/utils/url/useQueryStateWithLocalStorage.spec.tsx +++ b/static/app/utils/url/useQueryStateWithLocalStorage.spec.tsx @@ -1,4 +1,10 @@ -import {parseAsBoolean, parseAsInteger, parseAsString} from 'nuqs'; +import { + createMultiParser, + parseAsArrayOf, + parseAsBoolean, + parseAsInteger, + parseAsString, +} from 'nuqs'; import {withNuqsTestingAdapter} from 'nuqs/adapters/testing'; import {act, renderHook, waitFor} from 'sentry-test/reactTestingLibrary'; @@ -344,4 +350,193 @@ describe('useQueryStateWithLocalStorage', () => { expect(storedValue).toBe(''); }); }); + + it('works with array values using parseAsArrayOf (SingleParser)', async () => { + localStorageWrapper.setItem('testNamespace:tags', 'foo,bar,baz'); + + const onUrlUpdate = jest.fn(); + + const {result} = renderHook( + () => + useQueryStateWithLocalStorage( + 'tags', + 'testNamespace:tags', + parseAsArrayOf(parseAsString), + [] + ), + { + wrapper: withNuqsTestingAdapter({onUrlUpdate}), + } + ); + + // Should populate URL from localStorage on mount + await waitFor(() => { + expect(onUrlUpdate).toHaveBeenCalledWith( + expect.objectContaining({ + queryString: expect.stringContaining('tags=foo,bar,baz'), + }) + ); + }); + + // Value should be parsed as array + expect(result.current[0]).toEqual(['foo', 'bar', 'baz']); + + act(() => { + result.current[1](['new', 'tags']); + }); + + await waitFor(() => { + expect(onUrlUpdate).toHaveBeenCalledWith( + expect.objectContaining({ + queryString: expect.stringContaining('tags=new,tags'), + }) + ); + }); + + await waitFor(() => { + const storedValue = localStorageWrapper.getItem('testNamespace:tags'); + expect(storedValue).toBe('new,tags'); + }); + }); + + it('works with native array values using MultiParser', async () => { + // Create a custom MultiParser without .withDefault() + // This mimics parseAsNativeArrayOf but without the automatic default + const parseAsStringArray = createMultiParser({ + parse: values => { + const filtered = values.filter(v => v !== null); + return filtered.length > 0 ? filtered : null; + }, + serialize: values => values, + }); + + // Multi parser stores as JSON array in localStorage + localStorageWrapper.setItem('testNamespace:tags', '["foo","bar","baz"]'); + + const onUrlUpdate = jest.fn(); + + const {result} = renderHook( + () => + useQueryStateWithLocalStorage( + 'tags', + 'testNamespace:tags', + parseAsStringArray, + [] + ), + { + wrapper: withNuqsTestingAdapter({onUrlUpdate}), + } + ); + + // Should populate URL from localStorage on mount + await waitFor(() => { + // Native array format uses repeated keys: ?tags=foo&tags=bar&tags=baz + expect(onUrlUpdate).toHaveBeenCalledWith( + expect.objectContaining({ + queryString: expect.stringMatching(/tags=foo.*tags=bar.*tags=baz/), + }) + ); + }); + + // Value should be parsed as array + expect(result.current[0]).toEqual(['foo', 'bar', 'baz']); + + act(() => { + result.current[1](['new', 'tags']); + }); + + await waitFor(() => { + // Native array format: ?tags=new&tags=tags + expect(onUrlUpdate).toHaveBeenCalledWith( + expect.objectContaining({ + queryString: expect.stringMatching(/tags=new.*tags=tags/), + }) + ); + }); + + await waitFor(() => { + const storedValue = localStorageWrapper.getItem('testNamespace:tags'); + // Should be stored as JSON array + expect(JSON.parse(storedValue!)).toEqual(['new', 'tags']); + }); + }); + + it('setting value to null clears both URL and localStorage', async () => { + localStorageWrapper.setItem('testNamespace:param', 'fromStorage'); + + const onUrlUpdate = jest.fn(); + + const {result} = renderHook( + () => + useQueryStateWithLocalStorage( + 'param', + 'testNamespace:param', + parseAsString, + 'defaultValue' + ), + { + wrapper: withNuqsTestingAdapter({ + searchParams: {param: 'fromURL'}, + onUrlUpdate, + }), + } + ); + + expect(result.current[0]).toBe('fromURL'); + + // Set to null + act(() => { + result.current[1](null); + }); + + await waitFor(() => { + // Should fall back to default + expect(result.current[0]).toBe('defaultValue'); + }); + + // localStorage should be cleared + expect(localStorageWrapper.getItem('testNamespace:param')).toBeNull(); + + // URL should be cleared + await waitFor(() => { + expect(onUrlUpdate).toHaveBeenCalledWith( + expect.objectContaining({ + queryString: '', + }) + ); + }); + }); + + it('populates URL from localStorage on mount when URL is empty', async () => { + localStorageWrapper.setItem('testNamespace:param', 'fromStorage'); + + const onUrlUpdate = jest.fn(); + + const {result} = renderHook( + () => + useQueryStateWithLocalStorage( + 'param', + 'testNamespace:param', + parseAsString, + 'defaultValue' + ), + { + wrapper: withNuqsTestingAdapter({onUrlUpdate}), + } + ); + + // Initially shows localStorage value (as it gets written to URL) + await waitFor(() => { + expect(result.current[0]).toBe('fromStorage'); + }); + + // URL should be populated + await waitFor(() => { + expect(onUrlUpdate).toHaveBeenCalledWith( + expect.objectContaining({ + queryString: expect.stringContaining('param=fromStorage'), + }) + ); + }); + }); }); diff --git a/static/app/utils/url/useQueryStateWithLocalStorage.tsx b/static/app/utils/url/useQueryStateWithLocalStorage.tsx index bacb44142d391c..23bc529de58246 100644 --- a/static/app/utils/url/useQueryStateWithLocalStorage.tsx +++ b/static/app/utils/url/useQueryStateWithLocalStorage.tsx @@ -1,53 +1,112 @@ -import {useCallback, useEffect} from 'react'; -import {useQueryState, type SingleParserBuilder} from 'nuqs'; +import {useCallback, useEffect, useState} from 'react'; +import {useQueryState, type GenericParserBuilder, type MultiParserBuilder} from 'nuqs'; import {defined} from 'sentry/utils'; import localStorageWrapper from 'sentry/utils/localStorage'; +/** + * Type guard to detect if a parser is a MultiParserBuilder. + * Multi parsers require special handling for localStorage. + */ +function isMultiParser( + parser: GenericParserBuilder +): parser is MultiParserBuilder { + return (parser as MultiParserBuilder).type === 'multi'; +} + +/** + * Parse a value from localStorage based on parser type. + * - Single parsers: parse string directly + * - Multi parsers: parse JSON array first, then pass to parser + */ +function parseFromLocalStorage( + stored: string, + parser: GenericParserBuilder +): T | null { + if (isMultiParser(parser)) { + try { + const array = JSON.parse(stored); + if (Array.isArray(array)) { + return parser.parse(array); + } + return null; + } catch { + return null; + } + } + return parser.parse(stored); +} + +/** + * Serialize a value to localStorage based on parser type. + * - Single parsers: serialize returns string directly + * - Multi parsers: serialize returns Array, store as JSON + */ +function serializeToLocalStorage(value: T, parser: GenericParserBuilder): string { + if (isMultiParser(parser)) { + const serialized = parser.serialize(value); + return JSON.stringify(serialized); + } + return parser.serialize(value); +} + /** * Hook that syncs state between URL query parameters and localStorage. - * URL takes precedence over localStorage, enabling URL sharing while - * maintaining user preferences across sessions. + * After initial mount, URL becomes the single source of truth. On mount, if URL + * is empty and localStorage has a value, the URL is populated with that value. * - * Supports any data type via Nuqs parsers (parseAsString, parseAsInteger, - * parseAsBoolean, parseAsArrayOf, etc.). The parser handles both URL and - * localStorage serialization/deserialization. + * Supports both SingleParserBuilder (parseAsString, parseAsInteger, parseAsArrayOf) + * and MultiParserBuilder for maximum flexibility. * * **Important**: Pass the parser WITHOUT `.withDefault()` - use the separate - * `defaultValue` parameter instead. This ensures localStorage values are - * properly prioritized over defaults. + * `defaultValue` parameter instead. This is enforced at runtime. * * @param queryKey - The URL query parameter name * @param localStorageKey - The localStorage key - * @param parser - Nuqs parser WITHOUT .withDefault(). Pass the base parser (parseAsString, parseAsInteger, etc.) - * @param defaultValue - The default value when neither URL nor localStorage has a value - * @returns Tuple of [effectiveValue, setValue] + * @param parser - Nuqs parser WITHOUT .withDefault() + * @param defaultValue - The default value when URL has no value + * @returns Tuple of [value, setValue] where setValue accepts value or null to clear * * @example - * // String values with default + * // String values * const [sortBy, setSortBy] = useQueryStateWithLocalStorage( * 'sortBy', * 'dashboards:sortBy', * parseAsString, * 'date' * ); + * setSortBy('name'); // Set to 'name' + * setSortBy(null); // Clear and return to default 'date' + * + * @example + * // Array with parseAsArrayOf (SingleParser) + * const [tags, setTags] = useQueryStateWithLocalStorage( + * 'tags', + * 'filters:tags', + * parseAsArrayOf(parseAsString), + * [] + * ); */ export function useQueryStateWithLocalStorage( queryKey: string, localStorageKey: string, - parser: SingleParserBuilder, + parser: GenericParserBuilder, defaultValue: T -): [T, (value: T & {}) => void] { +): [T, (value: T | null) => void] { // Detect if parser has a default value configured (runtime check) // Parsers with .withDefault() have a 'defaultValue' property // // If the parser has `.withDefault()`, it will _always_ return that default - // instead of `null` when there's no URL param. This breaks our priority order - // (URL > localStorage > default) because we can't distinguish between: - // 1. No URL param exists (should fall back to localStorage) - // 2. URL param explicitly set to the default value (should use that) - // Both would return the same value, making localStorage never take effect. - if ('defaultValue' in parser && defined(parser.defaultValue)) { + // instead of `null` when there's no URL param. This breaks our priority + // because we need the parser to return null when there's no URL value. + // + // Note: MultiParsers may have internal defaultValue handling that's different, + // so we skip this check for them. + if ( + !isMultiParser(parser) && + 'defaultValue' in parser && + defined(parser.defaultValue) + ) { throw new Error( `useQueryStateWithLocalStorage: parser should not have .withDefault() configured. ` + `Pass the base parser and use the separate defaultValue parameter instead.` @@ -57,43 +116,63 @@ export function useQueryStateWithLocalStorage( // The authoritative state is from the URL parameter const [urlValue, setUrlValue] = useQueryState(queryKey, parser); - // The fallback state is read from `localStorage` directly. We are not using - // `useLocalStorageState` because we want to do the deserialization ourselves - // according to Nuqs configuration. - const stored = localStorageWrapper.getItem(localStorageKey); - const localStorageValue = defined(stored) ? parser.parse(stored) : null; + // Track whether we've initialized from localStorage + const [initialized, setInitialized] = useState(false); - const effectiveValue = urlValue ?? localStorageValue ?? defaultValue; + // On mount, read localStorage ONCE and populate URL if empty + // This ensures localStorage is only read on mount, not on every render + useEffect(() => { + if (!initialized && !defined(urlValue)) { + const stored = localStorageWrapper.getItem(localStorageKey); + if (defined(stored)) { + const parsed = parseFromLocalStorage(stored, parser); + if (defined(parsed)) { + // Cast to satisfy Nuqs's type signature (T & {} excludes null/undefined) + setUrlValue(parsed as T & {}); + } + } + setInitialized(true); + } + }, [initialized, urlValue, localStorageKey, parser, setUrlValue]); - // Synchronize the URL state and the `localStorage` state by writing the URL - // state into `localStorage`. This can happen on hook initialization, if the - // URL query state has been updated by the user by some means other than - // `setValue`. + // After initialization, URL is the single source of truth + const effectiveValue = urlValue ?? defaultValue; + + // Synchronize URL changes to localStorage + // This happens when URL is updated by external means (e.g., browser back/forward) useEffect(() => { if (defined(urlValue)) { + const storedRaw = localStorageWrapper.getItem(localStorageKey); + const storedValue = defined(storedRaw) + ? parseFromLocalStorage(storedRaw, parser) + : null; + // Use parser's equality check if available (for arrays, objects, etc.) - // Otherwise fall back to strict equality const areEqual = - defined(localStorageValue) && 'eq' in parser && typeof parser.eq === 'function' - ? parser.eq(urlValue, localStorageValue) - : urlValue === localStorageValue; + defined(storedValue) && 'eq' in parser && typeof parser.eq === 'function' + ? parser.eq(urlValue, storedValue) + : urlValue === storedValue; if (!areEqual) { - const serializedUrlValue = parser.serialize(urlValue); + const serializedUrlValue = serializeToLocalStorage(urlValue, parser); localStorageWrapper.setItem(localStorageKey, serializedUrlValue); } } - }, [parser, urlValue, localStorageValue, localStorageKey]); + }, [parser, urlValue, localStorageKey]); const setValue = useCallback( - (value: T & {}) => { - // `setURLValue` is async in Nuqs. Maybe it's because of throttling. - // Normally we might want to `await` the Promise from `setUrlValue` but it'd - // be bad for us to have the Nuqs state and the `localStorage` state be out - // of sync because the `useEffect` above might re-run. - setUrlValue(value); - const serializedValue = parser.serialize(value); - localStorageWrapper.setItem(localStorageKey, serializedValue); + (value: T | null) => { + if (value === null) { + // Clear both URL and localStorage + setUrlValue(null); + localStorageWrapper.removeItem(localStorageKey); + } else { + // Set value in both URL and localStorage + // Cast to satisfy Nuqs's type signature (T & {} excludes null/undefined) + setUrlValue(value as T & {}); + const serializedValue = serializeToLocalStorage(value, parser); + localStorageWrapper.setItem(localStorageKey, serializedValue); + } }, [setUrlValue, parser, localStorageKey] );