From fca1f5ee4f64a540e6462bd655b6902c57591ec7 Mon Sep 17 00:00:00 2001 From: Davis McPhee Date: Thu, 25 Jan 2024 18:00:07 -0400 Subject: [PATCH] [EuiResizableContainer] Fix issue where `onResizeEnd` becomes stale when renders occur between resize start and end (#7468) Co-authored-by: Cee Chen --- changelogs/upcoming/7468.md | 3 ++ .../resizable_container.test.tsx | 44 ++++++++++++++++++- .../resizable_container.tsx | 14 +++--- 3 files changed, 55 insertions(+), 6 deletions(-) create mode 100644 changelogs/upcoming/7468.md diff --git a/changelogs/upcoming/7468.md b/changelogs/upcoming/7468.md new file mode 100644 index 00000000000..6f41a0b2863 --- /dev/null +++ b/changelogs/upcoming/7468.md @@ -0,0 +1,3 @@ +**Bug fixes** + +- Fixed an issue in `EuiResizableContainer` where `onResizeEnd` could become a stale closure when renders occured between resize start and end, resulting in an outdated version of a consumer's `onResizeEnd` callback being called diff --git a/src/components/resizable_container/resizable_container.test.tsx b/src/components/resizable_container/resizable_container.test.tsx index c8d6edaf831..98a84e04416 100644 --- a/src/components/resizable_container/resizable_container.test.tsx +++ b/src/components/resizable_container/resizable_container.test.tsx @@ -6,8 +6,9 @@ * Side Public License, v 1. */ -import React from 'react'; +import React, { useState } from 'react'; import { act } from 'react-dom/test-utils'; +import { fireEvent } from '@testing-library/react'; import { mount } from 'enzyme'; import { findTestSubject, requiredProps } from '../../test'; @@ -348,5 +349,46 @@ describe('EuiResizableContainer', () => { button.simulate('blur'); expect(onResizeEnd).toHaveBeenCalledTimes(1); }); + + test('unmemoized consumer onResizeStart/End callbacks do not cause stale closures', () => { + const ConsumerUsage = () => { + const [rerender, setRerender] = useState(0); + // Unmemoized consumer callbacks + const onResizeStart = () => { + setRerender(rerender + 1); + }; + const onResizeEnd = () => { + setRerender(rerender + 1); + }; + + return ( + + {(EuiResizablePanel, EuiResizableButton) => ( + <> + Testing + + + {rerender} + + + )} + + ); + }; + const { getByTestSubject } = render(); + expect(getByTestSubject('rerenders')).toHaveTextContent('0'); + + fireEvent.mouseDown(getByTestSubject('euiResizableButton')); + expect(getByTestSubject('rerenders')).toHaveTextContent('1'); + + fireEvent.mouseUp(getByTestSubject('euiResizableButton')); + expect(getByTestSubject('rerenders')).toHaveTextContent('2'); + // Without `useLatest`, the rerender count doesn't correctly update due to `onResizeEnd` + // not being memoized and causing a stale `resizeEnd` closure to be called on event end + }); }); }); diff --git a/src/components/resizable_container/resizable_container.tsx b/src/components/resizable_container/resizable_container.tsx index c6e035f8d1a..ecf65c50c14 100644 --- a/src/components/resizable_container/resizable_container.tsx +++ b/src/components/resizable_container/resizable_container.tsx @@ -20,7 +20,7 @@ import React, { import classNames from 'classnames'; import { CommonProps } from '../common'; -import { keys } from '../../services'; +import { keys, useLatest } from '../../services'; import { useResizeObserver } from '../observer/resize_observer'; import { EuiResizableContainerContextProvider } from './context'; import { @@ -100,6 +100,10 @@ export const EuiResizableContainer: FunctionComponent< onResizeEnd, ...rest }) => { + // Note: It's important to memoize consumer callbacks to prevent our own functions + // from reinstantiating unnecessarily & causing window event listeners to call stale closures + const onResizeEndRef = useLatest(onResizeEnd); + const onResizeStartRef = useLatest(onResizeStart); const containerRef = useRef(null); const isHorizontal = direction === 'horizontal'; @@ -135,9 +139,9 @@ export const EuiResizableContainer: FunctionComponent< }>({}); const resizeEnd = useCallback(() => { - onResizeEnd?.(); + onResizeEndRef.current?.(); resizeContext.current = {}; - }, [onResizeEnd]); + }, [onResizeEndRef]); const resizeStart = useCallback( (trigger: ResizeTrigger, keyMoveDirection?: KeyMoveDirection) => { @@ -148,10 +152,10 @@ export const EuiResizableContainer: FunctionComponent< if (resizeContext.current.trigger) { resizeEnd(); } - onResizeStart?.(trigger); + onResizeStartRef.current?.(trigger); resizeContext.current = { trigger, keyMoveDirection }; }, - [onResizeStart, resizeEnd] + [onResizeStartRef, resizeEnd] ); const onMouseDown = useCallback(