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 <rickhanlonii@fb.com>
facebook · Apr 11, 2024 · e2844c2 · e2844c2
1 parent 51638d3
commit e2844c2
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 3 deletions.
diff --git 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(<Grandparent />));
-    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 <Child>{element}</Child>;
+        }
+      }
+      let child;
+      class Child extends React.Component {
+        render() {
+          child = this;
+          const clone = React.cloneElement(this.props.children, {
+            ref: 'xyz',
+          });
+          return <div>{clone}</div>;
+        }
+      }
+
+      const root = ReactDOMClient.createRoot(document.createElement('div'));
+      await act(() => root.render(<Parent />));
+      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
@@ -853,14 +853,14 @@ export function cloneElement(element, config, children) {
 
   if (config != null) {
     if (hasValidRef(config)) {
+      owner = ReactCurrentOwner.owner;
       if (!enableRefAsProp) {
         // Silently steal the ref from the parent.
         ref = config.ref;
         if (!disableStringRefs) {
           ref = coerceStringRef(ref, owner, element.type);
         }
       }
-      owner = ReactCurrentOwner.current;
     }
     if (hasValidKey(config)) {
       if (__DEV__) {