fix(ui): stale filters applied after sort/page/time change on Request Logs (#25789)
The useEffect that re-fetches logs on sort/page/time changes:
useEffect(() => {
if (hasBackendFilters && accessToken) {
performSearch(filters, currentPage);
}
}, [sortBy, sortOrder, currentPage, startTime, endTime, isCustomDate]);
intentionally omits `filters` and `hasBackendFilters` from its dep array
to avoid double-fetches when a filter is applied. The side-effect is a
stale-closure bug: the effect captures `filters` and `hasBackendFilters`
from the render where its deps last changed, not from the render where
the user selected, e.g., a Key Alias.
Reproduce: set Key Alias → results appear correctly → change page or
sort → the effect fires with the OLD `filters` snapshot (no key_alias)
→ API request is sent without the filter → table shows unfiltered data.
Fix: store the latest `filters` and `hasBackendFilters` in refs that are
kept in sync on every render. The sort/page/time effect reads from the
refs instead of the closure so it always uses the current filter state
without altering the dep array.
Co-authored-by: Bytechoreographer <Bytechoreographer@users.noreply.github.com>
Co-authored-by: Claude Sonnet 4 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
4b2fd870ca
commit
c26e304abc
@ -71,6 +71,14 @@ export function useLogFilterLogic({
|
||||
const [filters, setFilters] = useState<LogFilterState>(defaultFilters);
|
||||
const [backendFilteredLogs, setBackendFilteredLogs] = useState<PaginatedResponse | null>(null);
|
||||
const lastSearchTimestamp = useRef(0);
|
||||
|
||||
// Refs that always hold the latest filters and hasBackendFilters values.
|
||||
// The sort/page/time effect below intentionally omits these from its dep array
|
||||
// to avoid double-fetches when a filter changes; reading from refs instead of
|
||||
// the closure prevents stale-closure bugs (e.g. the effect using a snapshot of
|
||||
// filters taken before the user selected Key Alias).
|
||||
const filtersRef = useRef(filters);
|
||||
const hasBackendFiltersRef = useRef(false);
|
||||
const performSearch = useCallback(
|
||||
async (filters: LogFilterState, page = 1) => {
|
||||
if (!accessToken) return;
|
||||
@ -152,18 +160,25 @@ export function useLogFilterLogic({
|
||||
[filters],
|
||||
);
|
||||
|
||||
// Keep refs in sync on every render so the sort/page/time effect always reads
|
||||
// the latest values without those values being in its dep array.
|
||||
useEffect(() => {
|
||||
filtersRef.current = filters;
|
||||
hasBackendFiltersRef.current = hasBackendFilters;
|
||||
}, [filters, hasBackendFilters]);
|
||||
|
||||
// Refetch when sort, page, or time range changes (backend filters use their own fetch, not the main query)
|
||||
useEffect(() => {
|
||||
if (hasBackendFilters && accessToken) {
|
||||
if (hasBackendFiltersRef.current && accessToken) {
|
||||
// Cancel any pending debounced search to prevent it from overwriting this page's results
|
||||
debouncedSearch.cancel();
|
||||
performSearch(filters, currentPage);
|
||||
performSearch(filtersRef.current, currentPage);
|
||||
}
|
||||
// Intentionally omitted from deps:
|
||||
// - `filters` / `debouncedSearch` / `performSearch`: filter changes are handled by
|
||||
// handleFilterChange → debouncedSearch; adding them here would double-fetch on filter apply.
|
||||
// - `hasBackendFilters` / `accessToken`: stable across sort/page/time changes; including them
|
||||
// would cause spurious re-runs when the filter state first becomes active.
|
||||
// filters / hasBackendFilters are read via refs — avoids stale-closure bugs
|
||||
// when sort/page/time changes after a filter (e.g. Key Alias) was set.
|
||||
// debouncedSearch / performSearch: filter changes go through handleFilterChange
|
||||
// → debouncedSearch; adding them here would cause double-fetches on filter apply.
|
||||
// accessToken: stable across sort/page/time changes.
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [sortBy, sortOrder, currentPage, startTime, endTime, isCustomDate]);
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user