Skip to content

Commit c2a246e

Browse files
eps1lonacdlite
eps1lon
authored and
acdlite
committed
Turn on string ref deprecation warning for everybody (not codemoddable) (#25383)
## Summary Alternate to #25334 without any prod runtime changes i.e. the proposed codemod in https://github.com/reactjs/rfcs/blob/createlement-rfc/text/0000-create-element-changes.md#deprecate-string-refs-and-remove-production-mode-_owner-field would not work. ## How did you test this change? - [x] CI - [x] `yarn test` with and without `warnAboutStringRefs`
1 parent 2cfb474 commit c2a246e

37 files changed

+591
-300
lines changed

‎packages/react-dom/src/__tests__/ReactComponent-test.js

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
let React;
1313
let ReactDOM;
1414
let ReactDOMServer;
15+
let ReactFeatureFlags;
1516
let ReactTestUtils;
1617

1718
describe('ReactComponent', () => {
@@ -21,6 +22,7 @@ describe('ReactComponent', () => {
2122
React = require('react');
2223
ReactDOM = require('react-dom');
2324
ReactDOMServer = require('react-dom/server');
25+
ReactFeatureFlags = require('shared/ReactFeatureFlags');
2426
ReactTestUtils = require('react-dom/test-utils');
2527
});
2628

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

39-
it('should throw when supplying a ref outside of render method', () => {
41+
it('should throw when supplying a string ref outside of render method', () => {
4042
let instance = <div ref="badDiv" />;
4143
expect(function() {
4244
instance = ReactTestUtils.renderIntoDocument(instance);
@@ -102,7 +104,7 @@ describe('ReactComponent', () => {
102104
}
103105
});
104106

105-
it('should support refs on owned components', () => {
107+
it('should support string refs on owned components', () => {
106108
const innerObj = {};
107109
const outerObj = {};
108110

@@ -133,10 +135,29 @@ describe('ReactComponent', () => {
133135
}
134136
}
135137

136-
ReactTestUtils.renderIntoDocument(<Component />);
138+
expect(() => {
139+
ReactTestUtils.renderIntoDocument(<Component />);
140+
}).toErrorDev(
141+
ReactFeatureFlags.warnAboutStringRefs
142+
? [
143+
'Warning: Component "div" contains the string ref "inner". ' +
144+
'Support for string refs will be removed in a future major release. ' +
145+
'We recommend using useRef() or createRef() instead. ' +
146+
'Learn more about using refs safely here: https://reactjs.org/link/strict-mode-string-ref\n' +
147+
' in div (at **)\n' +
148+
' in Wrapper (at **)\n' +
149+
' in Component (at **)',
150+
'Warning: Component "Component" contains the string ref "outer". ' +
151+
'Support for string refs will be removed in a future major release. ' +
152+
'We recommend using useRef() or createRef() instead. ' +
153+
'Learn more about using refs safely here: https://reactjs.org/link/strict-mode-string-ref\n' +
154+
' in Component (at **)',
155+
]
156+
: [],
157+
);
137158
});
138159

139-
it('should not have refs on unmounted components', () => {
160+
it('should not have string refs on unmounted components', () => {
140161
class Parent extends React.Component {
141162
render() {
142163
return (

‎packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -378,7 +378,7 @@ describe('ReactComponentLifeCycle', () => {
378378
}
379379
// you would *NEVER* do anything like this in real code!
380380
this.state.hasRenderCompleted = true;
381-
return <div ref="theDiv">I am the inner DIV</div>;
381+
return <div ref={React.createRef()}>I am the inner DIV</div>;
382382
}
383383

384384
componentWillUnmount() {
@@ -477,7 +477,9 @@ describe('ReactComponentLifeCycle', () => {
477477
class Component extends React.Component {
478478
render() {
479479
return (
480-
<Tooltip ref="tooltip" tooltip={<div>{this.props.tooltipText}</div>}>
480+
<Tooltip
481+
ref={React.createRef()}
482+
tooltip={<div>{this.props.tooltipText}</div>}>
481483
{this.props.text}
482484
</Tooltip>
483485
);

‎packages/react-dom/src/__tests__/ReactCompositeComponent-test.js

Lines changed: 37 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -72,16 +72,18 @@ describe('ReactCompositeComponent', () => {
7272
MorphingComponent = class extends React.Component {
7373
state = {activated: false};
7474

75+
xRef = React.createRef();
76+
7577
_toggleActivatedState = () => {
7678
this.setState({activated: !this.state.activated});
7779
};
7880

7981
render() {
8082
const toggleActivatedState = this._toggleActivatedState;
8183
return !this.state.activated ? (
82-
<a ref="x" onClick={toggleActivatedState} />
84+
<a ref={this.xRef} onClick={toggleActivatedState} />
8385
) : (
84-
<b ref="x" onClick={toggleActivatedState} />
86+
<b ref={this.xRef} onClick={toggleActivatedState} />
8587
);
8688
}
8789
};
@@ -91,14 +93,16 @@ describe('ReactCompositeComponent', () => {
9193
* reallocated again.
9294
*/
9395
ChildUpdates = class extends React.Component {
96+
anchorRef = React.createRef();
97+
9498
getAnchor = () => {
95-
return this.refs.anch;
99+
return this.anchorRef.current;
96100
};
97101

98102
render() {
99103
const className = this.props.anchorClassOn ? 'anchorClass' : '';
100104
return this.props.renderAnchor ? (
101-
<a ref="anch" className={className} />
105+
<a ref={this.anchorRef} className={className} />
102106
) : (
103107
<b />
104108
);
@@ -186,11 +190,11 @@ describe('ReactCompositeComponent', () => {
186190
it('should rewire refs when rendering to different child types', () => {
187191
const instance = ReactTestUtils.renderIntoDocument(<MorphingComponent />);
188192

189-
expect(instance.refs.x.tagName).toBe('A');
193+
expect(instance.xRef.current.tagName).toBe('A');
190194
instance._toggleActivatedState();
191-
expect(instance.refs.x.tagName).toBe('B');
195+
expect(instance.xRef.current.tagName).toBe('B');
192196
instance._toggleActivatedState();
193-
expect(instance.refs.x.tagName).toBe('A');
197+
expect(instance.xRef.current.tagName).toBe('A');
194198
});
195199

196200
it('should not cache old DOM nodes when switching constructors', () => {
@@ -739,25 +743,28 @@ describe('ReactCompositeComponent', () => {
739743
}
740744

741745
class Wrapper extends React.Component {
746+
parentRef = React.createRef();
747+
childRef = React.createRef();
748+
742749
render() {
743750
return (
744-
<Parent ref="parent">
745-
<Child ref="child" />
751+
<Parent ref={this.parentRef}>
752+
<Child ref={this.childRef} />
746753
</Parent>
747754
);
748755
}
749756
}
750757

751758
const wrapper = ReactTestUtils.renderIntoDocument(<Wrapper />);
752759

753-
expect(wrapper.refs.parent.state.flag).toEqual(true);
754-
expect(wrapper.refs.child.context).toEqual({flag: true});
760+
expect(wrapper.parentRef.current.state.flag).toEqual(true);
761+
expect(wrapper.childRef.current.context).toEqual({flag: true});
755762

756763
// We update <Parent /> while <Child /> is still a static prop relative to this update
757-
wrapper.refs.parent.setState({flag: false});
764+
wrapper.parentRef.current.setState({flag: false});
758765

759-
expect(wrapper.refs.parent.state.flag).toEqual(false);
760-
expect(wrapper.refs.child.context).toEqual({flag: false});
766+
expect(wrapper.parentRef.current.state.flag).toEqual(false);
767+
expect(wrapper.childRef.current.context).toEqual({flag: false});
761768
});
762769

763770
it('should pass context transitively', () => {
@@ -1142,25 +1149,28 @@ describe('ReactCompositeComponent', () => {
11421149
}
11431150

11441151
class Component extends React.Component {
1152+
static0Ref = React.createRef();
1153+
static1Ref = React.createRef();
1154+
11451155
render() {
11461156
if (this.props.flipped) {
11471157
return (
11481158
<div>
1149-
<Static ref="static0" key="B">
1159+
<Static ref={this.static0Ref} key="B">
11501160
B (ignored)
11511161
</Static>
1152-
<Static ref="static1" key="A">
1162+
<Static ref={this.static1Ref} key="A">
11531163
A (ignored)
11541164
</Static>
11551165
</div>
11561166
);
11571167
} else {
11581168
return (
11591169
<div>
1160-
<Static ref="static0" key="A">
1170+
<Static ref={this.static0Ref} key="A">
11611171
A
11621172
</Static>
1163-
<Static ref="static1" key="B">
1173+
<Static ref={this.static1Ref} key="B">
11641174
B
11651175
</Static>
11661176
</div>
@@ -1171,14 +1181,14 @@ describe('ReactCompositeComponent', () => {
11711181

11721182
const container = document.createElement('div');
11731183
const comp = ReactDOM.render(<Component flipped={false} />, container);
1174-
expect(ReactDOM.findDOMNode(comp.refs.static0).textContent).toBe('A');
1175-
expect(ReactDOM.findDOMNode(comp.refs.static1).textContent).toBe('B');
1184+
expect(ReactDOM.findDOMNode(comp.static0Ref.current).textContent).toBe('A');
1185+
expect(ReactDOM.findDOMNode(comp.static1Ref.current).textContent).toBe('B');
11761186

11771187
// When flipping the order, the refs should update even though the actual
11781188
// contents do not
11791189
ReactDOM.render(<Component flipped={true} />, container);
1180-
expect(ReactDOM.findDOMNode(comp.refs.static0).textContent).toBe('B');
1181-
expect(ReactDOM.findDOMNode(comp.refs.static1).textContent).toBe('A');
1190+
expect(ReactDOM.findDOMNode(comp.static0Ref.current).textContent).toBe('B');
1191+
expect(ReactDOM.findDOMNode(comp.static1Ref.current).textContent).toBe('A');
11821192
});
11831193

11841194
it('should allow access to findDOMNode in componentWillUnmount', () => {
@@ -1453,10 +1463,11 @@ describe('ReactCompositeComponent', () => {
14531463
this.state = {
14541464
color: 'green',
14551465
};
1466+
this.appleRef = React.createRef();
14561467
}
14571468

14581469
render() {
1459-
return <Apple color={this.state.color} ref="apple" />;
1470+
return <Apple color={this.state.color} ref={this.appleRef} />;
14601471
}
14611472
}
14621473

@@ -1502,15 +1513,15 @@ describe('ReactCompositeComponent', () => {
15021513
expect(renderCalls).toBe(2);
15031514

15041515
// Re-render base on state
1505-
instance.refs.apple.cut();
1516+
instance.appleRef.current.cut();
15061517
expect(renderCalls).toBe(3);
15071518

15081519
// No re-render based on state
1509-
instance.refs.apple.cut();
1520+
instance.appleRef.current.cut();
15101521
expect(renderCalls).toBe(3);
15111522

15121523
// Re-render based on state again
1513-
instance.refs.apple.eatSlice();
1524+
instance.appleRef.current.eatSlice();
15141525
expect(renderCalls).toBe(4);
15151526
});
15161527

‎packages/react-dom/src/__tests__/ReactDOMEventListener-test.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -193,12 +193,13 @@ describe('ReactDOMEventListener', () => {
193193
const onMouseOut = event => mouseOut(event.target);
194194

195195
class Wrapper extends React.Component {
196+
innerRef = React.createRef();
196197
getInner = () => {
197-
return this.refs.inner;
198+
return this.innerRef.current;
198199
};
199200

200201
render() {
201-
const inner = <div ref="inner">Inner</div>;
202+
const inner = <div ref={this.innerRef}>Inner</div>;
202203
return (
203204
<div>
204205
<div onMouseOut={onMouseOut} id="outer">

‎packages/react-dom/src/__tests__/ReactDOMInput-test.js

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1071,22 +1071,31 @@ describe('ReactDOMInput', () => {
10711071

10721072
it('should control radio buttons', () => {
10731073
class RadioGroup extends React.Component {
1074+
aRef = React.createRef();
1075+
bRef = React.createRef();
1076+
cRef = React.createRef();
1077+
10741078
render() {
10751079
return (
10761080
<div>
10771081
<input
1078-
ref="a"
1082+
ref={this.aRef}
10791083
type="radio"
10801084
name="fruit"
10811085
checked={true}
10821086
onChange={emptyFunction}
10831087
/>
10841088
A
1085-
<input ref="b" type="radio" name="fruit" onChange={emptyFunction} />
1089+
<input
1090+
ref={this.bRef}
1091+
type="radio"
1092+
name="fruit"
1093+
onChange={emptyFunction}
1094+
/>
10861095
B
10871096
<form>
10881097
<input
1089-
ref="c"
1098+
ref={this.cRef}
10901099
type="radio"
10911100
name="fruit"
10921101
defaultChecked={true}
@@ -1099,9 +1108,9 @@ describe('ReactDOMInput', () => {
10991108
}
11001109

11011110
const stub = ReactDOM.render(<RadioGroup />, container);
1102-
const aNode = stub.refs.a;
1103-
const bNode = stub.refs.b;
1104-
const cNode = stub.refs.c;
1111+
const aNode = stub.aRef.current;
1112+
const bNode = stub.bRef.current;
1113+
const cNode = stub.cRef.current;
11051114

11061115
expect(aNode.checked).toBe(true);
11071116
expect(bNode.checked).toBe(false);

‎packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,7 @@ describe('ReactDOMServerIntegration', () => {
337337
itRenders('no ref attribute', async render => {
338338
class RefComponent extends React.Component {
339339
render() {
340-
return <div ref="foo" />;
340+
return <div ref={React.createRef()} />;
341341
}
342342
}
343343
const e = await render(<RefComponent />);

‎packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ const ReactDOMServerIntegrationUtils = require('./utils/ReactDOMServerIntegratio
1414
let React;
1515
let ReactDOM;
1616
let ReactDOMServer;
17+
let ReactFeatureFlags;
1718
let ReactTestUtils;
1819

1920
function initModules() {
@@ -22,6 +23,7 @@ function initModules() {
2223
React = require('react');
2324
ReactDOM = require('react-dom');
2425
ReactDOMServer = require('react-dom/server');
26+
ReactFeatureFlags = require('shared/ReactFeatureFlags');
2527
ReactTestUtils = require('react-dom/test-utils');
2628

2729
// Make them available to the helpers.
@@ -91,10 +93,22 @@ describe('ReactDOMServerIntegration', () => {
9193
root.innerHTML = markup;
9294
let component = null;
9395
resetModules();
94-
await asyncReactDOMRender(
95-
<RefsComponent ref={e => (component = e)} />,
96-
root,
97-
true,
96+
await expect(async () => {
97+
await asyncReactDOMRender(
98+
<RefsComponent ref={e => (component = e)} />,
99+
root,
100+
true,
101+
);
102+
}).toErrorDev(
103+
ReactFeatureFlags.warnAboutStringRefs
104+
? [
105+
'Warning: Component "RefsComponent" contains the string ref "myDiv". ' +
106+
'Support for string refs will be removed in a future major release. ' +
107+
'We recommend using useRef() or createRef() instead. ' +
108+
'Learn more about using refs safely here: https://reactjs.org/link/strict-mode-string-ref\n' +
109+
' in RefsComponent (at **)',
110+
]
111+
: [],
98112
);
99113
expect(component.refs.myDiv).toBe(root.firstChild);
100114
});

‎packages/react-dom/src/__tests__/ReactIdentity-test.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,17 +67,18 @@ describe('ReactIdentity', () => {
6767

6868
function renderAComponentWithKeyIntoContainer(key, container) {
6969
class Wrapper extends React.Component {
70+
spanRef = React.createRef();
7071
render() {
7172
return (
7273
<div>
73-
<span ref="span" key={key} />
74+
<span ref={this.spanRef} key={key} />
7475
</div>
7576
);
7677
}
7778
}
7879

7980
const instance = ReactDOM.render(<Wrapper />, container);
80-
const span = instance.refs.span;
81+
const span = instance.spanRef.current;
8182
expect(span).not.toBe(null);
8283
}
8384

0 commit comments

Comments
 (0)