From ff93a34ddfe5a82f1bab6f486c53a773511b7ff7 Mon Sep 17 00:00:00 2001 From: Luna Ruan Date: Fri, 17 Jan 2020 14:52:30 -0800 Subject: [PATCH] add warning when owner and self are different for string refs --- .../ReactDeprecationWarnings-test.internal.js | 40 ++++++++++++++++++ .../react-reconciler/src/ReactChildFiber.js | 12 +++++- packages/react/src/ReactElement.js | 41 ++++++++++++++++++- 3 files changed, 91 insertions(+), 2 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.internal.js b/packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.internal.js index 7105260288d05..3a8ce27ce7dd7 100644 --- a/packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.internal.js @@ -69,4 +69,44 @@ describe('ReactDeprecationWarnings', () => { '\n in Component (at **)', ); }); + + it('should not warn when owner and self are the same for string refs', () => { + ReactFeatureFlags.warnAboutStringRefs = false; + + class RefComponent extends React.Component { + render() { + return null; + } + } + class Component extends React.Component { + render() { + return ; + } + } + ReactNoop.renderLegacySyncRoot(); + expect(Scheduler).toFlushWithoutYielding(); + }); + + 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 ; + } + } + + ReactNoop.render(); + 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 ask 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 index bd226d7a25d07..f5203c7e0db39 100644 --- 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 index 47c7cf2015ef8..9a0f208696d67 100644 --- a/packages/react/src/ReactElement.js +++ b/packages/react/src/ReactElement.js @@ -5,6 +5,7 @@ * LICENSE file in the root directory of this source tree. */ +import getComponentName from 'shared/getComponentName'; import invariant from 'shared/invariant'; import {REACT_ELEMENT_TYPE} from 'shared/ReactSymbols'; @@ -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,33 @@ function defineRefPropWarningGetter(props, displayName) { }); } +function warnIfStringRefCannotBeAutoConverted(config) { + if (__DEV__) { + 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 ask 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 +294,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 +359,10 @@ 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;