add warning when owner and self are different for string refs

facebook · Jan 21, 2020 · 6cb196b · 6cb196b
1 parent 29b4d07
commit 6cb196b
Show file tree

Hide file tree

Showing 3 changed files with 94 additions and 2 deletions.
diff --git a/packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.internal.js b/packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.internal.js
@@ -69,4 +69,50 @@ describe('ReactDeprecationWarnings', () => {
         '\n    in Component (at **)',
     );
   });
+
+  it('should not warn when owner and self are the same for string refs', () => {
+    class RefComponent extends React.Component {
+      render() {
+        return null;
+      }
+    }
+    class Component extends React.Component {
+      render() {
+        return <RefComponent ref="refComponent" __self={this} />;
+      }
+    }
+
+    ReactNoop.render(<Component />);
+    expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev([
+      '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://fb.me/react-strict-mode-string-ref' +
+        '\n    in Component (at **)',
+    ]);
+  });
+
+  it('should warn when owner and self are different for string refs', () => {
+    class RefComponent extends React.Component {
+      render() {
+        return null;
+      }
+    }
+    class Component extends React.Component {
+      render() {
+        return <RefComponent ref="refComponent" __self={{}} />;
+      }
+    }
+
+    ReactNoop.render(<Component />);
+    expect(() => expect(Scheduler).toFlushWithoutYielding()).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 as you to manually fix this case by using useRef() or createRef() instead. ' +
+        'Learn more about using refs safely here: ' +
+        'https://fb.me/react-strict-mode-string-ref',
+    ]);
+  });
 });
diff --git a/packages/react-reconciler/src/ReactChildFiber.js b/packages/react-reconciler/src/ReactChildFiber.js
@@ -116,7 +116,17 @@ function coerceRef(
     if (__DEV__) {
       // TODO: Clean this up once we turn on the string ref warning for
       // everyone, because the strict mode case will no longer be relevant
-      if (returnFiber.mode & StrictMode || warnAboutStringRefs) {
+      if (
+        (returnFiber.mode & StrictMode || warnAboutStringRefs) &&
+        // We warn in ReactElement.js if owner and self are equal for string refs
+        // because these cannot be automatically converted to an arrow function
+        // using a codemod. Therefore, we don't have to warn about string refs again.
+        !(
+          element._owner &&
+          element._self &&
+          element._owner.stateNode !== element._self
+        )
+      ) {
         const componentName = getComponentName(returnFiber.type) || 'Component';
         if (!didWarnAboutStringRefs[componentName]) {
           if (warnAboutStringRefs) {

diff --git a/packages/react/src/ReactElement.js b/packages/react/src/ReactElement.js
@@ -9,6 +9,7 @@ import invariant from 'shared/invariant';
 import {REACT_ELEMENT_TYPE} from 'shared/ReactSymbols';
 
 import ReactCurrentOwner from './ReactCurrentOwner';
+import getComponentName from 'shared/getComponentName';
 
 const hasOwnProperty = Object.prototype.hasOwnProperty;
 
@@ -19,7 +20,13 @@ const RESERVED_PROPS = {
   __source: true,
 };
 
-let specialPropKeyWarningShown, specialPropRefWarningShown;
+let specialPropKeyWarningShown,
+  specialPropRefWarningShown,
+  didWarnAboutStringRefs;
+
+if (__DEV__) {
+  didWarnAboutStringRefs = {};
+}
 
 function hasValidRef(config) {
   if (__DEV__) {
@@ -89,6 +96,31 @@ function defineRefPropWarningGetter(props, displayName) {
   });
 }
 
+function warnIfStringRefCannotBeAutoConverted(config) {
+  if (
+    typeof config.ref === 'string' &&
+    ReactCurrentOwner.current &&
+    config.__self &&
+    ReactCurrentOwner.current.stateNode !== config.__self
+  ) {
+    const componentName = getComponentName(ReactCurrentOwner.current.type);
+
+    if (!didWarnAboutStringRefs[componentName]) {
+      console.error(
+        'Component "%s" contains the string ref "%s". ' +
+          'Support for string refs will be removed in a future major release. ' +
+          'This case cannot be automatically converted to an arrow function. ' +
+          'We as you to manually fix this case by using useRef() or createRef() instead. ' +
+          'Learn more about using refs safely here: ' +
+          'https://fb.me/react-strict-mode-string-ref',
+        getComponentName(ReactCurrentOwner.current.type),
+        config.ref,
+      );
+      didWarnAboutStringRefs[componentName] = true;
+    }
+  }
+}
+
 /**
  * Factory method to create a new React element. This no longer adheres to
  * the class pattern, so do not use new to call it. Also, instanceof check
@@ -260,6 +292,7 @@ export function jsxDEV(type, config, maybeKey, source, self) {
 
   if (hasValidRef(config)) {
     ref = config.ref;
+    warnIfStringRefCannotBeAutoConverted(config);
   }
 
   // Remaining properties are added to a new props object
@@ -324,6 +357,9 @@ export function createElement(type, config, children) {
   if (config != null) {
     if (hasValidRef(config)) {
       ref = config.ref;
+      if (__DEV__) {
+        warnIfStringRefCannotBeAutoConverted(config);
+      }
     }
     if (hasValidKey(config)) {
       key = '' + config.key;