Skip to content

Commit c4f0b93

Browse files
lunaruanlunaruan
authored
Warn when Using String Refs (#16217)
1 parent 7c838a6 commit c4f0b93

10 files changed

Lines changed: 77 additions & 23 deletions

‎packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.internal.js‎

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,20 +10,24 @@
1010
'use strict';
1111

1212
letReact;
13-
letReactTestUtils;
1413
letReactFeatureFlags;
14+
letReactNoop;
15+
letScheduler;
1516

1617
describe('ReactDeprecationWarnings',()=>{
1718
beforeEach(()=>{
1819
jest.resetModules();
1920
React=require('react');
2021
ReactFeatureFlags=require('shared/ReactFeatureFlags');
21-
ReactTestUtils=require('react-dom/test-utils');
22+
ReactNoop=require('react-noop-renderer');
23+
Scheduler=require('scheduler');
2224
ReactFeatureFlags.warnAboutDefaultPropsOnFunctionComponents=true;
25+
ReactFeatureFlags.warnAboutStringRefs=true;
2326
});
2427

2528
afterEach(()=>{
2629
ReactFeatureFlags.warnAboutDefaultPropsOnFunctionComponents=false;
30+
ReactFeatureFlags.warnAboutStringRefs=false;
2731
});
2832

2933
it('should warn when given defaultProps',()=>{
@@ -35,13 +39,37 @@ describe('ReactDeprecationWarnings', () => {
3539
testProp: true,
3640
};
3741

38-
expect(()=>
39-
ReactTestUtils.renderIntoDocument(<FunctionalComponent/>),
40-
).toWarnDev(
42+
ReactNoop.render(<FunctionalComponent/>);
43+
expect(()=>expect(Scheduler).toFlushWithoutYielding()).toWarnDev(
4144
'Warning: FunctionalComponent: Support for defaultProps '+
4245
'will be removed from function components in a future major '+
4346
'release. Use JavaScript default parameters instead.',
4447
{withoutStack: true},
4548
);
4649
});
50+
51+
it('should warn when given string refs',()=>{
52+
classRefComponentextendsReact.Component{
53+
render(){
54+
returnnull;
55+
}
56+
}
57+
classComponentextendsReact.Component{
58+
render(){
59+
return<RefComponentref="refComponent"/>;
60+
}
61+
}
62+
63+
ReactNoop.render(<Component/>);
64+
expect(()=>expect(Scheduler).toFlushWithoutYielding()).toWarnDev(
65+
'Warning: Component "Component" contains the string ref "refComponent". '+
66+
'Support for string refs will be removed in a future major release. '+
67+
'We recommend using useRef() or createRef() instead.'+
68+
'\n\n'+
69+
' in Component (at **)'+
70+
'\n\n'+
71+
'Learn more about using refs safely here:\n'+
72+
'https://fb.me/react-strict-mode-string-ref',
73+
);
74+
});
4775
});

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

Lines changed: 34 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import {
3030
importinvariantfrom'shared/invariant';
3131
importwarningfrom'shared/warning';
3232
importwarningWithoutStackfrom'shared/warningWithoutStack';
33+
import{warnAboutStringRefs}from'shared/ReactFeatureFlags';
3334

3435
import{
3536
createWorkInProgress,
@@ -49,15 +50,15 @@ import {StrictMode} from './ReactTypeOfMode';
4950

5051
letdidWarnAboutMaps;
5152
letdidWarnAboutGenerators;
52-
letdidWarnAboutStringRefInStrictMode;
53+
letdidWarnAboutStringRefs;
5354
letownerHasKeyUseWarning;
5455
letownerHasFunctionTypeWarning;
5556
letwarnForMissingKey=(child: mixed)=>{};
5657

5758
if(__DEV__){
5859
didWarnAboutMaps=false;
5960
didWarnAboutGenerators=false;
60-
didWarnAboutStringRefInStrictMode={};
61+
didWarnAboutStringRefs={};
6162

6263
/**
6364
* Warn if there's no key explicitly set on dynamic arrays of children or
@@ -114,21 +115,38 @@ function coerceRef(
114115
typeofmixedRef!=='object'
115116
){
116117
if(__DEV__){
117-
if(returnFiber.mode&StrictMode){
118+
// TODO: Clean this up once we turn on the string ref warning for
119+
// everyone, because the strict mode case will no longer be relevant
120+
if(returnFiber.mode&StrictMode||warnAboutStringRefs){
118121
constcomponentName=getComponentName(returnFiber.type)||'Component';
119-
if(!didWarnAboutStringRefInStrictMode[componentName]){
120-
warningWithoutStack(
121-
false,
122-
'A string ref, "%s", has been found within a strict mode tree. '+
123-
'String refs are a source of potential bugs and should be avoided. '+
124-
'We recommend using createRef() instead.'+
125-
'\n%s'+
126-
'\n\nLearn more about using refs safely here:'+
127-
'\nhttps://fb.me/react-strict-mode-string-ref',
128-
mixedRef,
129-
getStackByFiberInDevAndProd(returnFiber),
130-
);
131-
didWarnAboutStringRefInStrictMode[componentName]=true;
122+
if(!didWarnAboutStringRefs[componentName]){
123+
if(warnAboutStringRefs){
124+
warningWithoutStack(
125+
false,
126+
'Component "%s" contains the string ref "%s". Support for string refs '+
127+
'will be removed in a future major release. We recommend using '+
128+
'useRef() or createRef() instead.'+
129+
'\n%s'+
130+
'\n\nLearn more about using refs safely here:'+
131+
'\nhttps://fb.me/react-strict-mode-string-ref',
132+
componentName,
133+
mixedRef,
134+
getStackByFiberInDevAndProd(returnFiber),
135+
);
136+
}else{
137+
warningWithoutStack(
138+
false,
139+
'A string ref, "%s", has been found within a strict mode tree. '+
140+
'String refs are a source of potential bugs and should be avoided. '+
141+
'We recommend using useRef() or createRef() instead.'+
142+
'\n%s'+
143+
'\n\nLearn more about using refs safely here:'+
144+
'\nhttps://fb.me/react-strict-mode-string-ref',
145+
mixedRef,
146+
getStackByFiberInDevAndProd(returnFiber),
147+
);
148+
}
149+
didWarnAboutStringRefs[componentName]=true;
132150
}
133151
}
134152
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -693,7 +693,7 @@ Please update the following components: Parent`,
693693
}).toWarnDev(
694694
'Warning: A string ref, "somestring", has been found within a strict mode tree. '+
695695
'String refs are a source of potential bugs and should be avoided. '+
696-
'We recommend using createRef() instead.\n\n'+
696+
'We recommend using useRef() or createRef() instead.\n\n'+
697697
' in StrictMode (at **)\n'+
698698
' in OuterComponent (at **)\n\n'+
699699
'Learn more about using refs safely here:\n'+
@@ -735,7 +735,7 @@ Please update the following components: Parent`,
735735
}).toWarnDev(
736736
'Warning: A string ref, "somestring", has been found within a strict mode tree. '+
737737
'String refs are a source of potential bugs and should be avoided. '+
738-
'We recommend using createRef() instead.\n\n'+
738+
'We recommend using useRef() or createRef() instead.\n\n'+
739739
' in InnerComponent (at **)\n'+
740740
' in StrictMode (at **)\n'+
741741
' in OuterComponent (at **)\n\n'+

‎packages/shared/ReactFeatureFlags.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ export const enableSuspenseCallback = false;
9292
// from React.createElement to React.jsx
9393
// https://github.com/reactjs/rfcs/blob/createlement-rfc/text/0000-create-element-changes.md
9494
exportconstwarnAboutDefaultPropsOnFunctionComponents=false;
95+
exportconstwarnAboutStringRefs=false;
9596

9697
exportconstdisableLegacyContext=false;
9798

‎packages/shared/forks/ReactFeatureFlags.native-fb.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ export const flushSuspenseFallbacksInTests = true;
4040
exportconstenableUserBlockingEvents=false;
4141
exportconstenableSuspenseCallback=false;
4242
exportconstwarnAboutDefaultPropsOnFunctionComponents=false;
43+
exportconstwarnAboutStringRefs=false;
4344
exportconstdisableLegacyContext=false;
4445
exportconstdisableSchedulerTimeoutBasedOnReactExpirationTime=false;
4546

‎packages/shared/forks/ReactFeatureFlags.native-oss.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ export const flushSuspenseFallbacksInTests = true;
3535
exportconstenableUserBlockingEvents=false;
3636
exportconstenableSuspenseCallback=false;
3737
exportconstwarnAboutDefaultPropsOnFunctionComponents=false;
38+
exportconstwarnAboutStringRefs=false;
3839
exportconstdisableLegacyContext=false;
3940
exportconstdisableSchedulerTimeoutBasedOnReactExpirationTime=false;
4041

‎packages/shared/forks/ReactFeatureFlags.persistent.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ export const flushSuspenseFallbacksInTests = true;
3535
exportconstenableUserBlockingEvents=false;
3636
exportconstenableSuspenseCallback=false;
3737
exportconstwarnAboutDefaultPropsOnFunctionComponents=false;
38+
exportconstwarnAboutStringRefs=false;
3839
exportconstdisableLegacyContext=false;
3940
exportconstdisableSchedulerTimeoutBasedOnReactExpirationTime=false;
4041

‎packages/shared/forks/ReactFeatureFlags.test-renderer.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ export const flushSuspenseFallbacksInTests = true;
3535
exportconstenableUserBlockingEvents=false;
3636
exportconstenableSuspenseCallback=false;
3737
exportconstwarnAboutDefaultPropsOnFunctionComponents=false;
38+
exportconstwarnAboutStringRefs=false;
3839
exportconstdisableLegacyContext=false;
3940
exportconstdisableSchedulerTimeoutBasedOnReactExpirationTime=false;
4041

‎packages/shared/forks/ReactFeatureFlags.test-renderer.www.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ export const flushSuspenseFallbacksInTests = true;
3535
exportconstenableUserBlockingEvents=false;
3636
exportconstenableSuspenseCallback=true;
3737
exportconstwarnAboutDefaultPropsOnFunctionComponents=false;
38+
exportconstwarnAboutStringRefs=false;
3839
exportconstdisableLegacyContext=false;
3940
exportconstdisableSchedulerTimeoutBasedOnReactExpirationTime=false;
4041

‎packages/shared/forks/ReactFeatureFlags.www.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,8 @@ export const enableSuspenseCallback = true;
8282

8383
exportconstwarnAboutDefaultPropsOnFunctionComponents=false;
8484

85+
exportconstwarnAboutStringRefs=false;
86+
8587
exportconstflushSuspenseFallbacksInTests=true;
8688

8789
// Flow magic to verify the exports of this file match the original version.

0 commit comments

Comments
 (0)