Warn when Using String Refs (#16217)
lunaruan committed
Aug 7, 2019 at 00:10 UTC
c4f0b93703be3d165de78db7a381d6cca2203b2f
10 files changed
+77
-23
packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.internal.js
+33
-5
@@ -10,20 +10,24 @@
10
'use strict';
11
12
let React;
13
-let ReactTestUtils;
13
let ReactFeatureFlags;
14
+let ReactNoop;
15
+let Scheduler;
16
17
describe('ReactDeprecationWarnings', () => {
18
beforeEach(() => {
19
jest.resetModules();
20
React = require('react');
21
ReactFeatureFlags = require('shared/ReactFeatureFlags');
21
- ReactTestUtils = require('react-dom/test-utils');
22
+ ReactNoop = require('react-noop-renderer');
23
+ Scheduler = require('scheduler');
24
ReactFeatureFlags.warnAboutDefaultPropsOnFunctionComponents = true;
25
+ ReactFeatureFlags.warnAboutStringRefs = true;
26
});
27
28
afterEach(() => {
29
ReactFeatureFlags.warnAboutDefaultPropsOnFunctionComponents = false;
30
+ ReactFeatureFlags.warnAboutStringRefs = false;
31
});
32
33
it('should warn when given defaultProps', () => {
@@ -35,13 +39,37 @@ describe('ReactDeprecationWarnings', () => {
39
testProp: true,
40
};
41
38
- expect(() =>
39
- ReactTestUtils.renderIntoDocument(<FunctionalComponent />),
40
- ).toWarnDev(
42
+ ReactNoop.render(<FunctionalComponent />);
43
+ expect(() => expect(Scheduler).toFlushWithoutYielding()).toWarnDev(
44
'Warning: FunctionalComponent: Support for defaultProps ' +
45
'will be removed from function components in a future major ' +
46
'release. Use JavaScript default parameters instead.',
47
{withoutStack: true},
48
);
49
});
50
+
51
+ it('should warn when given string refs', () => {
52
+ class RefComponent extends React.Component {
53
+ render() {
54
+ return null;
55
+ }
56
+ }
57
+ class Component extends React.Component {
58
+ render() {
59
+ return <RefComponent ref="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
+ });
75
});
packages/react-reconciler/src/ReactChildFiber.js
+34
-16
@@ -30,6 +30,7 @@ import {
30
import invariant from 'shared/invariant';
31
import warning from 'shared/warning';
32
import warningWithoutStack from 'shared/warningWithoutStack';
33
+import {warnAboutStringRefs} from 'shared/ReactFeatureFlags';
34
35
import {
36
createWorkInProgress,
@@ -49,7 +50,7 @@ import {StrictMode} from './ReactTypeOfMode';
50
51
let didWarnAboutMaps;
52
let didWarnAboutGenerators;
52
-let didWarnAboutStringRefInStrictMode;
53
+let didWarnAboutStringRefs;
54
let ownerHasKeyUseWarning;
55
let ownerHasFunctionTypeWarning;
56
let warnForMissingKey = (child: mixed) => {};
@@ -57,7 +58,7 @@ let warnForMissingKey = (child: mixed) => {};
58
if (__DEV__) {
59
didWarnAboutMaps = false;
60
didWarnAboutGenerators = false;
60
- didWarnAboutStringRefInStrictMode = {};
61
+ didWarnAboutStringRefs = {};
62
63
/**
64
* Warn if there's no key explicitly set on dynamic arrays of children or
@@ -114,21 +115,38 @@ function coerceRef(
115
typeof mixedRef !== 'object'
116
) {
117
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) {
121
const componentName = 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;
150
}
151
}
152
}
packages/react/src/__tests__/ReactStrictMode-test.internal.js
+2
-2
@@ -693,7 +693,7 @@ Please update the following components: Parent`,
693
}).toWarnDev(
694
'Warning: A string ref, "somestring", has been found within a strict mode tree. ' +
695
'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' +
697
' in StrictMode (at **)\n' +
698
' in OuterComponent (at **)\n\n' +
699
'Learn more about using refs safely here:\n' +
@@ -735,7 +735,7 @@ Please update the following components: Parent`,
735
}).toWarnDev(
736
'Warning: A string ref, "somestring", has been found within a strict mode tree. ' +
737
'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' +
739
' in InnerComponent (at **)\n' +
740
' in StrictMode (at **)\n' +
741
' in OuterComponent (at **)\n\n' +
packages/shared/ReactFeatureFlags.js
+1
@@ -92,6 +92,7 @@ export const enableSuspenseCallback = false;
92
// from React.createElement to React.jsx
93
// https://github.com/reactjs/rfcs/blob/createlement-rfc/text/0000-create-element-changes.md
94
export const warnAboutDefaultPropsOnFunctionComponents = false;
95
+export const warnAboutStringRefs = false;
96
97
export const disableLegacyContext = false;
98
packages/shared/forks/ReactFeatureFlags.native-fb.js
+1
@@ -40,6 +40,7 @@ export const flushSuspenseFallbacksInTests = true;
40
export const enableUserBlockingEvents = false;
41
export const enableSuspenseCallback = false;
42
export const warnAboutDefaultPropsOnFunctionComponents = false;
43
+export const warnAboutStringRefs = false;
44
export const disableLegacyContext = false;
45
export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
46
packages/shared/forks/ReactFeatureFlags.native-oss.js
+1
@@ -35,6 +35,7 @@ export const flushSuspenseFallbacksInTests = true;
35
export const enableUserBlockingEvents = false;
36
export const enableSuspenseCallback = false;
37
export const warnAboutDefaultPropsOnFunctionComponents = false;
38
+export const warnAboutStringRefs = false;
39
export const disableLegacyContext = false;
40
export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
41
packages/shared/forks/ReactFeatureFlags.persistent.js
+1
@@ -35,6 +35,7 @@ export const flushSuspenseFallbacksInTests = true;
35
export const enableUserBlockingEvents = false;
36
export const enableSuspenseCallback = false;
37
export const warnAboutDefaultPropsOnFunctionComponents = false;
38
+export const warnAboutStringRefs = false;
39
export const disableLegacyContext = false;
40
export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
41
packages/shared/forks/ReactFeatureFlags.test-renderer.js
+1
@@ -35,6 +35,7 @@ export const flushSuspenseFallbacksInTests = true;
35
export const enableUserBlockingEvents = false;
36
export const enableSuspenseCallback = false;
37
export const warnAboutDefaultPropsOnFunctionComponents = false;
38
+export const warnAboutStringRefs = false;
39
export const disableLegacyContext = false;
40
export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
41
packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
+1
@@ -35,6 +35,7 @@ export const flushSuspenseFallbacksInTests = true;
35
export const enableUserBlockingEvents = false;
36
export const enableSuspenseCallback = true;
37
export const warnAboutDefaultPropsOnFunctionComponents = false;
38
+export const warnAboutStringRefs = false;
39
export const disableLegacyContext = false;
40
export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
41
packages/shared/forks/ReactFeatureFlags.www.js
+2
@@ -82,6 +82,8 @@ export const enableSuspenseCallback = true;
82
83
export const warnAboutDefaultPropsOnFunctionComponents = false;
84
85
+export const warnAboutStringRefs = false;
86
+
87
export const flushSuspenseFallbacksInTests = true;
88
89
// Flow magic to verify the exports of this file match the original version.