From 9b9395ac336a5d299677380b460aa69cecf59c2e Mon Sep 17 00:00:00 2001 From: Marco Antonio Ghiani Date: Tue, 30 Jan 2024 12:48:36 +0100 Subject: [PATCH 1/4] fix(euiTextTruncatee): clean up timeout when component unmounts --- src/components/text_truncate/text_truncate.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/components/text_truncate/text_truncate.tsx b/src/components/text_truncate/text_truncate.tsx index b2b38355e40..4b4aa0c3f16 100644 --- a/src/components/text_truncate/text_truncate.tsx +++ b/src/components/text_truncate/text_truncate.tsx @@ -138,9 +138,14 @@ const EuiTextTruncateWithWidth: FunctionComponent< // If necessary, wait a tick on mount before truncating const [ready, setReady] = useState(!calculationDelayMs); useEffect(() => { + let timerId: NodeJS.Timeout; if (calculationDelayMs) { - setTimeout(() => setReady(true), calculationDelayMs); + timerId = setTimeout(() => setReady(true), calculationDelayMs); } + + return () => { + clearTimeout(timerId); + }; }, [calculationDelayMs]); // Handle exceptions where we need to override the passed props From 1ccb1076fe16f1a7727477ac2ab871536f14e330 Mon Sep 17 00:00:00 2001 From: Marco Antonio Ghiani Date: Tue, 30 Jan 2024 13:12:22 +0100 Subject: [PATCH 2/4] docs(euiTextTruncatee): add changelog entry --- changelogs/upcoming/7495.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelogs/upcoming/7495.md diff --git a/changelogs/upcoming/7495.md b/changelogs/upcoming/7495.md new file mode 100644 index 00000000000..edfe954e183 --- /dev/null +++ b/changelogs/upcoming/7495.md @@ -0,0 +1,3 @@ +**Bug fixes** + +- Fixed `EuiTextTruncate` component to clean up timer from side effect on unmount \ No newline at end of file From 0f3389c002612606543382ced3bedbd64c4b546f Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Tue, 30 Jan 2024 11:26:08 -0800 Subject: [PATCH 3/4] Add unit test to confirm clearTimeout is called --- .../text_truncate/text_truncate.test.tsx | 38 ++++++++++++++----- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/src/components/text_truncate/text_truncate.test.tsx b/src/components/text_truncate/text_truncate.test.tsx index 640ab4afdbe..024fe925f71 100644 --- a/src/components/text_truncate/text_truncate.test.tsx +++ b/src/components/text_truncate/text_truncate.test.tsx @@ -38,18 +38,38 @@ describe('EuiTextTruncate', () => { expect(container.firstChild).toMatchSnapshot(); }); - it('allows delaying truncation calculation by `calculationDelayMs`', () => { - jest.useFakeTimers(); + describe('calculationDelayMs', () => { + beforeAll(jest.useFakeTimers); + afterAll(jest.useRealTimers); - const { queryByTestSubject } = render( - - ); - expect(queryByTestSubject('truncatedText')).not.toBeInTheDocument(); + it('allows delaying truncation calculation by the specified duration', () => { + const { queryByTestSubject } = render( + + ); + expect(queryByTestSubject('truncatedText')).not.toBeInTheDocument(); - act(() => jest.advanceTimersByTime(50)); - expect(queryByTestSubject('truncatedText')).toBeInTheDocument(); + act(() => jest.advanceTimersByTime(50)); + expect(queryByTestSubject('truncatedText')).toBeInTheDocument(); + }); - jest.useRealTimers(); + it('sets and clears the timeout on update or unmount', () => { + const clearTimeoutSpy = jest.spyOn(window, 'clearTimeout'); + + const { unmount, rerender } = render( + + ); + expect(clearTimeoutSpy).toHaveBeenCalledTimes(0); + + rerender( + + ); + expect(clearTimeoutSpy).toHaveBeenCalledTimes(1); + expect(clearTimeoutSpy).toHaveBeenLastCalledWith(expect.any(Number)); + + unmount(); + expect(clearTimeoutSpy).toHaveBeenCalledTimes(2); + expect(clearTimeoutSpy).toHaveBeenLastCalledWith(expect.any(Number)); + }); }); describe('resize observer', () => { From 48c0e3e769312c679c83c91b30322ebd38d81322 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Tue, 30 Jan 2024 11:34:18 -0800 Subject: [PATCH 4/4] Make the returned clearTimeout conditional + add unit test for that as welll --- src/components/text_truncate/text_truncate.test.tsx | 13 +++++++++++++ src/components/text_truncate/text_truncate.tsx | 8 ++------ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/components/text_truncate/text_truncate.test.tsx b/src/components/text_truncate/text_truncate.test.tsx index 024fe925f71..52c6af87a61 100644 --- a/src/components/text_truncate/text_truncate.test.tsx +++ b/src/components/text_truncate/text_truncate.test.tsx @@ -70,6 +70,19 @@ describe('EuiTextTruncate', () => { expect(clearTimeoutSpy).toHaveBeenCalledTimes(2); expect(clearTimeoutSpy).toHaveBeenLastCalledWith(expect.any(Number)); }); + + it('does not set or clear a timeout if a duration is not passed', () => { + const setTimeoutSpy = jest.spyOn(window, 'setTimeout'); + const clearTimeoutSpy = jest.spyOn(window, 'clearTimeout'); + + const { unmount } = render( + + ); + + expect(setTimeoutSpy).not.toHaveBeenCalled(); + unmount(); + expect(clearTimeoutSpy).not.toHaveBeenCalled(); + }); }); describe('resize observer', () => { diff --git a/src/components/text_truncate/text_truncate.tsx b/src/components/text_truncate/text_truncate.tsx index 4b4aa0c3f16..30022a43a8d 100644 --- a/src/components/text_truncate/text_truncate.tsx +++ b/src/components/text_truncate/text_truncate.tsx @@ -138,14 +138,10 @@ const EuiTextTruncateWithWidth: FunctionComponent< // If necessary, wait a tick on mount before truncating const [ready, setReady] = useState(!calculationDelayMs); useEffect(() => { - let timerId: NodeJS.Timeout; if (calculationDelayMs) { - timerId = setTimeout(() => setReady(true), calculationDelayMs); + const timerId = setTimeout(() => setReady(true), calculationDelayMs); + return () => clearTimeout(timerId); } - - return () => { - clearTimeout(timerId); - }; }, [calculationDelayMs]); // Handle exceptions where we need to override the passed props