Skip to content

Commit b5bd9af

Browse files
stefanonardoclaude
andcommitted
OCPBUGS-81519: Fix Search page state mutation and unnecessary component remounts
Fix Set reference copy bug in resource selection handlers, replace filter-embedded component keys with stable keys to prevent WebSocket watch teardown, and add memo/debounce to ResourceList to avoid expensive re-renders during typing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 46a9c16 commit b5bd9af

2 files changed

Lines changed: 88 additions & 37 deletions

File tree

frontend/packages/console-app/src/components/data-view/useConsoleDataViewFilters.ts

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useMemo } from 'react';
1+
import { useEffect, useMemo, useRef } from 'react';
22
import { useDataViewFilters } from '@patternfly/react-data-view';
33
import { useSearchParams } from 'react-router';
44
import { useExactSearch } from '@console/app/src/components/user-preferences/search/useExactSearch';
@@ -38,6 +38,39 @@ export const useConsoleDataViewFilters = <
3838
setSearchParams,
3939
});
4040

41+
// Sync URL search params → internal filter state.
42+
// useDataViewFilters only reads searchParams on mount (empty deps useEffect).
43+
// This effect ensures filters stay in sync when the URL changes externally
44+
// (e.g., the Search page updating query params without remounting).
45+
const filtersRef = useRef(filters);
46+
filtersRef.current = filters;
47+
useEffect(() => {
48+
const updates: Partial<TFilters> = {};
49+
let hasChanges = false;
50+
for (const key of Object.keys(filtersRef.current)) {
51+
const currentValue = filtersRef.current[key];
52+
if (Array.isArray(currentValue)) {
53+
const urlValues = searchParams.getAll(key);
54+
if (
55+
urlValues.length !== currentValue.length ||
56+
urlValues.some((v, i) => v !== currentValue[i])
57+
) {
58+
updates[key] = urlValues;
59+
hasChanges = true;
60+
}
61+
} else {
62+
const urlValue = searchParams.get(key) ?? '';
63+
if (urlValue !== currentValue) {
64+
updates[key] = urlValue;
65+
hasChanges = true;
66+
}
67+
}
68+
}
69+
if (hasChanges) {
70+
onSetFilters(updates as TFilters);
71+
}
72+
}, [searchParams, onSetFilters]);
73+
4174
const filteredData = useMemo(
4275
() =>
4376
data?.filter((resource) => {

frontend/public/components/search.tsx

Lines changed: 54 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import * as _ from 'lodash';
22
import type { FC, MouseEvent } from 'react';
3-
import { useState, useEffect } from 'react';
3+
import { useState, useEffect, useMemo, memo } from 'react';
44
import { DocumentTitle } from '@console/shared/src/components/document-title/DocumentTitle';
55
import { useDebounceCallback } from '@console/shared/src/hooks/useDebounceCallback';
66
import { useTranslation } from 'react-i18next';
@@ -51,37 +51,51 @@ import { useActivePerspective } from '@console/dynamic-plugin-sdk/src/perspectiv
5151
import { useActiveNamespace, useK8sModel } from '@console/dynamic-plugin-sdk/src/lib-core';
5252
import { ALL_NAMESPACES_KEY } from '@console/shared/src/constants';
5353

54-
const ResourceList = ({ kind, mock, namespace, selector, nameFilter }) => {
55-
const { plural } = useParams<{ plural?: string }>();
56-
const [kindObj] = useK8sModel(kind || plural);
57-
const resourceListPageExtensions = useExtensions<ResourceListPage>(isResourceListPage);
58-
if (!kindObj) {
59-
return <LoadingBox />;
60-
}
54+
const ResourceList = memo(
55+
({
56+
kind,
57+
mock,
58+
namespace,
59+
selector,
60+
nameFilter,
61+
}: {
62+
kind: string;
63+
mock: boolean;
64+
namespace: string;
65+
selector: any;
66+
nameFilter: string;
67+
}) => {
68+
const { plural } = useParams<{ plural?: string }>();
69+
const [kindObj] = useK8sModel(kind || plural);
70+
const resourceListPageExtensions = useExtensions<ResourceListPage>(isResourceListPage);
71+
if (!kindObj) {
72+
return <LoadingBox />;
73+
}
6174

62-
const componentLoader = getResourceListPages(resourceListPageExtensions).get(
63-
referenceForModel(kindObj),
64-
() => Promise.resolve(DefaultPage),
65-
);
66-
const ns = kindObj.namespaced ? namespace : undefined;
75+
const componentLoader = getResourceListPages(resourceListPageExtensions).get(
76+
referenceForModel(kindObj),
77+
() => Promise.resolve(DefaultPage),
78+
);
79+
const ns = kindObj.namespaced ? namespace : undefined;
6780

68-
return (
69-
<AsyncComponent
70-
loader={componentLoader}
71-
namespace={ns === ALL_NAMESPACES_KEY ? undefined : ns}
72-
selector={selector}
73-
nameFilter={nameFilter}
74-
kind={kindObj.crd ? referenceForModel(kindObj) : kindObj.kind}
75-
showTitle={false}
76-
hideTextFilter
77-
autoFocus={false}
78-
mock={mock}
79-
badge={getBadgeFromType(kindObj.badge)}
80-
hideNameLabelFilters
81-
hideColumnManagement
82-
/>
83-
);
84-
};
81+
return (
82+
<AsyncComponent
83+
loader={componentLoader}
84+
namespace={ns === ALL_NAMESPACES_KEY ? undefined : ns}
85+
selector={selector}
86+
nameFilter={nameFilter}
87+
kind={kindObj.crd ? referenceForModel(kindObj) : kindObj.kind}
88+
showTitle={false}
89+
hideTextFilter
90+
autoFocus={false}
91+
mock={mock}
92+
badge={getBadgeFromType(kindObj.badge)}
93+
hideNameLabelFilters
94+
hideColumnManagement
95+
/>
96+
);
97+
},
98+
);
8599

86100
const SearchPage_: FC<SearchProps> = (props) => {
87101
const { setQueryArgument, removeQueryArguments } = useQueryParamsMutator();
@@ -124,14 +138,15 @@ const SearchPage_: FC<SearchProps> = (props) => {
124138

125139
const debouncedNameFilterCallback = useDebounceCallback((nameFilter: string) => {
126140
setDebouncedNameFilter(nameFilter);
141+
setQueryArgument('name', nameFilter);
127142
}, 300);
128143

129144
useEffect(() => {
130145
debouncedNameFilterCallback(typeaheadNameFilter);
131146
}, [typeaheadNameFilter, debouncedNameFilterCallback]);
132147

133148
const updateSelectedItems = (selection: string) => {
134-
const updateItems = selectedItems;
149+
const updateItems = new Set(selectedItems);
135150
fireTelemetryEvent('search-resource-selected', {
136151
resource: selection,
137152
});
@@ -141,7 +156,7 @@ const SearchPage_: FC<SearchProps> = (props) => {
141156
};
142157

143158
const updateNewItems = (_filter: string, { key }: ToolbarLabel) => {
144-
const updateItems = selectedItems;
159+
const updateItems = new Set(selectedItems);
145160
updateItems.has(key) ? updateItems.delete(key) : updateItems.add(key);
146161
setSelectedItems(updateItems);
147162
setQueryArgument('kind', [...updateItems].join(','));
@@ -154,6 +169,7 @@ const SearchPage_: FC<SearchProps> = (props) => {
154169

155170
const clearNameFilter = () => {
156171
setTypeaheadNameFilter('');
172+
setDebouncedNameFilter('');
157173
setQueryArgument('name', '');
158174
};
159175

@@ -165,6 +181,7 @@ const SearchPage_: FC<SearchProps> = (props) => {
165181
const clearAll = () => {
166182
setSelectedItems(new Set([]));
167183
setTypeaheadNameFilter('');
184+
setDebouncedNameFilter('');
168185
setLabelFilter([]);
169186
removeQueryArguments('kind', 'name', 'q');
170187
};
@@ -188,7 +205,6 @@ const SearchPage_: FC<SearchProps> = (props) => {
188205

189206
const updateNameFilter = (value: string) => {
190207
setTypeaheadNameFilter(value);
191-
setQueryArgument('name', value);
192208
};
193209

194210
const updateLabelFilter = (value: string, endOfString: boolean) => {
@@ -239,6 +255,8 @@ const SearchPage_: FC<SearchProps> = (props) => {
239255
return model.labelKey ? t(model.labelKey) : model.label;
240256
};
241257

258+
const selector = useMemo(() => selectorFromString(labelFilter.join(',')), [labelFilter]);
259+
242260
return (
243261
<>
244262
<DocumentTitle>{t('public~Search')}</DocumentTitle>
@@ -338,11 +356,11 @@ const SearchPage_: FC<SearchProps> = (props) => {
338356
{!isCollapsed && (
339357
<ResourceList
340358
kind={resource}
341-
selector={selectorFromString(labelFilter.join(','))}
342-
nameFilter={typeaheadNameFilter}
359+
selector={selector}
360+
nameFilter={debouncedNameFilter}
343361
namespace={namespace}
344362
mock={noProjectsAvailable}
345-
key={`${resource}-${labelFilter.join(',')}-${debouncedNameFilter}`}
363+
key={resource}
346364
/>
347365
)}
348366
</AccordionContent>

0 commit comments

Comments
 (0)