Fix popover not calling onClose on unmount (#71252)

jeryj · getdave · jeryj · web-flow · commit a1a6a97cea89 · 2025-09-03T14:10:01.000Z
* Run onClose when unmounting component There was a cancelBlurCheck running on unmount, which prevented the onClose for the popover from running when the popover was unmounted. * Add test coverage for hook * Improve useFocusOutside test with Promise.withResolvers() Replace fragile setTimeout approach with Promise.withResolvers() to make the test more reliable and faster. The test now waits for the actual callback to be called rather than an arbitrary timeout. Addresses feedback from PR #71252 review. * Fix Node.js compatibility: replace Promise.withResolvers() with manual deferred pattern Promise.withResolvers() is only available in Node.js 22+, but the project requires Node.js >=20.10.0. Use the traditional manual deferred promise pattern for better compatibility. This maintains the same improved test reliability while ensuring compatibility with the project's Node.js version requirements. --------- Co-authored-by: Dave Smith <getdavemail@gmail.com> Co-authored-by: jeryj <jeryj@git.wordpress.org> Co-authored-by: getdave <get_dave@git.wordpress.org> Co-authored-by: aduth <aduth@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: ciampo <mciampini@git.wordpress.org> Co-authored-by: stokesman <presstoke@git.wordpress.org>
diff --git a/packages/compose/src/hooks/use-focus-outside/index.ts b/packages/compose/src/hooks/use-focus-outside/index.ts
@@ -87,11 +87,6 @@ export default function useFocusOutside(
 		clearTimeout( blurCheckTimeoutIdRef.current );
 	}, [] );
 
-	// Cancel blur checks on unmount.
-	useEffect( () => {
-		return () => cancelBlurCheck();
-	}, [] );
-
 	// Cancel a blur check if the callback or ref is no longer provided.
 	useEffect( () => {
 		if ( ! onFocusOutside ) {
diff --git a/packages/compose/src/hooks/use-focus-outside/test/index.js b/packages/compose/src/hooks/use-focus-outside/test/index.js
@@ -116,8 +116,14 @@ describe( 'useFocusOutside', () => {
 		mockedDocumentHasFocus.mockRestore();
 	} );
 
-	it( 'should cancel check when unmounting while queued', async () => {
-		const mockOnFocusOutside = jest.fn();
+	it( 'should call handler when unmounting while queued', async () => {
+		let resolvePromise;
+		const promise = new Promise( ( resolve ) => {
+			resolvePromise = resolve;
+		} );
+		const mockOnFocusOutside = jest
+			.fn()
+			.mockImplementation( resolvePromise );
 		const user = userEvent.setup();
 
 		const { unmount } = render(
@@ -130,11 +136,19 @@ describe( 'useFocusOutside', () => {
 		} );
 		await user.click( button );
 
-		// Simulate a blur event and the wrapper unmounting while the blur event
-		// handler is queued
-		button.blur();
+		// Click outside the wrapper to trigger a blur event and queue the callback
+		const outsideButton = screen.getByRole( 'button', {
+			name: 'Button outside the wrapper',
+		} );
+		await user.click( outsideButton );
+
+		// Immediately unmount the component while the blur event is queued
+		// The callback should still be called.
 		unmount();
 
-		expect( mockOnFocusOutside ).not.toHaveBeenCalled();
+		// Wait for the callback to be called
+		await promise;
+
+		expect( mockOnFocusOutside ).toHaveBeenCalled();
 	} );
 } );
