From 7f5d25e23e82adef262493794010385f1955f0b6 Mon Sep 17 00:00:00 2001 From: Joseph Savona Date: Tue, 9 Apr 2024 13:43:49 -0700 Subject: [PATCH] Fix cloneElement using string ref w no owner (#28797) Fix for an issue introduced in #28473 where cloneElement() with a string ref fails due to lack of an owner. We should use the current owner in this case. --------- Co-authored-by: Rick Hanlon --- .../src/__tests__/ReactElementClone-test.js | 61 ++++++++++++++++++- packages/react/src/jsx/ReactJSXElement.js | 2 +- 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/packages/react/src/__tests__/ReactElementClone-test.js b/packages/react/src/__tests__/ReactElementClone-test.js index 11bb847518e41..a8cf751635149 100644 --- a/packages/react/src/__tests__/ReactElementClone-test.js +++ b/packages/react/src/__tests__/ReactElementClone-test.js @@ -274,8 +274,56 @@ describe('ReactElementClone', () => { const root = ReactDOMClient.createRoot(document.createElement('div')); await act(() => root.render()); - expect(component.childRef).toEqual({current: null}); - expect(component.parentRef.current.xyzRef.current.tagName).toBe('SPAN'); + if (gate(flags => flags.enableRefAsProp && flags.disableStringRefs)) { + expect(component.childRef).toEqual({current: null}); + expect(component.parentRef.current.xyzRef.current.tagName).toBe('SPAN'); + } else if ( + gate(flags => !flags.enableRefAsProp && !flags.disableStringRefs) + ) { + expect(component.childRef).toEqual({current: null}); + expect(component.parentRef.current.xyzRef.current.tagName).toBe('SPAN'); + } else if ( + gate(flags => flags.enableRefAsProp && !flags.disableStringRefs) + ) { + expect(component.childRef).toEqual({current: null}); + expect(component.parentRef.current.xyzRef.current.tagName).toBe('SPAN'); + } else { + // Not going to bother testing every possible combination. + } + }); + + // @gate !disableStringRefs + it('should steal the ref if a new string ref is specified without an owner', async () => { + // Regression test for this specific feature combination calling cloneElement on an element + // without an owner + await expect(async () => { + // create an element without an owner + const element = React.createElement('div', {id: 'some-id'}); + class Parent extends React.Component { + render() { + return {element}; + } + } + let child; + class Child extends React.Component { + render() { + child = this; + const clone = React.cloneElement(this.props.children, { + ref: 'xyz', + }); + return
{clone}
; + } + } + + const root = ReactDOMClient.createRoot(document.createElement('div')); + await act(() => root.render()); + expect(child.refs.xyz.tagName).toBe('DIV'); + }).toErrorDev([ + 'Warning: Component "Child" contains the string ref "xyz". Support for ' + + 'string refs will be removed in a future major release. We recommend ' + + 'using useRef() or createRef() instead. Learn more about using refs ' + + 'safely here: https://react.dev/link/strict-mode-string-ref', + ]); }); it('should overwrite props', async () => { @@ -371,6 +419,15 @@ describe('ReactElementClone', () => { ) { expect(clone.ref).toBe(element.ref); expect(clone.props).toEqual({foo: 'ef'}); + } else if ( + gate(flags => flags.enableRefAsProp && !flags.disableStringRefs) + ) { + expect(() => { + expect(clone.ref).toBe(element.ref); + }).toErrorDev('Accessing element.ref was removed in React 19', { + withoutStack: true, + }); + expect(clone.props).toEqual({foo: 'ef', ref: element.ref}); } else { // Not going to bother testing every possible combination. } diff --git a/packages/react/src/jsx/ReactJSXElement.js b/packages/react/src/jsx/ReactJSXElement.js index b429db3326b5e..d7b70e0f39784 100644 --- a/packages/react/src/jsx/ReactJSXElement.js +++ b/packages/react/src/jsx/ReactJSXElement.js @@ -866,6 +866,7 @@ export function cloneElement(element, config, children) { if (config != null) { if (hasValidRef(config)) { + owner = ReactSharedInternals.owner; if (!enableRefAsProp) { // Silently steal the ref from the parent. ref = config.ref; @@ -873,7 +874,6 @@ export function cloneElement(element, config, children) { ref = coerceStringRef(ref, owner, element.type); } } - owner = ReactSharedInternals.owner; } if (hasValidKey(config)) { if (__DEV__) {