From 6cb196bb4b9bd31a5df952e7a4b8ba660ce1af2d 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 | 46 +++++++++++++++++++ .../react-reconciler/src/ReactChildFiber.js | 12 ++++- packages/react/src/ReactElement.js | 38 ++++++++++++++- 3 files changed, 94 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..2aa42c93e227f 100644 --- 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 ; + } + } + + 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. ' + + '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 ; + } + } + + 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 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 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..ba9bd21bb0a4b 100644 --- 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;