From a786481ae5702f1966ecdb62f3667f3d72966e78 Mon Sep 17 00:00:00 2001
From: Andrew Clark <git@andrewclark.io>
Date: Thu, 11 Apr 2024 14:58:38 -0400
Subject: [PATCH] Reuse string ref if conceptually identical

Now that string refs are coerced to callback refs in the JSX runtime,
a new callback ref is recreated each time. This is a subtle behavior
difference from the old behavior, because it means React will reattach
the ref on every render. While this is mostly not a huge issue, it is
technically observable because a child component can observe a parent
component's ref inside a layout effect or componentDidUpdate before the
parent ref is able to update (because layout effects and refs run in
child -> parent order).

To preserve the old behavior, I added the string refs "deps" as
extra properties on the callback. Then in the reconciler, we can compare
the deps to check whether the old callback ref can be reused.

This is similar to what we did before but in addition to checking the
string itself, we also need to check the other and the type, since those
are bound earlier than they were before.
---
 .../src/__tests__/ReactComponent-test.js      |  7 ++++++-
 .../src/ReactFiberBeginWork.js                | 19 +++++++++++++++++++
 packages/react/src/jsx/ReactJSXElement.js     | 10 +++++++++-
 3 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/packages/react-dom/src/__tests__/ReactComponent-test.js b/packages/react-dom/src/__tests__/ReactComponent-test.js
index 0fb81ba7366f4..678a7ce4f6db9 100644
--- a/packages/react-dom/src/__tests__/ReactComponent-test.js
+++ b/packages/react-dom/src/__tests__/ReactComponent-test.js
@@ -130,12 +130,17 @@ describe('ReactComponent', () => {
   });
 
   // @gate !disableStringRefs
-  it('should support accessing string refs from parent components', async () => {
+  it('string refs do not detach and reattach on every render', async () => {
     spyOnDev(console, 'error').mockImplementation(() => {});
 
     let refVal;
     class Child extends React.Component {
       componentDidUpdate() {
+        // The parent ref should still be attached because it hasn't changed
+        // since the last render. If the ref had changed, then this would be
+        // undefined because refs are attached during the same phase (layout)
+        // as componentDidUpdate, in child -> parent order. So the new parent
+        // ref wouldn't have attached yet.
         refVal = this.props.contextRef();
       }
 
diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js
index 21964cc456e3e..034c853976aa8 100644
--- a/packages/react-reconciler/src/ReactFiberBeginWork.js
+++ b/packages/react-reconciler/src/ReactFiberBeginWork.js
@@ -1048,6 +1048,25 @@ function markRef(current: Fiber | null, workInProgress: Fiber) {
       );
     }
     if (current === null || current.ref !== ref) {
+      if (!disableStringRefs && current !== null) {
+        const oldRef = current.ref;
+        const newRef = ref;
+        if (
+          typeof oldRef === 'function' &&
+          typeof newRef === 'function' &&
+          typeof oldRef.__stringRef === 'string' &&
+          oldRef.__stringRef === newRef.__stringRef &&
+          oldRef.__stringRefType === newRef.__stringRefType &&
+          oldRef.__stringRefOwner === newRef.__stringRefOwner
+        ) {
+          // Although this is a different callback, it represents the same
+          // string ref. To avoid breaking old Meta code that relies on string
+          // refs only being attached once, reuse the old ref. This will
+          // prevent us from detaching and reattaching the ref on each update.
+          workInProgress.ref = oldRef;
+          return;
+        }
+      }
       // Schedule a Ref effect
       workInProgress.flags |= Ref | RefStatic;
     }
diff --git a/packages/react/src/jsx/ReactJSXElement.js b/packages/react/src/jsx/ReactJSXElement.js
index 2b9955fcc536a..48feb83cdde35 100644
--- a/packages/react/src/jsx/ReactJSXElement.js
+++ b/packages/react/src/jsx/ReactJSXElement.js
@@ -1189,7 +1189,15 @@ function coerceStringRef(mixedRef, owner, type) {
     }
   }
 
-  return stringRefAsCallbackRef.bind(null, stringRef, type, owner);
+  const callback = stringRefAsCallbackRef.bind(null, stringRef, type, owner);
+  // This is used to check whether two callback refs conceptually represent
+  // the same string ref, and can therefore be reused by the reconciler. Needed
+  // for backwards compatibility with old Meta code that relies on string refs
+  // not being reattached on every render.
+  callback.__stringRef = stringRef;
+  callback.__type = type;
+  callback.__owner = owner;
+  return callback;
 }
 
 function stringRefAsCallbackRef(stringRef, type, owner, value) {