Skip to content

Commit

Permalink
add warning when owner and self are different for string refs
Browse files Browse the repository at this point in the history
  • Loading branch information
lunaruan committed Jan 21, 2020
1 parent 29b4d07 commit 75a0552
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 2 deletions.
Expand Up @@ -69,4 +69,61 @@ 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 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 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',
]);
});
});
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 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
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 75a0552

Please sign in to comment.