Skip to content

Commit c1b12b9

Browse files
committed
Updated warning message and merged with master
2 parents 9b19254 + 6f2f55e commit c1b12b9

File tree

3 files changed

+124
-12
lines changed

3 files changed

+124
-12
lines changed

packages/react-reconciler/src/ReactChildFiber.js

+36-11
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import type {ReactPortal} from 'shared/ReactTypes';
1212
import type {Fiber} from 'react-reconciler/src/ReactFiber';
1313
import type {ExpirationTime} from 'react-reconciler/src/ReactFiberExpirationTime';
1414

15+
import getComponentName from 'shared/getComponentName';
1516
import {Placement, Deletion} from 'shared/ReactTypeOfSideEffect';
1617
import {
1718
getIteratorFn,
@@ -26,6 +27,7 @@ import {
2627
HostPortal,
2728
Fragment,
2829
} from 'shared/ReactTypeOfWork';
30+
import {getStackAddendumByWorkInProgressFiber} from 'shared/ReactFiberComponentTreeHook';
2931
import emptyObject from 'fbjs/lib/emptyObject';
3032
import invariant from 'fbjs/lib/invariant';
3133
import warning from 'fbjs/lib/warning';
@@ -38,16 +40,20 @@ import {
3840
createFiberFromPortal,
3941
} from './ReactFiber';
4042
import ReactDebugCurrentFiber from './ReactDebugCurrentFiber';
43+
import {StrictMode} from './ReactTypeOfMode';
4144

4245
const {getCurrentFiberStackAddendum} = ReactDebugCurrentFiber;
4346

4447
let didWarnAboutMaps;
48+
let didWarnAboutStringRefInStrictMode;
4549
let ownerHasKeyUseWarning;
4650
let ownerHasFunctionTypeWarning;
4751
let warnForMissingKey = (child: mixed) => {};
4852

4953
if (__DEV__) {
5054
didWarnAboutMaps = false;
55+
didWarnAboutStringRefInStrictMode = {};
56+
5157
/**
5258
* Warn if there's no key explicitly set on dynamic arrays of children or
5359
* object keys are not valid. This allows us to keep track of children between
@@ -92,13 +98,32 @@ if (__DEV__) {
9298

9399
const isArray = Array.isArray;
94100

95-
function coerceRef(current: Fiber | null, element: ReactElement) {
101+
function coerceRef(
102+
returnFiber: Fiber,
103+
current: Fiber | null,
104+
element: ReactElement,
105+
) {
96106
let mixedRef = element.ref;
97-
if (
98-
mixedRef !== null &&
99-
typeof mixedRef !== 'function' &&
100-
typeof mixedRef !== 'object'
101-
) {
107+
if (mixedRef !== null && typeof mixedRef !== 'function' && typeof mixedRef !== 'object') {
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 createRef() 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+
102127
if (element._owner) {
103128
const owner: ?Fiber = (element._owner: any);
104129
let inst;
@@ -344,7 +369,7 @@ function ChildReconciler(shouldTrackSideEffects) {
344369
if (current !== null && current.type === element.type) {
345370
// Move based on index
346371
const existing = useFiber(current, element.props, expirationTime);
347-
existing.ref = coerceRef(current, element);
372+
existing.ref = coerceRef(returnFiber, current, element);
348373
existing.return = returnFiber;
349374
if (__DEV__) {
350375
existing._debugSource = element._source;
@@ -358,7 +383,7 @@ function ChildReconciler(shouldTrackSideEffects) {
358383
returnFiber.mode,
359384
expirationTime,
360385
);
361-
created.ref = coerceRef(current, element);
386+
created.ref = coerceRef(returnFiber, current, element);
362387
created.return = returnFiber;
363388
return created;
364389
}
@@ -443,7 +468,7 @@ function ChildReconciler(shouldTrackSideEffects) {
443468
returnFiber.mode,
444469
expirationTime,
445470
);
446-
created.ref = coerceRef(null, newChild);
471+
created.ref = coerceRef(returnFiber, null, newChild);
447472
created.return = returnFiber;
448473
return created;
449474
}
@@ -1081,7 +1106,7 @@ function ChildReconciler(shouldTrackSideEffects) {
10811106
: element.props,
10821107
expirationTime,
10831108
);
1084-
existing.ref = coerceRef(child, element);
1109+
existing.ref = coerceRef(returnFiber, child, element);
10851110
existing.return = returnFiber;
10861111
if (__DEV__) {
10871112
existing._debugSource = element._source;
@@ -1113,7 +1138,7 @@ function ChildReconciler(shouldTrackSideEffects) {
11131138
returnFiber.mode,
11141139
expirationTime,
11151140
);
1116-
created.ref = coerceRef(currentFirstChild, element);
1141+
created.ref = coerceRef(returnFiber, currentFirstChild, element);
11171142
created.return = returnFiber;
11181143
return created;
11191144
}

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

+3-1
Original file line numberDiff line numberDiff line change
@@ -1074,7 +1074,9 @@ describe('ReactIncrementalSideEffects', () => {
10741074
}
10751075

10761076
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+
);
10781080

10791081
expect(fooInstance.refs.bar.test).toEqual('test');
10801082
});

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

+85
Original file line numberDiff line numberDiff line change
@@ -745,4 +745,89 @@ describe('ReactStrictMode', () => {
745745
expect(rendered.toJSON()).toBe('count:2');
746746
});
747747
});
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+
});
748833
});

0 commit comments

Comments
 (0)