Skip to content

Commit

Permalink
Updated warning message and merged with master
Browse files Browse the repository at this point in the history
fix
  • Loading branch information
trueadm committed Feb 6, 2018
2 parents 9b19254 + 6f2f55e commit 3f995e7
Show file tree
Hide file tree
Showing 4 changed files with 200 additions and 84 deletions.
41 changes: 35 additions & 6 deletions packages/react-reconciler/src/ReactChildFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import type {ReactPortal} from 'shared/ReactTypes';
import type {Fiber} from 'react-reconciler/src/ReactFiber';
import type {ExpirationTime} from 'react-reconciler/src/ReactFiberExpirationTime';

import getComponentName from 'shared/getComponentName';
import {Placement, Deletion} from 'shared/ReactTypeOfSideEffect';
import {
getIteratorFn,
Expand All @@ -26,6 +27,7 @@ import {
HostPortal,
Fragment,
} from 'shared/ReactTypeOfWork';
import {getStackAddendumByWorkInProgressFiber} from 'shared/ReactFiberComponentTreeHook';
import emptyObject from 'fbjs/lib/emptyObject';
import invariant from 'fbjs/lib/invariant';
import warning from 'fbjs/lib/warning';
Expand All @@ -38,16 +40,20 @@ import {
createFiberFromPortal,
} from './ReactFiber';
import ReactDebugCurrentFiber from './ReactDebugCurrentFiber';
import {StrictMode} from './ReactTypeOfMode';

const {getCurrentFiberStackAddendum} = ReactDebugCurrentFiber;

let didWarnAboutMaps;
let didWarnAboutStringRefInStrictMode;
let ownerHasKeyUseWarning;
let ownerHasFunctionTypeWarning;
let warnForMissingKey = (child: mixed) => {};

if (__DEV__) {
didWarnAboutMaps = false;
didWarnAboutStringRefInStrictMode = {};

/**
* Warn if there's no key explicitly set on dynamic arrays of children or
* object keys are not valid. This allows us to keep track of children between
Expand Down Expand Up @@ -92,13 +98,36 @@ if (__DEV__) {

const isArray = Array.isArray;

function coerceRef(current: Fiber | null, element: ReactElement) {
function coerceRef(
returnFiber: Fiber,
current: Fiber | null,
element: ReactElement,
) {
let mixedRef = element.ref;
if (
mixedRef !== null &&
typeof mixedRef !== 'function' &&
typeof mixedRef !== 'object'
) {
if (__DEV__) {
if (returnFiber.mode & StrictMode) {
const componentName = getComponentName(returnFiber) || 'Component';
if (!didWarnAboutStringRefInStrictMode[componentName]) {
warning(
false,
'A string ref has been found within a strict mode tree. ' +
'String refs are a source of potential bugs and should be avoided. ' +
'We recommend using createRef() instead.' +
'\n%s' +
'\n\nLearn more about using refs safely here:' +
'\nhttps://fb.me/react-strict-mode-string-ref',
getStackAddendumByWorkInProgressFiber(returnFiber),
);
didWarnAboutStringRefInStrictMode[componentName] = true;
}
}
}

if (element._owner) {
const owner: ?Fiber = (element._owner: any);
let inst;
Expand Down Expand Up @@ -344,7 +373,7 @@ function ChildReconciler(shouldTrackSideEffects) {
if (current !== null && current.type === element.type) {
// Move based on index
const existing = useFiber(current, element.props, expirationTime);
existing.ref = coerceRef(current, element);
existing.ref = coerceRef(returnFiber, current, element);
existing.return = returnFiber;
if (__DEV__) {
existing._debugSource = element._source;
Expand All @@ -358,7 +387,7 @@ function ChildReconciler(shouldTrackSideEffects) {
returnFiber.mode,
expirationTime,
);
created.ref = coerceRef(current, element);
created.ref = coerceRef(returnFiber, current, element);
created.return = returnFiber;
return created;
}
Expand Down Expand Up @@ -443,7 +472,7 @@ function ChildReconciler(shouldTrackSideEffects) {
returnFiber.mode,
expirationTime,
);
created.ref = coerceRef(null, newChild);
created.ref = coerceRef(returnFiber, null, newChild);
created.return = returnFiber;
return created;
}
Expand Down Expand Up @@ -1081,7 +1110,7 @@ function ChildReconciler(shouldTrackSideEffects) {
: element.props,
expirationTime,
);
existing.ref = coerceRef(child, element);
existing.ref = coerceRef(returnFiber, child, element);
existing.return = returnFiber;
if (__DEV__) {
existing._debugSource = element._source;
Expand Down Expand Up @@ -1113,7 +1142,7 @@ function ChildReconciler(shouldTrackSideEffects) {
returnFiber.mode,
expirationTime,
);
created.ref = coerceRef(currentFirstChild, element);
created.ref = coerceRef(returnFiber, currentFirstChild, element);
created.return = returnFiber;
return created;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1074,7 +1074,9 @@ describe('ReactIncrementalSideEffects', () => {
}

ReactNoop.render(<Foo />);
ReactNoop.flush();
expect(ReactNoop.flush).toWarnDev(
'Warning: A string ref has been found within a strict mode tree.',
);

expect(fooInstance.refs.bar.test).toEqual('test');
});
Expand Down
85 changes: 85 additions & 0 deletions packages/react/src/__tests__/ReactStrictMode-test.internal.js
Original file line number Diff line number Diff line change
Expand Up @@ -745,4 +745,89 @@ describe('ReactStrictMode', () => {
expect(rendered.toJSON()).toBe('count:2');
});
});

describe('string refs', () => {
beforeEach(() => {
jest.resetModules();
React = require('react');
ReactTestRenderer = require('react-test-renderer');
});

it('should warn within a strict tree', () => {
const {StrictMode} = React;

class OuterComponent extends React.Component {
render() {
return (
<StrictMode>
<InnerComponent ref="somestring" />
</StrictMode>
);
}
}

class InnerComponent extends React.Component {
render() {
return null;
}
}

let renderer;
expect(() => {
renderer = ReactTestRenderer.create(<OuterComponent />);
}).toWarnDev(
'Warning: A string ref has been found within a strict mode tree. ' +
'String refs are a source of potential bugs and should be avoided. ' +
'We recommend using createRef() instead.\n\n' +
' in OuterComponent (at **)\n\n' +
'Learn more about using refs safely here:\n' +
'https://fb.me/react-strict-mode-string-ref',
);

// Dedup
renderer.update(<OuterComponent />);
});

it('should warn within a strict tree', () => {
const {StrictMode} = React;

class OuterComponent extends React.Component {
render() {
return (
<StrictMode>
<InnerComponent />
</StrictMode>
);
}
}

class InnerComponent extends React.Component {
render() {
return <MiddleComponent ref="somestring" />;
}
}

class MiddleComponent extends React.Component {
render() {
return null;
}
}

let renderer;
expect(() => {
renderer = ReactTestRenderer.create(<OuterComponent />);
}).toWarnDev(
'Warning: A string ref has been found within a strict mode tree. ' +
'String refs are a source of potential bugs and should be avoided. ' +
'We recommend using a createRef() instead.\n\n' +
' in InnerComponent (at **)\n' +
' in OuterComponent (at **)\n\n' +
'Learn more about using refs safely here:\n' +
'https://fb.me/react-strict-mode-string-ref',
);

// Dedup
renderer.update(<OuterComponent />);
});
});
});
Loading

0 comments on commit 3f995e7

Please sign in to comment.