Skip to content

Commit f9fcdc7

Browse files
eps1loneps1lon
committed
Adjust tests to expect string ref warning
1 parent ba950fc commit f9fcdc7

11 files changed

+170
-30
lines changed

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

Lines changed: 23 additions & 2 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

@@ -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,7 +135,26 @@ 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

139160
it('should not have string refs on unmounted components', () => {

‎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__/refs-test.js‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
let React = require('react');
1313
let ReactDOM = require('react-dom');
1414
let ReactTestUtils = require('react-dom/test-utils');
15+
let ReactFeatureFlags = require('shared/ReactFeatureFlags');
1516

1617
/**
1718
* Counts clicks and has a renders an item for each click. Each item rendered
@@ -108,6 +109,7 @@ describe('reactiverefs', () => {
108109
React = require('react');
109110
ReactDOM = require('react-dom');
110111
ReactTestUtils = require('react-dom/test-utils');
112+
ReactFeatureFlags = require('shared/ReactFeatureFlags');
111113
});
112114

113115
afterEach(() => {
@@ -355,7 +357,20 @@ describe('ref swapping', () => {
355357
return <div ref={1} />;
356358
}
357359
}
358-
const a = ReactTestUtils.renderIntoDocument(<A />);
360+
let a;
361+
expect(() => {
362+
a = ReactTestUtils.renderIntoDocument(<A />);
363+
}).toErrorDev(
364+
ReactFeatureFlags.warnAboutStringRefs
365+
? [
366+
'Warning: Component "A" contains the string ref "1". ' +
367+
'Support for string refs will be removed in a future major release. ' +
368+
'We recommend using useRef() or createRef() instead. ' +
369+
'Learn more about using refs safely here: https://reactjs.org/link/strict-mode-string-ref\n' +
370+
' in A (at **)',
371+
]
372+
: [],
373+
);
359374
expect(a.refs[1].nodeName).toBe('DIV');
360375
});
361376

‎packages/react-reconciler/src/ReactChildFiber.new.js‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,10 @@ if (__DEV__) {
9898
};
9999
}
100100

101+
function isReactClass(type) {
102+
return type.prototype && type.prototype.isReactComponent;
103+
}
104+
101105
function coerceRef(
102106
returnFiber: Fiber,
103107
current: Fiber | null,
@@ -121,7 +125,16 @@ function coerceRef(
121125
element._owner &&
122126
element._self &&
123127
element._owner.stateNode !== element._self
124-
)
128+
) &&
129+
// Will already throw with "Function components cannot have string refs"
130+
!(
131+
element._owner &&
132+
((element._owner: any): Fiber).tag !== ClassComponent
133+
) &&
134+
// Will already warn with "Function components cannot be given refs"
135+
!(typeof element.type === 'function' && !isReactClass(element.type)) &&
136+
// Will already throw with "Element ref was specified as a string (someStringRef) but no owner was set"
137+
element._owner
125138
) {
126139
const componentName =
127140
getComponentNameFromFiber(returnFiber) || 'Component';

‎packages/react-reconciler/src/ReactChildFiber.old.js‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,10 @@ if (__DEV__) {
9898
};
9999
}
100100

101+
function isReactClass(type) {
102+
return type.prototype && type.prototype.isReactComponent;
103+
}
104+
101105
function coerceRef(
102106
returnFiber: Fiber,
103107
current: Fiber | null,
@@ -121,7 +125,16 @@ function coerceRef(
121125
element._owner &&
122126
element._self &&
123127
element._owner.stateNode !== element._self
124-
)
128+
) &&
129+
// Will already throw with "Function components cannot have string refs"
130+
!(
131+
element._owner &&
132+
((element._owner: any): Fiber).tag !== ClassComponent
133+
) &&
134+
// Will already warn with "Function components cannot be given refs"
135+
!(typeof element.type === 'function' && !isReactClass(element.type)) &&
136+
// Will already throw with "Element ref was specified as a string (someStringRef) but no owner was set"
137+
element._owner
125138
) {
126139
const componentName =
127140
getComponentNameFromFiber(returnFiber) || 'Component';
@@ -256,7 +269,6 @@ function coerceRef(
256269
}
257270
}
258271
}
259-
260272
return mixedRef;
261273
}
262274

‎packages/react-reconciler/src/__tests__/ReactIncrementalSideEffects-test.js‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
'use strict';
1212

1313
let React;
14+
let ReactFeatureFlags;
1415
let ReactNoop;
1516
let Scheduler;
1617

@@ -19,6 +20,7 @@ describe('ReactIncrementalSideEffects', () => {
1920
jest.resetModules();
2021

2122
React = require('react');
23+
ReactFeatureFlags = require('shared/ReactFeatureFlags');
2224
ReactNoop = require('react-noop-renderer');
2325
Scheduler = require('scheduler');
2426
});
@@ -1306,8 +1308,19 @@ describe('ReactIncrementalSideEffects', () => {
13061308
}
13071309

13081310
ReactNoop.render(<Foo />);
1309-
expect(Scheduler).toFlushWithoutYielding();
1310-
1311+
expect(() => {
1312+
expect(Scheduler).toFlushWithoutYielding();
1313+
}).toErrorDev(
1314+
ReactFeatureFlags.warnAboutStringRefs
1315+
? [
1316+
'Warning: Component "Foo" contains the string ref "bar". ' +
1317+
'Support for string refs will be removed in a future major release. ' +
1318+
'We recommend using useRef() or createRef() instead. ' +
1319+
'Learn more about using refs safely here: https://reactjs.org/link/strict-mode-string-ref\n' +
1320+
' in Foo (at **)',
1321+
]
1322+
: [],
1323+
);
13111324
expect(fooInstance.refs.bar.test).toEqual('test');
13121325
});
13131326
});

‎packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ PropTypes = null
99
React = null
1010
ReactDOM = null
1111
ReactDOMClient = null
12+
ReactFeatureFlags = null
1213
act = null
1314

1415
describe 'ReactCoffeeScriptClass', ->
@@ -22,6 +23,7 @@ describe 'ReactCoffeeScriptClass', ->
2223
React = require 'react'
2324
ReactDOM = require 'react-dom'
2425
ReactDOMClient = require 'react-dom/client'
26+
ReactFeatureFlags = require 'shared/ReactFeatureFlags'
2527
act = require('jest-react').act
2628
PropTypes = require 'prop-types'
2729
container = document.createElement 'div'
@@ -537,7 +539,19 @@ describe 'ReactCoffeeScriptClass', ->
537539
)
538540

539541
ref = React.createRef()
540-
test(React.createElement(Foo, ref: ref), 'DIV', 'foo')
542+
expect(->
543+
test(React.createElement(Foo, ref: ref), 'DIV', 'foo')
544+
).toErrorDev(
545+
if ReactFeatureFlags.warnAboutStringRefs
546+
then [
547+
'Warning: Component "Foo" contains the string ref "inner". ' +
548+
'Support for string refs will be removed in a future major release. ' +
549+
'We recommend using useRef() or createRef() instead. ' +
550+
'Learn more about using refs safely here: https://reactjs.org/link/strict-mode-string-ref\n' +
551+
' in Foo (at **)'
552+
]
553+
else []
554+
);
541555
expect(ref.current.refs.inner.getName()).toBe 'foo'
542556

543557
it 'supports drilling through to the DOM using findDOMNode', ->

‎packages/react/src/__tests__/ReactES6Class-test.js‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ let PropTypes;
1313
let React;
1414
let ReactDOM;
1515
let ReactDOMClient;
16+
let ReactFeatureFlags;
1617
let act;
1718

1819
describe('ReactES6Class', () => {
@@ -31,6 +32,7 @@ describe('ReactES6Class', () => {
3132
React = require('react');
3233
ReactDOM = require('react-dom');
3334
ReactDOMClient = require('react-dom/client');
35+
ReactFeatureFlags = require('shared/ReactFeatureFlags');
3436
act = require('jest-react').act;
3537
container = document.createElement('div');
3638
root = ReactDOMClient.createRoot(container);
@@ -575,7 +577,19 @@ describe('ReactES6Class', () => {
575577
}
576578
}
577579
const ref = React.createRef();
578-
test(<Foo ref={ref} />, 'DIV', 'foo');
580+
expect(() => {
581+
test(<Foo ref={ref} />, 'DIV', 'foo');
582+
}).toErrorDev(
583+
ReactFeatureFlags.warnAboutStringRefs
584+
? [
585+
'Warning: Component "Foo" contains the string ref "inner". ' +
586+
'Support for string refs will be removed in a future major release. ' +
587+
'We recommend using useRef() or createRef() instead. ' +
588+
'Learn more about using refs safely here: https://reactjs.org/link/strict-mode-string-ref\n' +
589+
' in Foo (at **)',
590+
]
591+
: [],
592+
);
579593
expect(ref.current.refs.inner.getName()).toBe('foo');
580594
});
581595

‎packages/react/src/__tests__/ReactProfilerDevToolsIntegration-test.internal.js‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,6 @@ describe('ReactProfiler DevTools integration', () => {
101101
);
102102
});
103103

104-
// FIXME: What would throw in a host root apart from string refs without owner?
105-
// @gate !warnAboutStringRefs
106104
it('should reset the fiber stack correctly after an error when profiling host roots', () => {
107105
Scheduler.unstable_advanceTime(20);
108106

‎packages/react/src/__tests__/ReactStrictMode-test.js‎

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -741,12 +741,18 @@ describe('string refs', () => {
741741
expect(() => {
742742
ReactDOM.render(<OuterComponent />, container);
743743
}).toErrorDev(
744-
'Warning: A string ref, "somestring", has been found within a strict mode tree. ' +
745-
'String refs are a source of potential bugs and should be avoided. ' +
746-
'We recommend using useRef() or createRef() instead. ' +
747-
'Learn more about using refs safely here: ' +
748-
'https://reactjs.org/link/strict-mode-string-ref\n' +
749-
' in OuterComponent (at **)',
744+
ReactFeatureFlags.warnAboutStringRefs
745+
? 'Warning: Component "StrictMode" contains the string ref "somestring". ' +
746+
'Support for string refs will be removed in a future major release. ' +
747+
'We recommend using useRef() or createRef() instead. ' +
748+
'Learn more about using refs safely here: https://reactjs.org/link/strict-mode-string-ref\n' +
749+
' in OuterComponent (at **)'
750+
: 'Warning: A string ref, "somestring", has been found within a strict mode tree. ' +
751+
'String refs are a source of potential bugs and should be avoided. ' +
752+
'We recommend using useRef() or createRef() instead. ' +
753+
'Learn more about using refs safely here: ' +
754+
'https://reactjs.org/link/strict-mode-string-ref\n' +
755+
' in OuterComponent (at **)',
750756
);
751757

752758
// Dedup
@@ -782,13 +788,20 @@ describe('string refs', () => {
782788
expect(() => {
783789
ReactDOM.render(<OuterComponent />, container);
784790
}).toErrorDev(
785-
'Warning: A string ref, "somestring", has been found within a strict mode tree. ' +
786-
'String refs are a source of potential bugs and should be avoided. ' +
787-
'We recommend using useRef() or createRef() instead. ' +
788-
'Learn more about using refs safely here: ' +
789-
'https://reactjs.org/link/strict-mode-string-ref\n' +
790-
' in InnerComponent (at **)\n' +
791-
' in OuterComponent (at **)',
791+
ReactFeatureFlags.warnAboutStringRefs
792+
? 'Warning: Component "InnerComponent" contains the string ref "somestring". ' +
793+
'Support for string refs will be removed in a future major release. ' +
794+
'We recommend using useRef() or createRef() instead. ' +
795+
'Learn more about using refs safely here: https://reactjs.org/link/strict-mode-string-ref\n' +
796+
' in InnerComponent (at **)\n' +
797+
' in OuterComponent (at **)'
798+
: 'Warning: A string ref, "somestring", has been found within a strict mode tree. ' +
799+
'String refs are a source of potential bugs and should be avoided. ' +
800+
'We recommend using useRef() or createRef() instead. ' +
801+
'Learn more about using refs safely here: ' +
802+
'https://reactjs.org/link/strict-mode-string-ref\n' +
803+
' in InnerComponent (at **)\n' +
804+
' in OuterComponent (at **)',
792805
);
793806

794807
// Dedup

0 commit comments

Comments
 (0)