Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelogs/upcoming/7468.md
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 (
<EuiResizableContainer
{...requiredProps}
onResizeStart={onResizeStart}
onResizeEnd={onResizeEnd}
>
{(EuiResizablePanel, EuiResizableButton) => (
<>
<EuiResizablePanel initialSize={50}>Testing</EuiResizablePanel>
<EuiResizableButton data-test-subj="euiResizableButton" />
<EuiResizablePanel initialSize={50} data-test-subj="rerenders">
{rerender}
</EuiResizablePanel>
</>
)}
</EuiResizableContainer>
);
};
const { getByTestSubject } = render(<ConsumerUsage />);
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
});
});
});
14 changes: 9 additions & 5 deletions src/components/resizable_container/resizable_container.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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<HTMLDivElement>(null);
const isHorizontal = direction === 'horizontal';

Expand Down Expand Up @@ -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) => {
Expand All @@ -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(
Expand Down