Warn about string refs within strict mode trees (#12161)
* Warn about string refs within strict mode trees * Improved string ref warning message
Brian Vaughn committed
Feb 6, 2018 at 07:43 UTC
6f2f55ed568063a4efbe4b2f1e11297da877fe73
3 files changed
+123
-7
packages/react-reconciler/src/ReactChildFiber.js
+35
-6
@@ -12,6 +12,7 @@ import type {ReactPortal} from 'shared/ReactTypes';
12
import type {Fiber} from 'react-reconciler/src/ReactFiber';
13
import type {ExpirationTime} from 'react-reconciler/src/ReactFiberExpirationTime';
14
15
+import getComponentName from 'shared/getComponentName';
16
import {Placement, Deletion} from 'shared/ReactTypeOfSideEffect';
17
import {
18
getIteratorFn,
@@ -26,6 +27,7 @@ import {
27
HostPortal,
28
Fragment,
29
} from 'shared/ReactTypeOfWork';
30
+import {getStackAddendumByWorkInProgressFiber} from 'shared/ReactFiberComponentTreeHook';
31
import emptyObject from 'fbjs/lib/emptyObject';
32
import invariant from 'fbjs/lib/invariant';
33
import warning from 'fbjs/lib/warning';
@@ -38,16 +40,20 @@ import {
40
createFiberFromPortal,
41
} from './ReactFiber';
42
import ReactDebugCurrentFiber from './ReactDebugCurrentFiber';
43
+import {StrictMode} from './ReactTypeOfMode';
44
45
const {getCurrentFiberStackAddendum} = ReactDebugCurrentFiber;
46
47
let didWarnAboutMaps;
48
+let didWarnAboutStringRefInStrictMode;
49
let ownerHasKeyUseWarning;
50
let ownerHasFunctionTypeWarning;
51
let warnForMissingKey = (child: mixed) => {};
52
53
if (__DEV__) {
54
didWarnAboutMaps = false;
55
+ didWarnAboutStringRefInStrictMode = {};
56
+
57
/**
58
* Warn if there's no key explicitly set on dynamic arrays of children or
59
* object keys are not valid. This allows us to keep track of children between
@@ -92,9 +98,32 @@ if (__DEV__) {
98
99
const isArray = Array.isArray;
100
95
-function coerceRef(current: Fiber | null, element: ReactElement) {
101
+function coerceRef(
102
+ returnFiber: Fiber,
103
+ current: Fiber | null,
104
+ element: ReactElement,
105
+) {
106
let mixedRef = element.ref;
107
if (mixedRef !== null && typeof mixedRef !== 'function') {
108
+ if (__DEV__) {
109
+ if (returnFiber.mode & StrictMode) {
110
+ const componentName = getComponentName(returnFiber) || 'Component';
111
+ if (!didWarnAboutStringRefInStrictMode[componentName]) {
112
+ warning(
113
+ false,
114
+ 'A string ref has been found within a strict mode tree. ' +
115
+ 'String refs are a source of potential bugs and should be avoided. ' +
116
+ 'We recommend using a ref callback instead.' +
117
+ '\n%s' +
118
+ '\n\nLearn more about using refs safely here:' +
119
+ '\nhttps://fb.me/react-strict-mode-string-ref',
120
+ getStackAddendumByWorkInProgressFiber(returnFiber),
121
+ );
122
+ didWarnAboutStringRefInStrictMode[componentName] = true;
123
+ }
124
+ }
125
+ }
126
+
127
if (element._owner) {
128
const owner: ?Fiber = (element._owner: any);
129
let inst;
@@ -340,7 +369,7 @@ function ChildReconciler(shouldTrackSideEffects) {
369
if (current !== null && current.type === element.type) {
370
// Move based on index
371
const existing = useFiber(current, element.props, expirationTime);
343
- existing.ref = coerceRef(current, element);
372
+ existing.ref = coerceRef(returnFiber, current, element);
373
existing.return = returnFiber;
374
if (__DEV__) {
375
existing._debugSource = element._source;
@@ -354,7 +383,7 @@ function ChildReconciler(shouldTrackSideEffects) {
383
returnFiber.mode,
384
expirationTime,
385
);
357
- created.ref = coerceRef(current, element);
386
+ created.ref = coerceRef(returnFiber, current, element);
387
created.return = returnFiber;
388
return created;
389
}
@@ -439,7 +468,7 @@ function ChildReconciler(shouldTrackSideEffects) {
468
returnFiber.mode,
469
expirationTime,
470
);
442
- created.ref = coerceRef(null, newChild);
471
+ created.ref = coerceRef(returnFiber, null, newChild);
472
created.return = returnFiber;
473
return created;
474
}
@@ -1077,7 +1106,7 @@ function ChildReconciler(shouldTrackSideEffects) {
1106
: element.props,
1107
expirationTime,
1108
);
1080
- existing.ref = coerceRef(child, element);
1109
+ existing.ref = coerceRef(returnFiber, child, element);
1110
existing.return = returnFiber;
1111
if (__DEV__) {
1112
existing._debugSource = element._source;
@@ -1109,7 +1138,7 @@ function ChildReconciler(shouldTrackSideEffects) {
1138
returnFiber.mode,
1139
expirationTime,
1140
);
1112
- created.ref = coerceRef(currentFirstChild, element);
1141
+ created.ref = coerceRef(returnFiber, currentFirstChild, element);
1142
created.return = returnFiber;
1143
return created;
1144
}
packages/react-reconciler/src/__tests__/ReactIncrementalSideEffects-test.internal.js
+3
-1
@@ -1074,7 +1074,9 @@ describe('ReactIncrementalSideEffects', () => {
1074
}
1075
1076
ReactNoop.render(<Foo />);
1077
- ReactNoop.flush();
1077
+ expect(ReactNoop.flush).toWarnDev(
1078
+ 'Warning: A string ref has been found within a strict mode tree.',
1079
+ );
1080
1081
expect(fooInstance.refs.bar.test).toEqual('test');
1082
});
packages/react/src/__tests__/ReactStrictMode-test.internal.js
+85
@@ -745,4 +745,89 @@ describe('ReactStrictMode', () => {
745
expect(rendered.toJSON()).toBe('count:2');
746
});
747
});
748
+
749
+ describe('string refs', () => {
750
+ beforeEach(() => {
751
+ jest.resetModules();
752
+ React = require('react');
753
+ ReactTestRenderer = require('react-test-renderer');
754
+ });
755
+
756
+ it('should warn within a strict tree', () => {
757
+ const {StrictMode} = React;
758
+
759
+ class OuterComponent extends React.Component {
760
+ render() {
761
+ return (
762
+ <StrictMode>
763
+ <InnerComponent ref="somestring" />
764
+ </StrictMode>
765
+ );
766
+ }
767
+ }
768
+
769
+ class InnerComponent extends React.Component {
770
+ render() {
771
+ return null;
772
+ }
773
+ }
774
+
775
+ let renderer;
776
+ expect(() => {
777
+ renderer = ReactTestRenderer.create(<OuterComponent />);
778
+ }).toWarnDev(
779
+ 'Warning: A string ref has been found within a strict mode tree. ' +
780
+ 'String refs are a source of potential bugs and should be avoided. ' +
781
+ 'We recommend using a ref callback instead.\n\n' +
782
+ ' in OuterComponent (at **)\n\n' +
783
+ 'Learn more about using refs safely here:\n' +
784
+ 'https://fb.me/react-strict-mode-string-ref',
785
+ );
786
+
787
+ // Dedup
788
+ renderer.update(<OuterComponent />);
789
+ });
790
+
791
+ it('should warn within a strict tree', () => {
792
+ const {StrictMode} = React;
793
+
794
+ class OuterComponent extends React.Component {
795
+ render() {
796
+ return (
797
+ <StrictMode>
798
+ <InnerComponent />
799
+ </StrictMode>
800
+ );
801
+ }
802
+ }
803
+
804
+ class InnerComponent extends React.Component {
805
+ render() {
806
+ return <MiddleComponent ref="somestring" />;
807
+ }
808
+ }
809
+
810
+ class MiddleComponent extends React.Component {
811
+ render() {
812
+ return null;
813
+ }
814
+ }
815
+
816
+ let renderer;
817
+ expect(() => {
818
+ renderer = ReactTestRenderer.create(<OuterComponent />);
819
+ }).toWarnDev(
820
+ 'Warning: A string ref has been found within a strict mode tree. ' +
821
+ 'String refs are a source of potential bugs and should be avoided. ' +
822
+ 'We recommend using a ref callback instead.\n\n' +
823
+ ' in InnerComponent (at **)\n' +
824
+ ' in OuterComponent (at **)\n\n' +
825
+ 'Learn more about using refs safely here:\n' +
826
+ 'https://fb.me/react-strict-mode-string-ref',
827
+ );
828
+
829
+ // Dedup
830
+ renderer.update(<OuterComponent />);
831
+ });
832
+ });
833
});