[Synthetics UI] Fix infinite scroll in overview page (#146570)

## Summary

Closes #145378.

The overview page has a [`<EuiSpacer />` at the bottom of the
grid](e8d77b3f0f/x-pack/plugins/synthetics/public/apps/synthetics/components/monitors_page/overview/overview/overview_grid.tsx (L135-L137))
to track the user's scroll position. This is done [with an
`IntersectionObserver` (wrapped in a hook) and a
`ref`](e8d77b3f0f/x-pack/plugins/synthetics/public/apps/synthetics/components/monitors_page/overview/overview/overview_grid.tsx (L70-L75)).

Sometimes the `ref` got destroyed, and with it the
`IntersectionObserver` instance. This causes a race condition [in the
code that checks for the
intersection](e8d77b3f0f/x-pack/plugins/synthetics/public/apps/synthetics/components/monitors_page/overview/overview/overview_grid.tsx (L78-L88))
to determine if the next page should be loaded or not.

<img width="1278" alt="Screenshot 2022-11-29 at 16 35 42"
src="https://user-images.githubusercontent.com/57448/204574461-5056391c-e96d-4b38-9c9e-4976e0d14a40.png">


The reason why the `ref` got destroyed was [this early
return](e8d77b3f0f/x-pack/plugins/synthetics/public/apps/synthetics/components/monitors_page/overview/overview/overview_grid.tsx (L98-L100)).
With the code in `main` there is a brief moment in which the condition
holds `true`, right after the monitors are loaded. In that brief instant
the `status` is present but `monitorsSortedByStatus` is empty.

<img width="1084" alt="Screenshot 2022-11-29 at 16 44 35"
src="https://user-images.githubusercontent.com/57448/204575679-de4bc6bf-122b-4c6d-ae75-9c96a7c5fb85.png">

When this happens the `ref` is destroyed, because the underlying element
used to track the intersection is destroyed due of the early return.

This was caused because `monitorsSortedByStatus` was updated
asynchronously [with
`useEffect`](2f3313371b/x-pack/plugins/synthetics/public/apps/synthetics/hooks/use_monitors_sorted_by_status.tsx (L30-L72))
as part of the component lifecycle.

This PR uses `useMemo` instead of `useEffect`, to update the
`monitorsSortedByStatus` at the same time that `status` is present. This
prevents the early return from ever firing in the normal load cycle and
keeps the `ref`. Since the `ref` never gets destroyed there's no race
condition anymore.
This commit is contained in:
Alejandro Fernández Gómez 2022-11-30 09:23:34 +01:00 committed by GitHub
parent 1c076ee8fc
commit 217a04078d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 35 additions and 38 deletions

View file

@ -132,9 +132,9 @@ export const OverviewGrid = memo(() => {
) : (
<OverviewLoader />
)}
<span ref={intersectionRef}>
<div ref={intersectionRef}>
<EuiSpacer size="l" />
</span>
</div>
<EuiFlexGroup justifyContent="spaceBetween" alignItems="center">
{currentMonitors.length === monitors.length && (
<EuiFlexItem grow={false}>

View file

@ -5,8 +5,7 @@
* 2.0.
*/
import { useEffect, useMemo, useState, useRef } from 'react';
import { isEqual } from 'lodash';
import { useMemo, useRef } from 'react';
import { useSelector } from 'react-redux';
import { MonitorOverviewItem } from '../../../../common/runtime_types';
import { selectOverviewState } from '../state/overview';
@ -20,17 +19,19 @@ export function useMonitorsSortedByStatus() {
data: { monitors },
status,
} = useSelector(selectOverviewState);
const [monitorsSortedByStatus, setMonitorsSortedByStatus] = useState<
Record<string, MonitorOverviewItem[]>
>({ up: [], down: [], disabled: [] });
const downMonitors = useRef<Record<string, string[]> | null>(null);
const currentMonitors = useRef<MonitorOverviewItem[] | null>(monitors);
const locationNames = useLocationNames();
useEffect(() => {
const monitorsSortedByStatus = useMemo(() => {
if (!status) {
return;
return {
down: [],
up: [],
disabled: [],
};
}
const { downConfigs } = status;
const downMonitorMap: Record<string, string[]> = {};
downConfigs.forEach(({ location, configId }) => {
@ -41,34 +42,30 @@ export function useMonitorsSortedByStatus() {
}
});
if (
!isEqual(downMonitorMap, downMonitors.current) ||
!isEqual(monitors, currentMonitors.current)
) {
const orderedDownMonitors: MonitorOverviewItem[] = [];
const orderedUpMonitors: MonitorOverviewItem[] = [];
const orderedDisabledMonitors: MonitorOverviewItem[] = [];
monitors.forEach((monitor) => {
const monitorLocation = locationNames[monitor.location.id];
if (!monitor.isEnabled) {
orderedDisabledMonitors.push(monitor);
} else if (
Object.keys(downMonitorMap).includes(monitor.id) &&
downMonitorMap[monitor.id].includes(monitorLocation)
) {
orderedDownMonitors.push(monitor);
} else {
orderedUpMonitors.push(monitor);
}
});
downMonitors.current = downMonitorMap;
currentMonitors.current = monitors;
setMonitorsSortedByStatus({
down: orderedDownMonitors,
up: orderedUpMonitors,
disabled: orderedDisabledMonitors,
});
}
const orderedDownMonitors: MonitorOverviewItem[] = [];
const orderedUpMonitors: MonitorOverviewItem[] = [];
const orderedDisabledMonitors: MonitorOverviewItem[] = [];
monitors.forEach((monitor) => {
const monitorLocation = locationNames[monitor.location.id];
if (!monitor.isEnabled) {
orderedDisabledMonitors.push(monitor);
} else if (
monitor.id in downMonitorMap &&
downMonitorMap[monitor.id].includes(monitorLocation)
) {
orderedDownMonitors.push(monitor);
} else {
orderedUpMonitors.push(monitor);
}
});
downMonitors.current = downMonitorMap;
return {
down: orderedDownMonitors,
up: orderedUpMonitors,
disabled: orderedDisabledMonitors,
};
}, [monitors, locationNames, downMonitors, status]);
return useMemo(() => {