Skip to content

Commit 6fe0612

Browse files
committed
Fix ref instability by using useMergeRefs
Replace the inline ref callback with useMergeRefs from @wordpress/compose to prevent unnecessary ref detach/reattach cycles on every render. Add a regression test verifying forwarded callback refs remain stable across rerenders.
1 parent 796fe96 commit 6fe0612

2 files changed

Lines changed: 28 additions & 12 deletions

File tree

packages/components/src/navigable-container/container.tsx

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import type { ForwardedRef } from 'react';
77
* WordPress dependencies
88
*/
99
import { forwardRef, useRef, useEffect, useCallback } from '@wordpress/element';
10+
import { useMergeRefs } from '@wordpress/compose';
1011
import { focus } from '@wordpress/dom';
1112

1213
/**
@@ -153,19 +154,10 @@ function UnforwardedNavigableContainer(
153154
getFocusableContext,
154155
] );
155156

157+
const mergedRef = useMergeRefs( [ containerRef, ref ] );
158+
156159
return (
157-
<div
158-
ref={ ( node ) => {
159-
containerRef.current = node;
160-
161-
if ( typeof ref === 'function' ) {
162-
ref( node );
163-
} else if ( ref ) {
164-
ref.current = node;
165-
}
166-
} }
167-
{ ...restProps }
168-
>
160+
<div ref={ mergedRef } { ...restProps }>
169161
{ children }
170162
</div>
171163
);

packages/components/src/navigable-container/test/navigable-menu.tsx

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,30 @@ describe( 'NavigableMenu', () => {
215215
expect( externalWrapperOnKeyDownSpy ).toHaveBeenCalledTimes( 2 );
216216
} );
217217

218+
it( 'should keep forwarded callback refs stable across rerenders', () => {
219+
const refSpy = jest.fn();
220+
221+
const { rerender } = render(
222+
<NavigableMenu ref={ refSpy }>
223+
<button>Item 1</button>
224+
</NavigableMenu>
225+
);
226+
227+
expect( refSpy ).toHaveBeenCalledTimes( 1 );
228+
expect( refSpy ).toHaveBeenCalledWith( expect.any( HTMLElement ) );
229+
230+
rerender(
231+
<NavigableMenu ref={ refSpy }>
232+
<button>Item 1</button>
233+
</NavigableMenu>
234+
);
235+
236+
// With a stable merged ref (useMergeRefs), the callback ref should
237+
// not be called again on rerender. Previously, an inline ref callback
238+
// would cause React to detach (null) and reattach on every render.
239+
expect( refSpy ).toHaveBeenCalledTimes( 1 );
240+
} );
241+
218242
it( 'skips its internal logic when the tab key is pressed', async () => {
219243
const user = userEvent.setup();
220244

0 commit comments

Comments
 (0)