From 9d2dd811dc4a1fb3db4533ccb589bdc5e9f21c84 Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Tue, 9 Apr 2024 11:14:26 -0700
Subject: [PATCH 1/7] Fix cloneElement using string ref w no owner

---
 .../src/__tests__/ReactElementClone-test.js   | 23 +++++++++++++++++--
 packages/react/src/jsx/ReactJSXElement.js     |  2 +-
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/packages/react/src/__tests__/ReactElementClone-test.js b/packages/react/src/__tests__/ReactElementClone-test.js
index 11bb847518e41..fdf5f26ecc951 100644
--- a/packages/react/src/__tests__/ReactElementClone-test.js
+++ b/packages/react/src/__tests__/ReactElementClone-test.js
@@ -274,8 +274,22 @@ 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.
+    }
   });
 
   it('should overwrite props', async () => {
@@ -371,6 +385,11 @@ describe('ReactElementClone', () => {
     ) {
       expect(clone.ref).toBe(element.ref);
       expect(clone.props).toEqual({foo: 'ef'});
+    } else if (
+      gate(flags => flags.enableRefAsProp && !flags.disableStringRefs)
+    ) {
+      expect(clone.ref).toBe(element.ref);
+      expect(clone.props).toEqual({foo: 'ef'});
     } 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__) {

From f7b42b3fb8ff16b14aa261c801475912c85e86db Mon Sep 17 00:00:00 2001
From: Rick Hanlon <rickhanlonii@fb.com>
Date: Tue, 9 Apr 2024 15:06:01 -0400
Subject: [PATCH 2/7] Fix tests

---
 packages/react/src/__tests__/ReactElementClone-test.js | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/packages/react/src/__tests__/ReactElementClone-test.js b/packages/react/src/__tests__/ReactElementClone-test.js
index fdf5f26ecc951..fe7a624b3b91b 100644
--- a/packages/react/src/__tests__/ReactElementClone-test.js
+++ b/packages/react/src/__tests__/ReactElementClone-test.js
@@ -388,8 +388,12 @@ describe('ReactElementClone', () => {
     } else if (
       gate(flags => flags.enableRefAsProp && !flags.disableStringRefs)
     ) {
-      expect(clone.ref).toBe(element.ref);
-      expect(clone.props).toEqual({foo: 'ef'});
+      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.
     }

From c97b811cd89940af5fdd5cc63d2f09ccc399ac43 Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Tue, 9 Apr 2024 13:14:11 -0700
Subject: [PATCH 3/7] add repro

---
 .../src/__tests__/ReactElementClone-test.js   | 33 +++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/packages/react/src/__tests__/ReactElementClone-test.js b/packages/react/src/__tests__/ReactElementClone-test.js
index fdf5f26ecc951..e94f7e5d66d18 100644
--- a/packages/react/src/__tests__/ReactElementClone-test.js
+++ b/packages/react/src/__tests__/ReactElementClone-test.js
@@ -292,6 +292,39 @@ describe('ReactElementClone', () => {
     }
   });
 
+  it('should steal the ref if a new string ref is specified', async () => {
+    if (gate(flags => !flags.enableRefAsProp && !flags.disableStringRefs)) {
+      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 () => {
     class Component extends React.Component {
       render() {

From 28d13d2cb247edcbd50c11b7ad4ef13a2e0468f8 Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Tue, 9 Apr 2024 13:23:52 -0700
Subject: [PATCH 4/7] fix test name

---
 packages/react/src/__tests__/ReactElementClone-test.js | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/packages/react/src/__tests__/ReactElementClone-test.js b/packages/react/src/__tests__/ReactElementClone-test.js
index a35456210a3e2..4a350103d22cd 100644
--- a/packages/react/src/__tests__/ReactElementClone-test.js
+++ b/packages/react/src/__tests__/ReactElementClone-test.js
@@ -292,7 +292,9 @@ describe('ReactElementClone', () => {
     }
   });
 
-  it('should steal the ref if a new string ref is specified', async () => {
+  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
     if (gate(flags => !flags.enableRefAsProp && !flags.disableStringRefs)) {
       await expect(async () => {
         // create an element without an owner

From bb30f2bb99628bdf478a01812c510ee570dd5a55 Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Tue, 9 Apr 2024 13:24:32 -0700
Subject: [PATCH 5/7] use @gate comment

---
 .../src/__tests__/ReactElementClone-test.js   | 53 +++++++++----------
 1 file changed, 26 insertions(+), 27 deletions(-)

diff --git a/packages/react/src/__tests__/ReactElementClone-test.js b/packages/react/src/__tests__/ReactElementClone-test.js
index 4a350103d22cd..8f27b97aaceec 100644
--- a/packages/react/src/__tests__/ReactElementClone-test.js
+++ b/packages/react/src/__tests__/ReactElementClone-test.js
@@ -292,39 +292,38 @@ describe('ReactElementClone', () => {
     }
   });
 
+  // @gate !flags.enableRefAsProp && !flags.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
-    if (gate(flags => !flags.enableRefAsProp && !flags.disableStringRefs)) {
-      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>;
-          }
+    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>;
-          }
+      }
+      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',
-      ]);
-    }
+      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 () => {

From 9aedf716001e92a5340557b23b5ba08eab0aed67 Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Tue, 9 Apr 2024 13:31:33 -0700
Subject: [PATCH 6/7] undo use of at-gate since it fails in other combinations

---
 .../src/__tests__/ReactElementClone-test.js   | 53 ++++++++++---------
 1 file changed, 27 insertions(+), 26 deletions(-)

diff --git a/packages/react/src/__tests__/ReactElementClone-test.js b/packages/react/src/__tests__/ReactElementClone-test.js
index 8f27b97aaceec..4a350103d22cd 100644
--- a/packages/react/src/__tests__/ReactElementClone-test.js
+++ b/packages/react/src/__tests__/ReactElementClone-test.js
@@ -292,38 +292,39 @@ describe('ReactElementClone', () => {
     }
   });
 
-  // @gate !flags.enableRefAsProp && !flags.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>;
+    if (gate(flags => !flags.enableRefAsProp && !flags.disableStringRefs)) {
+      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>;
+        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',
-    ]);
+        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 () => {

From 51621113edbf82f60c4ad7c2ff4e9c5bb63e2a17 Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Tue, 9 Apr 2024 13:35:39 -0700
Subject: [PATCH 7/7] maybe gate only on !disableStringRefs????

---
 .../src/__tests__/ReactElementClone-test.js   | 53 +++++++++----------
 1 file changed, 26 insertions(+), 27 deletions(-)

diff --git a/packages/react/src/__tests__/ReactElementClone-test.js b/packages/react/src/__tests__/ReactElementClone-test.js
index 4a350103d22cd..a8cf751635149 100644
--- a/packages/react/src/__tests__/ReactElementClone-test.js
+++ b/packages/react/src/__tests__/ReactElementClone-test.js
@@ -292,39 +292,38 @@ describe('ReactElementClone', () => {
     }
   });
 
+  // @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
-    if (gate(flags => !flags.enableRefAsProp && !flags.disableStringRefs)) {
-      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>;
-          }
+    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>;
-          }
+      }
+      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',
-      ]);
-    }
+      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 () => {