Skip to content

Commit c06d89a

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

File tree

4 files changed

+200
-84
lines changed

4 files changed

+200
-84
lines changed

packages/react-reconciler/src/ReactChildFiber.js

+35-6
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,36 @@ 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;
97107
if (
98108
mixedRef !== null &&
99109
typeof mixedRef !== 'function' &&
100110
typeof mixedRef !== 'object'
101111
) {
112+
if (__DEV__) {
113+
if (returnFiber.mode & StrictMode) {
114+
const componentName = getComponentName(returnFiber) || 'Component';
115+
if (!didWarnAboutStringRefInStrictMode[componentName]) {
116+
warning(
117+
false,
118+
'A string ref has been found within a strict mode tree. ' +
119+
'String refs are a source of potential bugs and should be avoided. ' +
120+
'We recommend using createRef() instead.' +
121+
'\n%s' +
122+
'\n\nLearn more about using refs safely here:' +
123+
'\nhttps://fb.me/react-strict-mode-string-ref',
124+
getStackAddendumByWorkInProgressFiber(returnFiber),
125+
);
126+
didWarnAboutStringRefInStrictMode[componentName] = true;
127+
}
128+
}
129+
}
130+
102131
if (element._owner) {
103132
const owner: ?Fiber = (element._owner: any);
104133
let inst;
@@ -344,7 +373,7 @@ function ChildReconciler(shouldTrackSideEffects) {
344373
if (current !== null && current.type === element.type) {
345374
// Move based on index
346375
const existing = useFiber(current, element.props, expirationTime);
347-
existing.ref = coerceRef(current, element);
376+
existing.ref = coerceRef(returnFiber, current, element);
348377
existing.return = returnFiber;
349378
if (__DEV__) {
350379
existing._debugSource = element._source;
@@ -358,7 +387,7 @@ function ChildReconciler(shouldTrackSideEffects) {
358387
returnFiber.mode,
359388
expirationTime,
360389
);
361-
created.ref = coerceRef(current, element);
390+
created.ref = coerceRef(returnFiber, current, element);
362391
created.return = returnFiber;
363392
return created;
364393
}
@@ -443,7 +472,7 @@ function ChildReconciler(shouldTrackSideEffects) {
443472
returnFiber.mode,
444473
expirationTime,
445474
);
446-
created.ref = coerceRef(null, newChild);
475+
created.ref = coerceRef(returnFiber, null, newChild);
447476
created.return = returnFiber;
448477
return created;
449478
}
@@ -1081,7 +1110,7 @@ function ChildReconciler(shouldTrackSideEffects) {
10811110
: element.props,
10821111
expirationTime,
10831112
);
1084-
existing.ref = coerceRef(child, element);
1113+
existing.ref = coerceRef(returnFiber, child, element);
10851114
existing.return = returnFiber;
10861115
if (__DEV__) {
10871116
existing._debugSource = element._source;
@@ -1113,7 +1142,7 @@ function ChildReconciler(shouldTrackSideEffects) {
11131142
returnFiber.mode,
11141143
expirationTime,
11151144
);
1116-
created.ref = coerceRef(currentFirstChild, element);
1145+
created.ref = coerceRef(returnFiber, currentFirstChild, element);
11171146
created.return = returnFiber;
11181147
return created;
11191148
}

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 createRef() 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 createRef() 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)