Skip to content

Commit

Permalink
[Resolver] Fix useSelector usage (#76129) (#76146)
Browse files Browse the repository at this point in the history
In some cases we have selectors returning thunks. The thunks need to be
called inside `useSelector` in order for a rerender to be reliably
triggered.

`useSelector` triggers a re-render if its return value changes. By calling the thunk inside of the selector passed to `useSelector`, we will trigger re-renders when needed.
  • Loading branch information
Robert Austin authored Aug 28, 2020
1 parent 22e872e commit ced19a6
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import {
import { CubeForProcess } from './cube_for_process';
import { ResolverEvent } from '../../../../common/endpoint/types';
import { useResolverTheme } from '../assets';
import { CrumbInfo } from '../../types';
import { CrumbInfo, ResolverState } from '../../types';

const StyledDescriptionList = styled(EuiDescriptionList)`
&.euiDescriptionList.euiDescriptionList--column dt.euiDescriptionList__title.desc-title {
Expand All @@ -52,7 +52,9 @@ export const ProcessDetails = memo(function ProcessDetails({
}) {
const processName = event.eventName(processEvent);
const entityId = event.entityId(processEvent);
const isProcessTerminated = useSelector(selectors.isProcessTerminated)(entityId);
const isProcessTerminated = useSelector((state: ResolverState) =>
selectors.isProcessTerminated(state)(entityId)
);
const processInfoEntry: EuiDescriptionListProps['listItems'] = useMemo(() => {
const eventTime = event.eventTimestamp(processEvent);
const dateTime = eventTime === undefined ? null : formatDate(eventTime);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { ResolverEvent } from '../../../../common/endpoint/types';
import * as selectors from '../../store/selectors';
import { useResolverDispatch } from '../use_resolver_dispatch';
import { PanelContentError } from './panel_content_error';
import { CrumbInfo } from '../../types';
import { CrumbInfo, ResolverState } from '../../types';

// Adding some styles to prevent horizontal scrollbars, per request from UX review
const StyledDescriptionList = memo(styled(EuiDescriptionList)`
Expand Down Expand Up @@ -154,9 +154,8 @@ export const RelatedEventDetail = memo(function RelatedEventDetail({
relatedEventCategory = naString,
sections,
formattedDate,
] = useSelector(selectors.relatedEventDisplayInfoByEntityAndSelfId)(
processEntityId,
relatedEventId
] = useSelector((state: ResolverState) =>
selectors.relatedEventDisplayInfoByEntityAndSelfId(state)(processEntityId, relatedEventId)
);

const waitCrumbs = useMemo(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { htmlIdGenerator, EuiButton, EuiI18nNumber, EuiFlexGroup, EuiFlexItem }
import { useSelector } from 'react-redux';
import { NodeSubMenu, subMenuAssets } from './submenu';
import { applyMatrix3 } from '../models/vector2';
import { Vector2, Matrix3 } from '../types';
import { Vector2, Matrix3, ResolverState } from '../types';
import { SymbolIds, useResolverTheme, calculateResolverFontSize } from './assets';
import { ResolverEvent, SafeResolverEvent } from '../../../common/endpoint/types';
import { useResolverDispatch } from './use_resolver_dispatch';
Expand Down Expand Up @@ -118,19 +118,23 @@ const UnstyledProcessEventDot = React.memo(
// NB: this component should be taking nodeID as a `string` instead of handling this logic here
throw new Error('Tried to render a node with no ID');
}
const relatedEventStats = useSelector(selectors.relatedEventsStats)(nodeID);
const relatedEventStats = useSelector((state: ResolverState) =>
selectors.relatedEventsStats(state)(nodeID)
);

// define a standard way of giving HTML IDs to nodes based on their entity_id/nodeID.
// this is used to link nodes via aria attributes
const nodeHTMLID = useCallback((id: string) => htmlIdGenerator(htmlIDPrefix)(`${id}:node`), [
htmlIDPrefix,
]);

const ariaLevel: number | null = useSelector(selectors.ariaLevel)(nodeID);
const ariaLevel: number | null = useSelector((state: ResolverState) =>
selectors.ariaLevel(state)(nodeID)
);

// the node ID to 'flowto'
const ariaFlowtoNodeID: string | null = useSelector(selectors.ariaFlowtoNodeID)(timeAtRender)(
nodeID
const ariaFlowtoNodeID: string | null = useSelector((state: ResolverState) =>
selectors.ariaFlowtoNodeID(state)(timeAtRender)(nodeID)
);

const isShowingEventActions = xScale > 0.8;
Expand Down Expand Up @@ -290,8 +294,8 @@ const UnstyledProcessEventDot = React.memo(
? subMenuAssets.initialMenuStatus
: relatedEventOptions;

const grandTotal: number | null = useSelector(selectors.relatedEventTotalForProcess)(
event as ResolverEvent
const grandTotal: number | null = useSelector((state: ResolverState) =>
selectors.relatedEventTotalForProcess(state)(event as ResolverEvent)
);

/* eslint-disable jsx-a11y/click-events-have-key-events */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { useStateSyncingActions } from './use_state_syncing_actions';
import { StyledMapContainer, StyledPanel, GraphContainer } from './styles';
import { entityIDSafeVersion } from '../../../common/endpoint/models/event';
import { SideEffectContext } from './side_effect_context';
import { ResolverProps } from '../types';
import { ResolverProps, ResolverState } from '../types';

/**
* The highest level connected Resolver component. Needs a `Provider` in its ancestry to work.
Expand All @@ -46,9 +46,12 @@ export const ResolverWithoutProviders = React.memo(
// use this for the entire render in order to keep things in sync
const timeAtRender = timestamp();

const { processNodePositions, connectingEdgeLineSegments } = useSelector(
selectors.visibleNodesAndEdgeLines
)(timeAtRender);
const {
processNodePositions,
connectingEdgeLineSegments,
} = useSelector((state: ResolverState) =>
selectors.visibleNodesAndEdgeLines(state)(timeAtRender)
);
const terminatedProcesses = useSelector(selectors.terminatedProcesses);
const { projectionMatrix, ref: cameraRef, onMouseDown } = useCamera();

Expand Down

0 comments on commit ced19a6

Please sign in to comment.