Skip to content

Commit 9784246

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 9784246

2 files changed

Lines changed: 89 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: 55 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();
@@ -120,18 +134,20 @@ const SearchPage_: FC<SearchProps> = (props) => {
120134
const validTags = _.reject(tags, (tag) => requirementFromString(tag) === undefined);
121135
setLabelFilter(validTags);
122136
setTypeaheadNameFilter(name || '');
137+
setDebouncedNameFilter(name || '');
123138
}, [location.search]);
124139

125140
const debouncedNameFilterCallback = useDebounceCallback((nameFilter: string) => {
126141
setDebouncedNameFilter(nameFilter);
142+
setQueryArgument('name', nameFilter);
127143
}, 300);
128144

129145
useEffect(() => {
130146
debouncedNameFilterCallback(typeaheadNameFilter);
131147
}, [typeaheadNameFilter, debouncedNameFilterCallback]);
132148

133149
const updateSelectedItems = (selection: string) => {
134-
const updateItems = selectedItems;
150+
const updateItems = new Set(selectedItems);
135151
fireTelemetryEvent('search-resource-selected', {
136152
resource: selection,
137153
});
@@ -141,7 +157,7 @@ const SearchPage_: FC<SearchProps> = (props) => {
141157
};
142158

143159
const updateNewItems = (_filter: string, { key }: ToolbarLabel) => {
144-
const updateItems = selectedItems;
160+
const updateItems = new Set(selectedItems);
145161
updateItems.has(key) ? updateItems.delete(key) : updateItems.add(key);
146162
setSelectedItems(updateItems);
147163
setQueryArgument('kind', [...updateItems].join(','));
@@ -154,6 +170,7 @@ const SearchPage_: FC<SearchProps> = (props) => {
154170

155171
const clearNameFilter = () => {
156172
setTypeaheadNameFilter('');
173+
setDebouncedNameFilter('');
157174
setQueryArgument('name', '');
158175
};
159176

@@ -165,6 +182,7 @@ const SearchPage_: FC<SearchProps> = (props) => {
165182
const clearAll = () => {
166183
setSelectedItems(new Set([]));
167184
setTypeaheadNameFilter('');
185+
setDebouncedNameFilter('');
168186
setLabelFilter([]);
169187
removeQueryArguments('kind', 'name', 'q');
170188
};
@@ -188,7 +206,6 @@ const SearchPage_: FC<SearchProps> = (props) => {
188206

189207
const updateNameFilter = (value: string) => {
190208
setTypeaheadNameFilter(value);
191-
setQueryArgument('name', value);
192209
};
193210

194211
const updateLabelFilter = (value: string, endOfString: boolean) => {
@@ -239,6 +256,8 @@ const SearchPage_: FC<SearchProps> = (props) => {
239256
return model.labelKey ? t(model.labelKey) : model.label;
240257
};
241258

259+
const selector = useMemo(() => selectorFromString(labelFilter.join(',')), [labelFilter]);
260+
242261
return (
243262
<>
244263
<DocumentTitle>{t('public~Search')}</DocumentTitle>
@@ -338,11 +357,11 @@ const SearchPage_: FC<SearchProps> = (props) => {
338357
{!isCollapsed && (
339358
<ResourceList
340359
kind={resource}
341-
selector={selectorFromString(labelFilter.join(','))}
342-
nameFilter={typeaheadNameFilter}
360+
selector={selector}
361+
nameFilter={debouncedNameFilter}
343362
namespace={namespace}
344363
mock={noProjectsAvailable}
345-
key={`${resource}-${labelFilter.join(',')}-${debouncedNameFilter}`}
364+
key={resource}
346365
/>
347366
)}
348367
</AccordionContent>

0 commit comments

Comments
 (0)