mirror of
https://github.com/supabase/supabase.git
synced 2026-05-07 17:30:25 -04:00
2349f76e18
## I have read the [CONTRIBUTING.md](https://github.com/supabase/supabase/blob/master/CONTRIBUTING.md) file. YES ## What kind of change does this PR introduce? Bug fix. ## What is the current behavior? Advisor dismissals use `useLocalStorageQuery`. When advisor signals pruning ran, it sometimes invoked `setDismissedKeys` even when nothing needed to change (no-op updater returning the same array reference). Separately, `useLocalStorageQuery` would still persist + `invalidateQueries` even when the computed next value was reference-equal to the current cached value. When `useAdvisorSignals` is mounted in two places at once (`AdvisorSection` + `AdvisorPanel`), those redundant invalidations / subscriber churn could occasionally cascade into React’s “Maximum update depth exceeded” error (often surfaced via Radix `composeRefs` in stack traces). CI saw this as an unhandled error during `AdvisorSignals.integration.test.tsx`. ## What is the new behavior? - `useLocalStorageQuery` now **early-returns** when `Object.is(next, current)` so no-op updates don’t write localStorage or invalidate the query. - `useAdvisorSignals` pruning effect now **short-circuits** unless there is actually a stale banned-IP dismissal to remove. ## Additional context Follow-up from #44372 (advisor signal items for banned IPs). Tests run locally: - `pnpm --filter studio exec vitest run components/ui/AdvisorPanel/useAdvisorSignals.test.tsx components/ui/AdvisorPanel/AdvisorSignals.integration.test.tsx hooks/misc/__tests__/useLocalStorageQuery.test.ts` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Enhanced handling of dismissed security alerts by preventing unnecessary state updates for stale dismissals, significantly reducing overhead and improving overall application performance. * Optimized local storage operations to skip redundant writes to storage and prevent triggering unnecessary cache updates and query invalidations when stored data values remain unchanged from the previous operation. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
114 lines
3.8 KiB
TypeScript
114 lines
3.8 KiB
TypeScript
// Reference: https://usehooks.com/useLocalStorage/
|
|
|
|
import { useQuery, useQueryClient } from '@tanstack/react-query'
|
|
import { Dispatch, SetStateAction, useCallback, useMemo, useState } from 'react'
|
|
|
|
export function useLocalStorage<T>(key: string, initialValue: T) {
|
|
// State to store our value
|
|
// Pass initial state function to useState so logic is only executed once
|
|
const [storedValue, setStoredValue] = useState<T>(() => {
|
|
if (typeof window === 'undefined') {
|
|
return initialValue
|
|
}
|
|
|
|
try {
|
|
// Get from local storage by key
|
|
const item = window.localStorage.getItem(key)
|
|
// Parse stored json or if none return initialValue
|
|
return item ? JSON.parse(item) : initialValue
|
|
} catch (error) {
|
|
// If error also return initialValue
|
|
console.log(error)
|
|
return initialValue
|
|
}
|
|
})
|
|
|
|
// Return a wrapped version of useState's setter function that ...
|
|
// ... persists the new value to localStorage.
|
|
const setValue = useCallback(
|
|
(value: T | ((val: T) => T)) => {
|
|
try {
|
|
// Allow value to be a function so we have same API as useState
|
|
const valueToStore = value instanceof Function ? value(storedValue) : value
|
|
// Save state
|
|
setStoredValue(valueToStore)
|
|
// Save to local storage
|
|
if (typeof window !== 'undefined') {
|
|
window.localStorage.setItem(key, JSON.stringify(valueToStore))
|
|
}
|
|
} catch (error) {
|
|
// A more advanced implementation would handle the error case
|
|
console.log(error)
|
|
}
|
|
},
|
|
[key, storedValue]
|
|
)
|
|
|
|
return [storedValue, setValue] as const
|
|
}
|
|
|
|
/**
|
|
* Hook to load/store values from local storage with an API similar
|
|
* to `useState()`.
|
|
*
|
|
* Differs from `useLocalStorage()` in that it uses `react-query` to
|
|
* invalidate stale values across hooks with the same key.
|
|
*/
|
|
export function useLocalStorageQuery<T>(key: string, initialValue: T) {
|
|
const queryClient = useQueryClient()
|
|
const queryKey = useMemo(() => ['localStorage', key], [key])
|
|
|
|
const {
|
|
error,
|
|
data: storedValue = initialValue,
|
|
isSuccess,
|
|
isLoading,
|
|
isError,
|
|
} = useQuery({
|
|
queryKey,
|
|
queryFn: () => {
|
|
if (typeof window === 'undefined') {
|
|
return initialValue
|
|
}
|
|
|
|
const item = window.localStorage.getItem(key)
|
|
|
|
if (!item) {
|
|
return initialValue
|
|
}
|
|
|
|
return JSON.parse(item) as T
|
|
},
|
|
})
|
|
|
|
const setValue: Dispatch<SetStateAction<T>> = useCallback(
|
|
(value) => {
|
|
const currentValue = queryClient.getQueryData<T>(queryKey) ?? initialValue
|
|
const valueToStore = value instanceof Function ? value(currentValue) : value
|
|
|
|
// Bail out when the value is unchanged (matches useState semantics).
|
|
// Without this, no-op updates from consumers — like a pruning effect
|
|
// whose updater returns `current` unchanged — still write to
|
|
// localStorage and invalidate the query, which churns subscribers and
|
|
// can cascade into "Maximum update depth exceeded" when two consumers
|
|
// of the same key are mounted together.
|
|
if (Object.is(valueToStore, currentValue)) return
|
|
|
|
if (typeof window !== 'undefined') {
|
|
window.localStorage.setItem(key, JSON.stringify(valueToStore))
|
|
}
|
|
|
|
queryClient.setQueryData(queryKey, valueToStore)
|
|
queryClient.invalidateQueries({ queryKey })
|
|
},
|
|
// initialValue is intentionally excluded: the function body reads the
|
|
// current value via queryClient.getQueryData so it doesn't close over
|
|
// initialValue reactively — including it would cause a new function
|
|
// reference every render when callers pass an inline literal (e.g. []).
|
|
// eslint-disable-next-line react-hooks/exhaustive-deps
|
|
[key, queryKey, queryClient]
|
|
)
|
|
|
|
return [storedValue, setValue, { isSuccess, isLoading, isError, error }] as const
|
|
}
|