Skip to content

Commit

Permalink
add warning when owner and self are different for string refs (#17864)
Browse files Browse the repository at this point in the history
When owner and self are different for string refs, we can't easily convert them to callback refs. This PR adds a warning for string refs when owner and self are different to tell users to manually update these refs.
  • Loading branch information
lunaruan committed Jan 22, 2020
1 parent 6c00c5b commit 3b3decf
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 2 deletions.
Expand Up @@ -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 <RefComponent ref="refComponent" __self={this} />;
}
}
ReactNoop.renderLegacySyncRoot(<Component />);
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 <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 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',
]);
});
});
12 changes: 11 additions & 1 deletion packages/react-reconciler/src/ReactChildFiber.js
Expand Up @@ -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) {
Expand Down
41 changes: 40 additions & 1 deletion packages/react/src/ReactElement.js
Expand Up @@ -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';

Expand All @@ -19,7 +20,13 @@ const RESERVED_PROPS = {
__source: true,
};

let specialPropKeyWarningShown, specialPropRefWarningShown;
let specialPropKeyWarningShown,
specialPropRefWarningShown,
didWarnAboutStringRefs;

if (__DEV__) {
didWarnAboutStringRefs = {};
}

function hasValidRef(config) {
if (__DEV__) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 3b3decf

Please sign in to comment.