Files
supabase/apps/studio/components/ui/AdvisorPanel/useAdvisorSignals.ts
Danny White 2349f76e18 fix(studio): guard no-op advisor dismissal localStorage updates (#45031)
## 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 -->
2026-04-20 18:41:32 +10:00

107 lines
3.6 KiB
TypeScript

import { useCallback, useEffect, useMemo } from 'react'
import type { AdvisorSignalItem } from './AdvisorPanel.types'
import { useBannedIPsQuery } from '@/data/banned-ips/banned-ips-query'
import type { IPData } from '@/data/banned-ips/banned-ips-query'
import { useLocalStorageQuery } from '@/hooks/misc/useLocalStorage'
const createDismissalStorageKey = (projectRef: string) => `advisor-signal-dismissals:${projectRef}`
const createBannedIPDismissalKey = (ip: string) => `signal:banned-ip:${ip}:v1`
const createBannedIPSignalItems = ({
projectRef,
bannedIPsData,
}: {
projectRef?: string
bannedIPsData?: IPData
}): AdvisorSignalItem[] => {
if (!projectRef) return []
const bannedIPs = bannedIPsData?.banned_ipv4_addresses ?? []
return bannedIPs.map((ip) => ({
id: createBannedIPDismissalKey(ip),
dismissalKey: createBannedIPDismissalKey(ip),
source: 'signal' as const,
type: 'banned-ip' as const,
severity: 'warning' as const,
tab: 'security' as const,
title: 'Banned IP address',
summary: `The IP address \`${ip}\` is temporarily blocked because of suspicious traffic or repeated failed password attempts.`,
description:
'This IP address is temporarily blocked because of suspicious traffic or repeated failed password attempts. If this block is expected, you can dismiss this signal or remove the ban.',
docsUrl: 'https://supabase.com/docs/reference/cli/supabase-network-bans',
actions: [
{
label: 'Edit network bans',
href: `/project/${projectRef}/database/settings#banned-ips`,
},
],
sourceData: { type: 'banned-ip' as const, ip },
}))
}
interface UseAdvisorSignalsOptions {
projectRef?: string
enabled?: boolean
}
export const useAdvisorSignals = ({ projectRef, enabled = true }: UseAdvisorSignalsOptions) => {
const { data, isPending, isError } = useBannedIPsQuery({ projectRef }, { enabled })
const storageKey = projectRef
? createDismissalStorageKey(projectRef)
: 'advisor-signal-dismissals:unknown-project'
const [dismissedKeys, setDismissedKeys] = useLocalStorageQuery<string[]>(storageKey, [])
const dismissedKeySet = useMemo(() => new Set(dismissedKeys), [dismissedKeys])
const dismissSignal = useCallback(
(dismissalKey: string) => {
setDismissedKeys((current) =>
current.includes(dismissalKey) ? current : [...current, dismissalKey]
)
},
[setDismissedKeys]
)
const signalItems = useMemo(
() => createBannedIPSignalItems({ projectRef, bannedIPsData: data }),
[projectRef, data]
)
// Prune stale dismissals when the active signal list changes (e.g. an IP was unbanned).
// Only call the setter when pruning would actually change something — otherwise we
// churn subscribers unnecessarily, which can cause feedback loops when this hook is
// mounted in more than one place (AdvisorSection + AdvisorPanel).
useEffect(() => {
if (!data) return
const hasStaleBannedIPDismissal = dismissedKeys.some(
(key) =>
key.startsWith('signal:banned-ip:') &&
!signalItems.some((item) => item.dismissalKey === key)
)
if (!hasStaleBannedIPDismissal) return
const activeKeys = new Set(signalItems.map((item) => item.dismissalKey))
setDismissedKeys((current) =>
current.filter((key) => (key.startsWith('signal:banned-ip:') ? activeKeys.has(key) : true))
)
}, [data, signalItems, dismissedKeys, setDismissedKeys])
const formattedData = useMemo(
() => signalItems.filter((item) => !dismissedKeySet.has(item.dismissalKey)),
[signalItems, dismissedKeySet]
)
return {
data: formattedData,
dismissSignal,
isPending,
isError,
}
}