From a02d0a8117082754389eb5f6a547b2c2f886b6d5 Mon Sep 17 00:00:00 2001 From: Ilyas Foo Date: Thu, 13 Apr 2023 23:16:45 +0800 Subject: [PATCH] Fix invalid return callback ref warning (#37655) * Fix invalid return in callback ref and add tests * Changelog --- .../fix-37654-invalid-return-callback-ref | 4 +++ .../src/inbox-note/test/inbox-note.tsx | 2 ++ .../test/use-callback-on-link-click.tsx | 13 +++++++++ .../inbox-note/use-callback-on-link-click.ts | 28 +++++++++++-------- 4 files changed, 35 insertions(+), 12 deletions(-) create mode 100644 packages/js/experimental/changelog/fix-37654-invalid-return-callback-ref diff --git a/packages/js/experimental/changelog/fix-37654-invalid-return-callback-ref b/packages/js/experimental/changelog/fix-37654-invalid-return-callback-ref new file mode 100644 index 00000000000..d14e18f1448 --- /dev/null +++ b/packages/js/experimental/changelog/fix-37654-invalid-return-callback-ref @@ -0,0 +1,4 @@ +Significance: patch +Type: fix + +Fix invalid return callback ref warning diff --git a/packages/js/experimental/src/inbox-note/test/inbox-note.tsx b/packages/js/experimental/src/inbox-note/test/inbox-note.tsx index f1078577e04..bbe03171804 100644 --- a/packages/js/experimental/src/inbox-note/test/inbox-note.tsx +++ b/packages/js/experimental/src/inbox-note/test/inbox-note.tsx @@ -23,6 +23,8 @@ jest.mock( 'react-visibility-sensor', () => } ) ); +window.open = jest.fn(); + describe( 'InboxNoteCard', () => { const note = { id: 1, diff --git a/packages/js/experimental/src/inbox-note/test/use-callback-on-link-click.tsx b/packages/js/experimental/src/inbox-note/test/use-callback-on-link-click.tsx index a7c4d334565..2efcb221b98 100644 --- a/packages/js/experimental/src/inbox-note/test/use-callback-on-link-click.tsx +++ b/packages/js/experimental/src/inbox-note/test/use-callback-on-link-click.tsx @@ -42,4 +42,17 @@ describe( 'useCallbackOnLinkClick hook', () => { userEvent.click( getByText( 'Button' ) ); expect( callback ).not.toHaveBeenCalled(); } ); + + it( 'should remove listener on unmount', () => { + const listener = jest.fn(); + const { getByText, unmount } = render( + + ); + const span = getByText( 'Some Text' ); + jest.spyOn( span, 'removeEventListener' ).mockImplementation( + listener + ); + unmount(); + expect( listener ).toHaveBeenCalledTimes( 1 ); + } ); } ); diff --git a/packages/js/experimental/src/inbox-note/use-callback-on-link-click.ts b/packages/js/experimental/src/inbox-note/use-callback-on-link-click.ts index e7243aff958..537c08e0467 100644 --- a/packages/js/experimental/src/inbox-note/use-callback-on-link-click.ts +++ b/packages/js/experimental/src/inbox-note/use-callback-on-link-click.ts @@ -1,7 +1,7 @@ /** * External dependencies */ -import { useCallback } from '@wordpress/element'; +import { useCallback, useEffect, useRef } from '@wordpress/element'; export function useCallbackOnLinkClick( onClick: ( link: string ) => void ) { const onNodeClick = useCallback( @@ -20,17 +20,21 @@ export function useCallbackOnLinkClick( onClick: ( link: string ) => void ) { [ onClick ] ); - return useCallback( - ( node: HTMLElement ) => { + const nodeRef = useRef< HTMLElement | null >( null ); + + useEffect( () => { + const node = nodeRef.current; + if ( node ) { + node.addEventListener( 'click', onNodeClick ); + } + return () => { if ( node ) { - node.addEventListener( 'click', onNodeClick ); + node.removeEventListener( 'click', onNodeClick ); } - return () => { - if ( node ) { - node.removeEventListener( 'click', onNodeClick ); - } - }; - }, - [ onNodeClick ] - ); + }; + }, [ onNodeClick ] ); + + return useCallback( ( node: HTMLElement ) => { + nodeRef.current = node; + }, [] ); }