Skip to content

Commit e41a749

Browse files
committed
Avoid re-rendering of Dashboards when settings haven't changed
The list pages were re-rendering the dashboards over and over again because the identity of the dashboard settings changes with every render and therefore caused a re-render loop.
1 parent e99eb37 commit e41a749

2 files changed

Lines changed: 147 additions & 16 deletions

File tree

src/web/components/dashboard/Dashboard.tsx

Lines changed: 45 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* SPDX-License-Identifier: AGPL-3.0-or-later
44
*/
55

6-
import {useCallback, useEffect, useMemo} from 'react';
6+
import {useCallback, useEffect, useMemo, useRef} from 'react';
77
import {type ThunkDispatch} from '@reduxjs/toolkit';
88
import memoize from 'memoize-one';
99
import {connect} from 'react-redux';
@@ -129,11 +129,51 @@ export const Dashboard = ({
129129
const [_] = useTranslation();
130130
const callLoadSettings = useLatestCallback(loadSettings);
131131
const callSetDefaultSettings = useLatestCallback(setDefaultSettings);
132+
const permittedDisplaysSignature = JSON.stringify(permittedDisplays ?? []);
133+
const defaultDisplaysSignature = JSON.stringify(defaultDisplays ?? []);
134+
135+
const stablePermittedDisplaysRef = useRef(permittedDisplays ?? []);
136+
const permittedDisplaysSignatureRef = useRef(permittedDisplaysSignature);
137+
const defaultDashboardSettingsRef = useRef(
138+
convertDefaultDisplays(defaultDisplays),
139+
);
140+
const defaultDisplaysSignatureRef = useRef(defaultDisplaysSignature);
141+
142+
// Update the stable permitted displays if the signature has changed
143+
if (permittedDisplaysSignatureRef.current !== permittedDisplaysSignature) {
144+
stablePermittedDisplaysRef.current = permittedDisplays ?? [];
145+
permittedDisplaysSignatureRef.current = permittedDisplaysSignature;
146+
}
147+
148+
// Update the default dashboard settings if the signature has changed
149+
if (defaultDisplaysSignatureRef.current !== defaultDisplaysSignature) {
150+
defaultDashboardSettingsRef.current =
151+
convertDefaultDisplays(defaultDisplays);
152+
defaultDisplaysSignatureRef.current = defaultDisplaysSignature;
153+
}
154+
155+
const defaultDashboardSettings = defaultDashboardSettingsRef.current;
156+
const stablePermittedDisplays = stablePermittedDisplaysRef.current;
157+
158+
const defaults = useMemo(
159+
() => ({
160+
...defaultDashboardSettings,
161+
permittedDisplays: stablePermittedDisplays,
162+
maxItemsPerRow,
163+
maxRows,
164+
}),
165+
[
166+
defaultDashboardSettings,
167+
maxItemsPerRow,
168+
maxRows,
169+
stablePermittedDisplays,
170+
],
171+
);
132172

133173
const components = useMemo(() => {
134174
const mappedComponents: Record<string, DisplayComponent> = {};
135175

136-
(permittedDisplays ?? []).forEach((displayId: string) => {
176+
stablePermittedDisplays.forEach((displayId: string) => {
137177
const display = getDisplay(displayId);
138178

139179
if (isDefined(display)) {
@@ -144,7 +184,7 @@ export const Dashboard = ({
144184
});
145185

146186
return mappedComponents;
147-
}, [permittedDisplays]);
187+
}, [stablePermittedDisplays]);
148188

149189
const getDisplaysByIdMemoized = useMemo(
150190
() => memoize((rows: DashboardRow[] = []) => getDisplaysById(rows)),
@@ -158,24 +198,14 @@ export const Dashboard = ({
158198
);
159199

160200
useEffect(() => {
161-
const defaultDashboardSettings = convertDefaultDisplays(defaultDisplays);
162-
const defaults: DashboardSettings = {
163-
...defaultDashboardSettings,
164-
permittedDisplays,
165-
maxItemsPerRow,
166-
maxRows,
167-
};
168-
169201
callSetDefaultSettings(id, defaultDashboardSettings);
170202
callLoadSettings(id, defaults);
171203
}, [
172204
callLoadSettings,
173205
callSetDefaultSettings,
174-
defaultDisplays,
206+
defaultDashboardSettings,
207+
defaults,
175208
id,
176-
maxItemsPerRow,
177-
maxRows,
178-
permittedDisplays,
179209
]);
180210

181211
const rows = getRowsFromSettings();

src/web/components/dashboard/__tests__/Dashboard.test.tsx

Lines changed: 102 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,9 @@
33
* SPDX-License-Identifier: AGPL-3.0-or-later
44
*/
55

6+
import {useState} from 'react';
67
import {beforeEach, describe, expect, test, testing} from '@gsa/testing';
7-
import {rendererWith, screen, waitFor} from 'web/testing';
8+
import {rendererWith, screen, waitFor, fireEvent} from 'web/testing';
89
import {vi} from 'vitest';
910
import Dashboard, {
1011
DEFAULT_MAX_ITEMS_PER_ROW,
@@ -181,4 +182,104 @@ describe('Dashboard', () => {
181182

182183
expect(globals.__dashboardSortableGridCalls).toEqual([]);
183184
});
185+
186+
test('should not reload settings when display props keep same content across rerenders', async () => {
187+
const {dashboardId, defaultDisplays, gmp, permittedDisplays} =
188+
createDashboardState();
189+
190+
const {render} = rendererWith({gmp, store: true});
191+
192+
const StablePropsTestComponent = () => {
193+
// create new arrays on rerender to simulate prop changes,
194+
// but keep the content the same
195+
const [rerendered, setRerendered] = useState(false);
196+
const currentDefaultDisplays = rerendered
197+
? [...defaultDisplays.map(row => [...row])]
198+
: defaultDisplays;
199+
const currentPermittedDisplays = rerendered
200+
? [...permittedDisplays]
201+
: permittedDisplays;
202+
203+
return (
204+
<>
205+
<button
206+
data-testid="rerender-dashboard"
207+
type="button"
208+
onClick={() => setRerendered(true)}
209+
/>
210+
<Dashboard
211+
defaultDisplays={currentDefaultDisplays}
212+
id={dashboardId}
213+
permittedDisplays={currentPermittedDisplays}
214+
/>
215+
</>
216+
);
217+
};
218+
219+
render(<StablePropsTestComponent />);
220+
221+
await waitFor(() => {
222+
expect(gmp.dashboard.getSetting).toHaveBeenCalledTimes(1);
223+
});
224+
225+
fireEvent.click(screen.getByTestId('rerender-dashboard'));
226+
227+
await waitFor(() => {
228+
expect(gmp.dashboard.getSetting).toHaveBeenCalledTimes(1);
229+
});
230+
});
231+
232+
test('should reload settings when display props content changes', async () => {
233+
const {
234+
dashboardId,
235+
defaultDisplayId,
236+
defaultDisplays,
237+
gmp,
238+
permittedDisplays,
239+
} = createDashboardState();
240+
const extraDisplayId = `extra-${testCounter}`;
241+
242+
registerDisplay(createDisplayComponent(extraDisplayId), 'Extra display');
243+
244+
const {render} = rendererWith({gmp, store: true});
245+
246+
const ChangedPropsTestComponent = () => {
247+
// create new arrays on rerender to simulate prop changes,
248+
// and change the content of the arrays on rerender
249+
const [rerendered, setRerendered] = useState(false);
250+
const currentDefaultDisplays = rerendered
251+
? [[defaultDisplayId, extraDisplayId]]
252+
: defaultDisplays;
253+
const currentPermittedDisplays = rerendered
254+
? [...permittedDisplays, extraDisplayId]
255+
: permittedDisplays;
256+
257+
return (
258+
<>
259+
<button
260+
data-testid="rerender-with-changes"
261+
type="button"
262+
onClick={() => setRerendered(true)}
263+
/>
264+
<Dashboard
265+
defaultDisplays={currentDefaultDisplays}
266+
id={dashboardId}
267+
permittedDisplays={currentPermittedDisplays}
268+
/>
269+
</>
270+
);
271+
};
272+
273+
render(<ChangedPropsTestComponent />);
274+
275+
await waitFor(() => {
276+
expect(gmp.dashboard.getSetting).toHaveBeenCalledTimes(1);
277+
});
278+
279+
fireEvent.click(screen.getByTestId('rerender-with-changes'));
280+
281+
await waitFor(() => {
282+
expect(gmp.dashboard.getSetting).toHaveBeenCalledTimes(2);
283+
});
284+
});
184285
});

0 commit comments

Comments
 (0)