Warn when Using String Refs (#16217)

facebook · Aug 7, 2019 · c4f0b93 · c4f0b93
1 parent 7c838a6
commit c4f0b93
Show file tree

Hide file tree

Showing 10 changed files with 77 additions and 23 deletions.
diff --git a/packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.internal.js b/packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.internal.js
@@ -10,20 +10,24 @@
 'use strict';
 
 let React;
-let ReactTestUtils;
 let ReactFeatureFlags;
+let ReactNoop;
+let Scheduler;
 
 describe('ReactDeprecationWarnings', () => {
   beforeEach(() => {
     jest.resetModules();
     React = require('react');
     ReactFeatureFlags = require('shared/ReactFeatureFlags');
-    ReactTestUtils = require('react-dom/test-utils');
+    ReactNoop = require('react-noop-renderer');
+    Scheduler = require('scheduler');
     ReactFeatureFlags.warnAboutDefaultPropsOnFunctionComponents = true;
+    ReactFeatureFlags.warnAboutStringRefs = true;
   });
 
   afterEach(() => {
     ReactFeatureFlags.warnAboutDefaultPropsOnFunctionComponents = false;
+    ReactFeatureFlags.warnAboutStringRefs = false;
   });
 
   it('should warn when given defaultProps', () => {
@@ -35,13 +39,37 @@ describe('ReactDeprecationWarnings', () => {
       testProp: true,
     };
 
-    expect(() =>
-      ReactTestUtils.renderIntoDocument(<FunctionalComponent />),
-    ).toWarnDev(
+    ReactNoop.render(<FunctionalComponent />);
+    expect(() => expect(Scheduler).toFlushWithoutYielding()).toWarnDev(
       'Warning: FunctionalComponent: Support for defaultProps ' +
         'will be removed from function components in a future major ' +
         'release. Use JavaScript default parameters instead.',
       {withoutStack: true},
     );
   });
+
+  it('should warn when given string refs', () => {
+    class RefComponent extends React.Component {
+      render() {
+        return null;
+      }
+    }
+    class Component extends React.Component {
+      render() {
+        return <RefComponent ref="refComponent" />;
+      }
+    }
+
+    ReactNoop.render(<Component />);
+    expect(() => expect(Scheduler).toFlushWithoutYielding()).toWarnDev(
+      '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.' +
+        '\n\n' +
+        '    in Component (at **)' +
+        '\n\n' +
+        'Learn more about using refs safely here:\n' +
+        'https://fb.me/react-strict-mode-string-ref',
+    );
+  });
 });
diff --git a/packages/react-reconciler/src/ReactChildFiber.js b/packages/react-reconciler/src/ReactChildFiber.js
@@ -30,6 +30,7 @@ import {
 import invariant from 'shared/invariant';
 import warning from 'shared/warning';
 import warningWithoutStack from 'shared/warningWithoutStack';
+import {warnAboutStringRefs} from 'shared/ReactFeatureFlags';
 
 import {
   createWorkInProgress,
@@ -49,15 +50,15 @@ import {StrictMode} from './ReactTypeOfMode';
 
 let didWarnAboutMaps;
 let didWarnAboutGenerators;
-let didWarnAboutStringRefInStrictMode;
+let didWarnAboutStringRefs;
 let ownerHasKeyUseWarning;
 let ownerHasFunctionTypeWarning;
 let warnForMissingKey = (child: mixed) => {};
 
 if (__DEV__) {
   didWarnAboutMaps = false;
   didWarnAboutGenerators = false;
-  didWarnAboutStringRefInStrictMode = {};
+  didWarnAboutStringRefs = {};
 
   /**
    * Warn if there's no key explicitly set on dynamic arrays of children or
@@ -114,21 +115,38 @@ function coerceRef(
     typeof mixedRef !== 'object'
   ) {
     if (__DEV__) {
-      if (returnFiber.mode & StrictMode) {
+      // 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) {
         const componentName = getComponentName(returnFiber.type) || 'Component';
-        if (!didWarnAboutStringRefInStrictMode[componentName]) {
-          warningWithoutStack(
-            false,
-            'A string ref, "%s", has been found within a strict mode tree. ' +
-              'String refs are a source of potential bugs and should be avoided. ' +
-              'We recommend using createRef() instead.' +
-              '\n%s' +
-              '\n\nLearn more about using refs safely here:' +
-              '\nhttps://fb.me/react-strict-mode-string-ref',
-            mixedRef,
-            getStackByFiberInDevAndProd(returnFiber),
-          );
-          didWarnAboutStringRefInStrictMode[componentName] = true;
+        if (!didWarnAboutStringRefs[componentName]) {
+          if (warnAboutStringRefs) {
+            warningWithoutStack(
+              false,
+              'Component "%s" contains the string ref "%s". Support for string refs ' +
+                'will be removed in a future major release. We recommend using ' +
+                'useRef() or createRef() instead.' +
+                '\n%s' +
+                '\n\nLearn more about using refs safely here:' +
+                '\nhttps://fb.me/react-strict-mode-string-ref',
+              componentName,
+              mixedRef,
+              getStackByFiberInDevAndProd(returnFiber),
+            );
+          } else {
+            warningWithoutStack(
+              false,
+              'A string ref, "%s", has been found within a strict mode tree. ' +
+                'String refs are a source of potential bugs and should be avoided. ' +
+                'We recommend using useRef() or createRef() instead.' +
+                '\n%s' +
+                '\n\nLearn more about using refs safely here:' +
+                '\nhttps://fb.me/react-strict-mode-string-ref',
+              mixedRef,
+              getStackByFiberInDevAndProd(returnFiber),
+            );
+          }
+          didWarnAboutStringRefs[componentName] = true;
         }
       }
     }

diff --git a/packages/react/src/__tests__/ReactStrictMode-test.internal.js b/packages/react/src/__tests__/ReactStrictMode-test.internal.js
@@ -693,7 +693,7 @@ Please update the following components: Parent`,
       }).toWarnDev(
         'Warning: A string ref, "somestring", has been found within a strict mode tree. ' +
           'String refs are a source of potential bugs and should be avoided. ' +
-          'We recommend using createRef() instead.\n\n' +
+          'We recommend using useRef() or createRef() instead.\n\n' +
           '    in StrictMode (at **)\n' +
           '    in OuterComponent (at **)\n\n' +
           'Learn more about using refs safely here:\n' +
@@ -735,7 +735,7 @@ Please update the following components: Parent`,
       }).toWarnDev(
         'Warning: A string ref, "somestring", has been found within a strict mode tree. ' +
           'String refs are a source of potential bugs and should be avoided. ' +
-          'We recommend using createRef() instead.\n\n' +
+          'We recommend using useRef() or createRef() instead.\n\n' +
           '    in InnerComponent (at **)\n' +
           '    in StrictMode (at **)\n' +
           '    in OuterComponent (at **)\n\n' +

diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js
@@ -92,6 +92,7 @@ export const enableSuspenseCallback = false;
 // from React.createElement to React.jsx
 // https://github.com/reactjs/rfcs/blob/createlement-rfc/text/0000-create-element-changes.md
 export const warnAboutDefaultPropsOnFunctionComponents = false;
+export const warnAboutStringRefs = false;
 
 export const disableLegacyContext = false;
 

diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js
@@ -40,6 +40,7 @@ export const flushSuspenseFallbacksInTests = true;
 export const enableUserBlockingEvents = false;
 export const enableSuspenseCallback = false;
 export const warnAboutDefaultPropsOnFunctionComponents = false;
+export const warnAboutStringRefs = false;
 export const disableLegacyContext = false;
 export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
 

diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js
@@ -35,6 +35,7 @@ export const flushSuspenseFallbacksInTests = true;
 export const enableUserBlockingEvents = false;
 export const enableSuspenseCallback = false;
 export const warnAboutDefaultPropsOnFunctionComponents = false;
+export const warnAboutStringRefs = false;
 export const disableLegacyContext = false;
 export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
 

diff --git a/packages/shared/forks/ReactFeatureFlags.persistent.js b/packages/shared/forks/ReactFeatureFlags.persistent.js
@@ -35,6 +35,7 @@ export const flushSuspenseFallbacksInTests = true;
 export const enableUserBlockingEvents = false;
 export const enableSuspenseCallback = false;
 export const warnAboutDefaultPropsOnFunctionComponents = false;
+export const warnAboutStringRefs = false;
 export const disableLegacyContext = false;
 export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
 

diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js
@@ -35,6 +35,7 @@ export const flushSuspenseFallbacksInTests = true;
 export const enableUserBlockingEvents = false;
 export const enableSuspenseCallback = false;
 export const warnAboutDefaultPropsOnFunctionComponents = false;
+export const warnAboutStringRefs = false;
 export const disableLegacyContext = false;
 export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
 

diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
@@ -35,6 +35,7 @@ export const flushSuspenseFallbacksInTests = true;
 export const enableUserBlockingEvents = false;
 export const enableSuspenseCallback = true;
 export const warnAboutDefaultPropsOnFunctionComponents = false;
+export const warnAboutStringRefs = false;
 export const disableLegacyContext = false;
 export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
 

diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js
@@ -82,6 +82,8 @@ export const enableSuspenseCallback = true;
 
 export const warnAboutDefaultPropsOnFunctionComponents = false;
 
+export const warnAboutStringRefs = false;
+
 export const flushSuspenseFallbacksInTests = true;
 
 // Flow magic to verify the exports of this file match the original version.