Remove string refs (behind flag)

acdlite · acdlite · commit 8bf8b678629c · 2024-02-27T11:34:45.000-05:00
This removes string refs, which has been deprecated in Strict Mode for
seven years.

I've left them behind a flag for Meta, but in open source this fully
removes the feature.
diff --git a/packages/react-dom/src/__tests__/ReactComponent-test.js b/packages/react-dom/src/__tests__/ReactComponent-test.js
@@ -38,6 +38,7 @@ describe('ReactComponent', () => {
     }).toThrowError(/Target container is not a DOM element./);
   });
 
+  // @gate !disableStringRefs || !__DEV__
   it('should throw when supplying a string ref outside of render method', async () => {
     const container = document.createElement('div');
     const root = ReactDOMClient.createRoot(container);
@@ -125,6 +126,7 @@ describe('ReactComponent', () => {
     }
   });
 
+  // @gate !disableStringRefs
   it('should support string refs on owned components', async () => {
     const innerObj = {};
     const outerObj = {};
diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js
@@ -76,6 +76,7 @@ describe('ReactDOMServerIntegration', () => {
       expect(refElement).toBe(e);
     });
 
+    // @gate !disableStringRefs
     it('should have string refs on client when rendered over server markup', async () => {
       class RefsComponent extends React.Component {
         render() {
diff --git a/packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.js b/packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.js
@@ -64,6 +64,7 @@ describe('ReactDeprecationWarnings', () => {
     );
   });
 
+  // @gate !disableStringRefs
   it('should warn when given string refs', async () => {
     class RefComponent extends React.Component {
       render() {
@@ -87,6 +88,7 @@ describe('ReactDeprecationWarnings', () => {
     );
   });
 
+  // @gate !disableStringRefs
   it('should warn when owner and self are the same for string refs', async () => {
     class RefComponent extends React.Component {
       render() {
@@ -109,6 +111,7 @@ describe('ReactDeprecationWarnings', () => {
     await waitForAll([]);
   });
 
+  // @gate !disableStringRefs
   it('should warn when owner and self are different for string refs', async () => {
     class RefComponent extends React.Component {
       render() {
@@ -139,39 +142,39 @@ describe('ReactDeprecationWarnings', () => {
     ]);
   });
 
-  if (__DEV__) {
-    it('should warn when owner and self are different for string refs', async () => {
-      class RefComponent extends React.Component {
-        render() {
-          return null;
-        }
+  // @gate __DEV__
+  // @gate !disableStringRefs
+  it('should warn when owner and self are different for string refs', async () => {
+    class RefComponent extends React.Component {
+      render() {
+        return null;
       }
-      class Component extends React.Component {
-        render() {
-          return JSXDEVRuntime.jsxDEV(
-            RefComponent,
-            {ref: 'refComponent'},
-            null,
-            false,
-            {},
-            {},
-          );
-        }
+    }
+    class Component extends React.Component {
+      render() {
+        return JSXDEVRuntime.jsxDEV(
+          RefComponent,
+          {ref: 'refComponent'},
+          null,
+          false,
+          {},
+          {},
+        );
       }
+    }
 
-      ReactNoop.render(<Component />);
-      await expect(async () => await waitForAll([])).toErrorDev([
-        'Warning: Component "Component" contains the string ref "refComponent". ' +
-          'Support for string refs will be removed in a future major release. ' +
-          'This case cannot be automatically converted to an arrow function. ' +
-          'We ask you to manually fix this case by using useRef() or createRef() instead. ' +
-          'Learn more about using refs safely here: ' +
-          'https://reactjs.org/link/strict-mode-string-ref',
-        'Warning: Component "Component" contains the string ref "refComponent". ' +
-          '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://reactjs.org/link/strict-mode-string-ref',
-      ]);
-    });
-  }
+    ReactNoop.render(<Component />);
+    await expect(async () => await waitForAll([])).toErrorDev([
+      'Warning: Component "Component" contains the string ref "refComponent". ' +
+        'Support for string refs will be removed in a future major release. ' +
+        'This case cannot be automatically converted to an arrow function. ' +
+        'We ask you to manually fix this case by using useRef() or createRef() instead. ' +
+        'Learn more about using refs safely here: ' +
+        'https://reactjs.org/link/strict-mode-string-ref',
+      'Warning: Component "Component" contains the string ref "refComponent". ' +
+        '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://reactjs.org/link/strict-mode-string-ref',
+    ]);
+  });
 });
diff --git a/packages/react-dom/src/__tests__/ReactFunctionComponent-test.js b/packages/react-dom/src/__tests__/ReactFunctionComponent-test.js
@@ -173,6 +173,7 @@ describe('ReactFunctionComponent', () => {
     ).resolves.not.toThrowError();
   });
 
+  // @gate !disableStringRefs
   it('should throw on string refs in pure functions', async () => {
     function Child() {
       return <div ref="me" />;
diff --git a/packages/react-dom/src/__tests__/multiple-copies-of-react-test.js b/packages/react-dom/src/__tests__/multiple-copies-of-react-test.js
@@ -22,6 +22,7 @@ class TextWithStringRef extends React.Component {
 }
 
 describe('when different React version is used with string ref', () => {
+  // @gate !disableStringRefs
   it('throws the "Refs must have owner" warning', async () => {
     const container = document.createElement('div');
     const root = ReactDOMClient.createRoot(container);
diff --git a/packages/react-dom/src/__tests__/refs-test.js b/packages/react-dom/src/__tests__/refs-test.js
@@ -164,6 +164,7 @@ describe('reactiverefs', () => {
    * Ensure that for every click log there is a corresponding ref (from the
    * perspective of the injected ClickCounter component.
    */
+  // @gate !disableStringRefs
   it('Should increase refs with an increase in divs', async () => {
     const testRefsComponent = await renderTestRefsComponent();
     const clickIncrementer =
@@ -366,6 +367,7 @@ describe('ref swapping', () => {
     expect(refCalled).toBe(1);
   });
 
+  // @gate !disableStringRefs
   it('coerces numbers to strings', async () => {
     class A extends React.Component {
       render() {
@@ -390,6 +392,7 @@ describe('ref swapping', () => {
     expect(a.refs[1].nodeName).toBe('DIV');
   });
 
+  // @gate !disableStringRefs
   it('provides an error for invalid refs', async () => {
     const container = document.createElement('div');
     const root = ReactDOMClient.createRoot(container);
@@ -547,6 +550,7 @@ describe('creating element with string ref in constructor', () => {
     }
   }
 
+  // @gate !disableStringRefs
   it('throws an error', async () => {
     await expect(async function () {
       const container = document.createElement('div');
@@ -567,6 +571,7 @@ describe('creating element with string ref in constructor', () => {
 });
 
 describe('strings refs across renderers', () => {
+  // @gate !disableStringRefs
   it('does not break', async () => {
     class Parent extends React.Component {
       render() {
diff --git a/packages/react-reconciler/src/ReactChildFiber.js b/packages/react-reconciler/src/ReactChildFiber.js
@@ -44,7 +44,7 @@ import {
 import isArray from 'shared/isArray';
 import assign from 'shared/assign';
 import {checkPropStringCoercion} from 'shared/CheckStringCoercion';
-import {enableRefAsProp} from 'shared/ReactFeatureFlags';
+import {enableRefAsProp, disableStringRefs} from 'shared/ReactFeatureFlags';
 
 import {
   createWorkInProgress,
@@ -266,6 +266,7 @@ function coerceRef(
 
   let coercedRef;
   if (
+    !disableStringRefs &&
     mixedRef !== null &&
     typeof mixedRef !== 'function' &&
     typeof mixedRef !== 'object'
diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js
@@ -54,6 +54,7 @@ import {
   enableFloat,
   enableLegacyHidden,
   alwaysThrottleRetries,
+  disableStringRefs,
 } from 'shared/ReactFeatureFlags';
 import {
   FunctionComponent,
@@ -1624,7 +1625,11 @@ function commitAttachRef(finishedWork: Fiber) {
       }
     } else {
       if (__DEV__) {
-        if (!ref.hasOwnProperty('current')) {
+        // TODO: We should move these warnings to happen during the render
+        // phase (markRef).
+        if (disableStringRefs && typeof ref === 'string') {
+          console.error('String refs are no longer supported.');
+        } else if (!ref.hasOwnProperty('current')) {
           console.error(
             'Unexpected ref object provided for %s. ' +
               'Use either a ref-setter function or React.createRef().',
diff --git a/packages/react-reconciler/src/__tests__/ReactFiberRefs-test.js b/packages/react-reconciler/src/__tests__/ReactFiberRefs-test.js
@@ -86,6 +86,7 @@ describe('ReactFiberRefs', () => {
   });
 
   // @gate enableRefAsProp
+  // @gate !disableStringRefs
   test('string ref props are converted to function refs', async () => {
     let refProp;
     function Child({ref}) {
@@ -112,4 +113,25 @@ describe('ReactFiberRefs', () => {
     expect(typeof refProp === 'function').toBe(true);
     expect(owner.refs.child.type).toBe('div');
   });
+
+  // @gate disableStringRefs
+  test('log an error in dev if a string ref is passed to a ref-receiving component', async () => {
+    let refProp;
+    function Child({ref}) {
+      refProp = ref;
+      return <div ref={ref} />;
+    }
+
+    class Owner extends React.Component {
+      render() {
+        return <Child ref="child" />;
+      }
+    }
+
+    const root = ReactNoop.createRoot();
+    await expect(async () => {
+      await expect(act(() => root.render(<Owner />))).rejects.toThrow();
+    }).toErrorDev('String refs are no longer supported');
+    expect(refProp).toBe('child');
+  });
 });
diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalSideEffects-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalSideEffects-test.js
@@ -1346,6 +1346,7 @@ describe('ReactIncrementalSideEffects', () => {
   // TODO: Test that mounts, updates, refs, unmounts and deletions happen in the
   // expected way for aborted and resumed render life-cycles.
 
+  // @gate !disableStringRefs
   it('supports string refs', async () => {
     let fooInstance = null;
 
diff --git a/packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee b/packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee
@@ -535,25 +535,26 @@ describe 'ReactCoffeeScriptClass', ->
 
       test React.createElement(Foo), 'DIV', 'bar-through-context'
 
-  it 'supports string refs', ->
-    class Foo extends React.Component
-      render: ->
-        React.createElement(InnerComponent,
-          name: 'foo'
-          ref: 'inner'
-        )
+  if !featureFlags.disableStringRefs
+    it 'supports string refs', ->
+      class Foo extends React.Component
+        render: ->
+          React.createElement(InnerComponent,
+            name: 'foo'
+            ref: 'inner'
+          )
 
-    ref = React.createRef()
-    expect(->
-      test(React.createElement(Foo, ref: ref), 'DIV', 'foo')
-    ).toErrorDev([
-      'Warning: Component "Foo" contains the string ref "inner". ' +
-        '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://reactjs.org/link/strict-mode-string-ref\n' +
-        '    in Foo (at **)'
-    ]);
-    expect(ref.current.refs.inner.getName()).toBe 'foo'
+      ref = React.createRef()
+      expect(->
+        test(React.createElement(Foo, ref: ref), 'DIV', 'foo')
+      ).toErrorDev([
+        'Warning: Component "Foo" contains the string ref "inner". ' +
+          '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://reactjs.org/link/strict-mode-string-ref\n' +
+          '    in Foo (at **)'
+      ]);
+      expect(ref.current.refs.inner.getName()).toBe 'foo'
 
   it 'supports drilling through to the DOM using findDOMNode', ->
     ref = React.createRef()
diff --git a/packages/react/src/__tests__/ReactES6Class-test.js b/packages/react/src/__tests__/ReactES6Class-test.js
@@ -576,24 +576,26 @@ describe('ReactES6Class', () => {
     });
   }
 
-  it('supports string refs', () => {
-    class Foo extends React.Component {
-      render() {
-        return <Inner name="foo" ref="inner" />;
+  if (!require('shared/ReactFeatureFlags').disableStringRefs) {
+    it('supports string refs', () => {
+      class Foo extends React.Component {
+        render() {
+          return <Inner name="foo" ref="inner" />;
+        }
       }
-    }
-    const ref = React.createRef();
-    expect(() => {
-      test(<Foo ref={ref} />, 'DIV', 'foo');
-    }).toErrorDev([
-      'Warning: Component "Foo" contains the string ref "inner". ' +
-        '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://reactjs.org/link/strict-mode-string-ref\n' +
-        '    in Foo (at **)',
-    ]);
-    expect(ref.current.refs.inner.getName()).toBe('foo');
-  });
+      const ref = React.createRef();
+      expect(() => {
+        test(<Foo ref={ref} />, 'DIV', 'foo');
+      }).toErrorDev([
+        'Warning: Component "Foo" contains the string ref "inner". ' +
+          '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://reactjs.org/link/strict-mode-string-ref\n' +
+          '    in Foo (at **)',
+      ]);
+      expect(ref.current.refs.inner.getName()).toBe('foo');
+    });
+  }
 
   it('supports drilling through to the DOM using findDOMNode', () => {
     const ref = React.createRef();
diff --git a/packages/react/src/__tests__/ReactProfilerDevToolsIntegration-test.internal.js b/packages/react/src/__tests__/ReactProfilerDevToolsIntegration-test.internal.js
@@ -118,13 +118,17 @@ describe('ReactProfiler DevTools integration', () => {
 
     Scheduler.unstable_advanceTime(20);
 
+    function Throws() {
+      throw new Error('Oops!');
+    }
+
     expect(() => {
       rendered.update(
-        <div ref="this-will-cause-an-error">
+        <Throws>
           <AdvanceTime byAmount={3} />
-        </div>,
+        </Throws>,
       );
-    }).toThrow();
+    }).toThrow('Oops!');
 
     Scheduler.unstable_advanceTime(20);
 
diff --git a/packages/react/src/__tests__/ReactStrictMode-test.js b/packages/react/src/__tests__/ReactStrictMode-test.js
@@ -968,6 +968,7 @@ describe('string refs', () => {
     act = require('internal-test-utils').act;
   });
 
+  // @gate !disableStringRefs
   it('should warn within a strict tree', async () => {
     const {StrictMode} = React;
 
@@ -1006,6 +1007,7 @@ describe('string refs', () => {
     });
   });
 
+  // @gate !disableStringRefs
   it('should warn within a strict tree', async () => {
     const {StrictMode} = React;
 
diff --git a/packages/react/src/__tests__/ReactTypeScriptClass-test.ts b/packages/react/src/__tests__/ReactTypeScriptClass-test.ts
diff --git a/packages/react/src/jsx/ReactJSXElement.js b/packages/react/src/jsx/ReactJSXElement.js

Original file line number	Diff line number	Diff line change
`@@ -22,6 +22,7 @@ class TextWithStringRef extends React.Component {`
`22`	`22`	`}`
`23`	`23`
`24`	`24`	`describe('when different React version is used with string ref', () => {`
	`25`	`+ // @gate !disableStringRefs`
`25`	`26`	`it('throws the "Refs must have owner" warning', async () => {`
`26`	`27`	`const container = document.createElement('div');`
`27`	`28`	`const root = ReactDOMClient.createRoot(container);`