Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Turn on string ref deprecation warning for everybody (codemoddable) #25334

Closed
29 changes: 25 additions & 4 deletions packages/react-dom/src/__tests__/ReactComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
let React;
let ReactDOM;
let ReactDOMServer;
let ReactFeatureFlags;
let ReactTestUtils;

describe('ReactComponent', () => {
Expand All @@ -21,6 +22,7 @@ describe('ReactComponent', () => {
React = require('react');
ReactDOM = require('react-dom');
ReactDOMServer = require('react-dom/server');
ReactFeatureFlags = require('shared/ReactFeatureFlags');
ReactTestUtils = require('react-dom/test-utils');
});

Expand All @@ -36,7 +38,7 @@ describe('ReactComponent', () => {
}).toThrowError(/Target container is not a DOM element./);
});

it('should throw when supplying a ref outside of render method', () => {
it('should throw when supplying a string ref outside of render method', () => {
let instance = <div ref="badDiv" />;
expect(function() {
instance = ReactTestUtils.renderIntoDocument(instance);
Expand Down Expand Up @@ -102,7 +104,7 @@ describe('ReactComponent', () => {
}
});

it('should support refs on owned components', () => {
it('should support string refs on owned components', () => {
const innerObj = {};
const outerObj = {};

Expand Down Expand Up @@ -133,10 +135,29 @@ describe('ReactComponent', () => {
}
}

ReactTestUtils.renderIntoDocument(<Component />);
expect(() => {
ReactTestUtils.renderIntoDocument(<Component />);
}).toErrorDev(
ReactFeatureFlags.warnAboutStringRefs
? [
'Warning: Component "div" contains the string ref "inner". ' +
'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://reactjs.org/link/strict-mode-string-ref\n' +
' in div (at **)\n' +
' in Wrapper (at **)\n' +
' in Component (at **)',
'Warning: Component "Component" contains the string ref "outer". ' +
'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://reactjs.org/link/strict-mode-string-ref\n' +
' in Component (at **)',
]
: [],
);
});

it('should not have refs on unmounted components', () => {
it('should not have string refs on unmounted components', () => {
class Parent extends React.Component {
render() {
return (
Expand Down
15 changes: 13 additions & 2 deletions packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,14 @@ describe('ReactComponentLifeCycle', () => {
}
// you would *NEVER* do anything like this in real code!
this.state.hasRenderCompleted = true;
return <div ref="theDiv">I am the inner DIV</div>;
return (
<div
ref={current => {
this.refs.theDiv = current;
}}>
I am the inner DIV
</div>
);
}

componentWillUnmount() {
Expand Down Expand Up @@ -477,7 +484,11 @@ describe('ReactComponentLifeCycle', () => {
class Component extends React.Component {
render() {
return (
<Tooltip ref="tooltip" tooltip={<div>{this.props.tooltipText}</div>}>
<Tooltip
ref={current => {
this.refs.tooltip = current;
}}
tooltip={<div>{this.props.tooltipText}</div>}>
{this.props.text}
</Tooltip>
);
Expand Down
65 changes: 55 additions & 10 deletions packages/react-dom/src/__tests__/ReactCompositeComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,19 @@ describe('ReactCompositeComponent', () => {
render() {
const toggleActivatedState = this._toggleActivatedState;
return !this.state.activated ? (
<a ref="x" onClick={toggleActivatedState} />
<a
ref={current => {
this.refs.x = current;
}}
onClick={toggleActivatedState}
/>
) : (
<b ref="x" onClick={toggleActivatedState} />
<b
ref={current => {
this.refs.x = current;
}}
onClick={toggleActivatedState}
/>
);
}
};
Expand All @@ -98,7 +108,12 @@ describe('ReactCompositeComponent', () => {
render() {
const className = this.props.anchorClassOn ? 'anchorClass' : '';
return this.props.renderAnchor ? (
<a ref="anch" className={className} />
<a
ref={current => {
this.refs.anch = current;
}}
className={className}
/>
) : (
<b />
);
Expand Down Expand Up @@ -741,8 +756,15 @@ describe('ReactCompositeComponent', () => {
class Wrapper extends React.Component {
render() {
return (
<Parent ref="parent">
<Child ref="child" />
<Parent
ref={current => {
this.refs.parent = current;
}}>
<Child
ref={current => {
this.refs.child = current;
}}
/>
</Parent>
);
}
Expand Down Expand Up @@ -1146,21 +1168,37 @@ describe('ReactCompositeComponent', () => {
if (this.props.flipped) {
return (
<div>
<Static ref="static0" key="B">
<Static
ref={current => {
this.refs.static0 = current;
}}
key="B">
B (ignored)
</Static>
<Static ref="static1" key="A">
<Static
ref={current => {
this.refs.static1 = current;
}}
key="A">
A (ignored)
</Static>
</div>
);
} else {
return (
<div>
<Static ref="static0" key="A">
<Static
ref={current => {
this.refs.static0 = current;
}}
key="A">
A
</Static>
<Static ref="static1" key="B">
<Static
ref={current => {
this.refs.static1 = current;
}}
key="B">
B
</Static>
</div>
Expand Down Expand Up @@ -1456,7 +1494,14 @@ describe('ReactCompositeComponent', () => {
}

render() {
return <Apple color={this.state.color} ref="apple" />;
return (
<Apple
color={this.state.color}
ref={current => {
this.refs.apple = current;
}}
/>
);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,14 @@ describe('ReactDOMEventListener', () => {
};

render() {
const inner = <div ref="inner">Inner</div>;
const inner = (
<div
ref={current => {
this.refs.inner = current;
}}>
Inner
</div>
);
return (
<div>
<div onMouseOut={onMouseOut} id="outer">
Expand Down
17 changes: 14 additions & 3 deletions packages/react-dom/src/__tests__/ReactDOMInput-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1075,18 +1075,29 @@ describe('ReactDOMInput', () => {
return (
<div>
<input
ref="a"
ref={current => {
this.refs.a = current;
}}
type="radio"
name="fruit"
checked={true}
onChange={emptyFunction}
/>
A
<input ref="b" type="radio" name="fruit" onChange={emptyFunction} />
<input
ref={current => {
this.refs.b = current;
}}
type="radio"
name="fruit"
onChange={emptyFunction}
/>
B
<form>
<input
ref="c"
ref={current => {
this.refs.c = current;
}}
type="radio"
name="fruit"
defaultChecked={true}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,13 @@ describe('ReactDOMServerIntegration', () => {
itRenders('no ref attribute', async render => {
class RefComponent extends React.Component {
render() {
return <div ref="foo" />;
return (
<div
ref={current => {
this.refs.foo = current;
}}
/>
);
}
}
const e = await render(<RefComponent />);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const ReactDOMServerIntegrationUtils = require('./utils/ReactDOMServerIntegratio
let React;
let ReactDOM;
let ReactDOMServer;
let ReactFeatureFlags;
let ReactTestUtils;

function initModules() {
Expand All @@ -22,6 +23,7 @@ function initModules() {
React = require('react');
ReactDOM = require('react-dom');
ReactDOMServer = require('react-dom/server');
ReactFeatureFlags = require('shared/ReactFeatureFlags');
ReactTestUtils = require('react-dom/test-utils');

// Make them available to the helpers.
Expand Down Expand Up @@ -91,10 +93,22 @@ describe('ReactDOMServerIntegration', () => {
root.innerHTML = markup;
let component = null;
resetModules();
await asyncReactDOMRender(
<RefsComponent ref={e => (component = e)} />,
root,
true,
await expect(async () => {
await asyncReactDOMRender(
<RefsComponent ref={e => (component = e)} />,
root,
true,
);
}).toErrorDev(
ReactFeatureFlags.warnAboutStringRefs
? [
'Warning: Component "RefsComponent" contains the string ref "myDiv". ' +
'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://reactjs.org/link/strict-mode-string-ref\n' +
' in RefsComponent (at **)',
]
: [],
);
expect(component.refs.myDiv).toBe(root.firstChild);
});
Expand Down
7 changes: 6 additions & 1 deletion packages/react-dom/src/__tests__/ReactIdentity-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,12 @@ describe('ReactIdentity', () => {
render() {
return (
<div>
<span ref="span" key={key} />
<span
ref={current => {
this.refs.span = current;
}}
key={key}
/>
</div>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,9 @@ class FriendsStatusDisplay extends React.Component {
!status ? null : (
<StatusDisplay
key={key}
ref={key}
ref={current => {
this.refs[key] = current;
}}
contentKey={key}
onFlush={this.verifyPreviousRefsResolved.bind(this, key)}
status={status}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,11 @@ describe('ReactDOMServerHydration', () => {

render() {
return (
<span ref="span" onClick={this.click}>
<span
ref={current => {
this.refs.span = current;
}}
onClick={this.click}>
Name: {this.props.name}
</span>
);
Expand Down
21 changes: 17 additions & 4 deletions packages/react-dom/src/__tests__/ReactTestUtils-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -224,11 +224,22 @@ describe('ReactTestUtils', () => {
class Root extends React.Component {
render() {
return (
<html ref="html">
<head ref="head">
<html
ref={current => {
this.refs.html = current;
}}>
<head
ref={current => {
this.refs.head = current;
}}>
<title>hello</title>
</head>
<body ref="body">hello, world</body>
<body
ref={current => {
this.refs.body = current;
}}>
hello, world
</body>
</html>
);
}
Expand Down Expand Up @@ -354,7 +365,9 @@ describe('ReactTestUtils', () => {
<div>
<input
type="text"
ref="input"
ref={current => {
this.refs.input = current;
}}
onChange={this.props.handleChange}
/>
</div>
Expand Down
Loading