Skip to content

Commit

Permalink
Show component stack in PropTypes warnings
Browse files Browse the repository at this point in the history
  • Loading branch information
sophiebits committed May 15, 2016
1 parent 3cc733a commit fef1900
Show file tree
Hide file tree
Showing 13 changed files with 251 additions and 40 deletions.
4 changes: 4 additions & 0 deletions src/isomorphic/ReactDebugTool.js
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,10 @@ var ReactDebugTool = {
checkDebugID(debugID);
emitEvent('onSetOwner', debugID, ownerDebugID);
},
onSetParent(debugID, parentDebugID) {
checkDebugID(debugID);
emitEvent('onSetParent', debugID, parentDebugID);
},
onSetText(debugID, text) {
checkDebugID(debugID);
emitEvent('onSetText', debugID, text);
Expand Down
20 changes: 13 additions & 7 deletions src/isomorphic/classic/element/ReactElementValidator.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@

'use strict';

var ReactCurrentOwner = require('ReactCurrentOwner');
var ReactComponentTreeDevtool = require('ReactComponentTreeDevtool');
var ReactElement = require('ReactElement');
var ReactPropTypeLocations = require('ReactPropTypeLocations');
var ReactPropTypeLocationNames = require('ReactPropTypeLocationNames');
var ReactCurrentOwner = require('ReactCurrentOwner');
var ReactPropTypeLocations = require('ReactPropTypeLocations');

var canDefineProperty = require('canDefineProperty');
var getIteratorFn = require('getIteratorFn');
Expand Down Expand Up @@ -171,13 +172,14 @@ function validateChildKeys(node, parentType) {
/**
* Assert that the props are valid
*
* @param {object} element
* @param {string} componentName Name of the component for error messages.
* @param {object} propTypes Map of prop name to a ReactPropType
* @param {object} props
* @param {string} location e.g. "prop", "context", "child context"
* @private
*/
function checkPropTypes(componentName, propTypes, props, location) {
function checkPropTypes(element, componentName, propTypes, location) {
var props = element.props;
for (var propName in propTypes) {
if (propTypes.hasOwnProperty(propName)) {
var error;
Expand Down Expand Up @@ -216,8 +218,12 @@ function checkPropTypes(componentName, propTypes, props, location) {
// same error.
loggedTypeFailures[error.message] = true;

var addendum = getDeclarationErrorAddendum();
warning(false, 'Failed propType: %s%s', error.message, addendum);
warning(
false,
'Failed propType: %s%s',
error.message,
ReactComponentTreeDevtool.getCurrentStackAddendum(element)
);
}
}
}
Expand All @@ -237,9 +243,9 @@ function validatePropTypes(element) {
var name = componentClass.displayName || componentClass.name;
if (componentClass.propTypes) {
checkPropTypes(
element,
name,
componentClass.propTypes,
element.props,
ReactPropTypeLocations.prop
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,10 @@ describe('ReactElementClone', function() {
expect(console.error.argsForCall[0][0]).toBe(
'Warning: Failed propType: ' +
'Invalid prop `color` of type `number` supplied to `Component`, ' +
'expected `string`. Check the render method of `Parent`.'
'expected `string`.\n' +
' in Component (created by GrandParent)\n' +
' in Parent (created by GrandParent)\n' +
' in GrandParent'
);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,9 @@ describe('ReactElementValidator', function() {
expect(console.error.argsForCall[0][0]).toBe(
'Warning: Failed propType: ' +
'Invalid prop `color` of type `number` supplied to `MyComp`, ' +
'expected `string`. Check the render method of `ParentComp`.'
'expected `string`.\n' +
' in MyComp (created by ParentComp)\n' +
' in ParentComp'
);
});

Expand Down Expand Up @@ -318,7 +320,8 @@ describe('ReactElementValidator', function() {
expect(console.error.calls.length).toBe(1);
expect(console.error.argsForCall[0][0]).toBe(
'Warning: Failed propType: ' +
'Required prop `prop` was not specified in `Component`.'
'Required prop `prop` was not specified in `Component`.\n' +
' in Component'
);
});

Expand All @@ -342,7 +345,8 @@ describe('ReactElementValidator', function() {
expect(console.error.calls.length).toBe(1);
expect(console.error.argsForCall[0][0]).toBe(
'Warning: Failed propType: ' +
'Required prop `prop` was not specified in `Component`.'
'Required prop `prop` was not specified in `Component`.\n' +
' in Component'
);
});

Expand All @@ -368,13 +372,15 @@ describe('ReactElementValidator', function() {
expect(console.error.calls.length).toBe(2);
expect(console.error.argsForCall[0][0]).toBe(
'Warning: Failed propType: ' +
'Required prop `prop` was not specified in `Component`.'
'Required prop `prop` was not specified in `Component`.\n' +
' in Component'
);

expect(console.error.argsForCall[1][0]).toBe(
'Warning: Failed propType: ' +
'Invalid prop `prop` of type `number` supplied to ' +
'`Component`, expected `string`.'
'`Component`, expected `string`.\n' +
' in Component'
);

ReactTestUtils.renderIntoDocument(
Expand Down
7 changes: 5 additions & 2 deletions src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -891,8 +891,11 @@ describe('ReactPropTypes', function() {
var instance = <Component num={6} />;
instance = ReactTestUtils.renderIntoDocument(instance);
expect(console.error.argsForCall.length).toBe(1);
expect(console.error.argsForCall[0][0]).toBe(
'Warning: Failed propType: num must be 5!'
expect(
console.error.argsForCall[0][0].replace(/\(at .+?:\d+\)/g, '(at **)')
).toBe(
'Warning: Failed propType: num must be 5!\n' +
' in Component (at **)'
);
});

Expand Down
73 changes: 70 additions & 3 deletions src/isomorphic/devtools/ReactComponentTreeDevtool.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

'use strict';

var ReactCurrentOwner = require('ReactCurrentOwner');

var invariant = require('invariant');

var tree = {};
Expand Down Expand Up @@ -53,7 +55,6 @@ var ReactComponentTreeDevtool = {

onSetChildren(id, nextChildIDs) {
updateTree(id, item => {
var prevChildIDs = item.childIDs;
item.childIDs = nextChildIDs;

nextChildIDs.forEach(nextChildID => {
Expand All @@ -78,10 +79,20 @@ var ReactComponentTreeDevtool = {
'Expected onMountComponent() to fire for the child ' +
'before its parent includes it in onSetChildren().'
);

if (prevChildIDs.indexOf(nextChildID) === -1) {
if (nextChild.parentID == null) {
nextChild.parentID = id;
// TODO: This shouldn't be necessary but mounting a new root during in
// componentWillMount currently causes not-yet-mounted components to
// be purged from our tree data so their parent ID is missing.
}
invariant(
nextChild.parentID === id,
'Expected onSetParent() and onSetChildren() to be consistent (%s ' +
'has parents %s and %s).',
nextChildID,
nextChild.parentID,
id
);
});
});
},
Expand All @@ -90,6 +101,10 @@ var ReactComponentTreeDevtool = {
updateTree(id, item => item.ownerID = ownerID);
},

onSetParent(id, parentID) {
updateTree(id, item => item.parentID = parentID);
},

onSetText(id, text) {
updateTree(id, item => item.text = text);
},
Expand Down Expand Up @@ -138,6 +153,53 @@ var ReactComponentTreeDevtool = {
return item ? item.isMounted : false;
},

getCurrentStackAddendum(topElement) {
function describeComponentFrame(name, source, ownerName) {
return '\n in ' + name + (
source ?
' (at ' + source.fileName.replace(/^.*[\\\/]/, '') + ':' +
source.lineNumber + ')' :
ownerName ?
' (created by ' + ownerName + ')' :
''
);
}

function describeID(id) {
var name = ReactComponentTreeDevtool.getDisplayName(id);
var element = ReactComponentTreeDevtool.getElement(id);
var ownerID = ReactComponentTreeDevtool.getOwnerID(id);
var ownerName;
if (ownerID) {
ownerName = ReactComponentTreeDevtool.getDisplayName(ownerID);
}
return describeComponentFrame(name, element._source, ownerName);
}

var info = '';
if (topElement) {
var type = topElement.type;
var name = typeof type === 'function' ?
type.displayName || type.name :
type;
var owner = topElement._owner;
info += describeComponentFrame(
name || 'Unknown',
topElement._source,
owner && owner.getName()
);
}

var currentOwner = ReactCurrentOwner.current;
var id = currentOwner && currentOwner._debugID;
while (id) {
info += describeID(id);
id = ReactComponentTreeDevtool.getParentID(id);
}

return info;
},

getChildIDs(id) {
var item = tree[id];
return item ? item.childIDs : [];
Expand All @@ -148,6 +210,11 @@ var ReactComponentTreeDevtool = {
return item ? item.displayName : 'Unknown';
},

getElement(id) {
var item = tree[id];
return item ? item.element : null;
},

getOwnerID(id) {
var item = tree[id];
return item ? item.ownerID : null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1741,4 +1741,78 @@ describe('ReactComponentTreeDevtool', () => {
expect(getRootDisplayNames()).toEqual([]);
expect(getRegisteredDisplayNames()).toEqual([]);
});

it('creates stack addenda', () => {
function getAddendum(element) {
var addendum = ReactComponentTreeDevtool.getCurrentStackAddendum(element);
return addendum.replace(/\(at .+?:\d+\)/g, '(at **)');
}

var Anon = React.createClass({displayName: null, render: () => null});
var Orange = React.createClass({render: () => null});

expect(getAddendum()).toBe(
''
);
expect(getAddendum(<div />)).toBe(
'\n in div (at **)'
);
expect(getAddendum(<Anon />)).toBe(
'\n in Unknown (at **)'
);
expect(getAddendum(<Orange />)).toBe(
'\n in Orange (at **)'
);
expect(getAddendum(React.createElement(Orange))).toBe(
'\n in Orange'
);

var renders = 0;
var rOwnedByQ;

function Q() {
return (rOwnedByQ = React.createElement(R));
}
function R() {
return <div><S /></div>;
}
class S extends React.Component {
componentDidMount() {
// Check that the parent path is still fetched when only S itself is on
// the stack.
this.forceUpdate();
}
render() {
expect(getAddendum()).toBe(
'\n in S (at **)' +
'\n in div (at **)' +
'\n in R (created by Q)' +
'\n in Q (at **)'
);
expect(getAddendum(<span />)).toBe(
'\n in span (at **)' +
'\n in S (at **)' +
'\n in div (at **)' +
'\n in R (created by Q)' +
'\n in Q (at **)'
);
expect(getAddendum(React.createElement('span'))).toBe(
'\n in span (created by S)' +
'\n in S (at **)' +
'\n in div (at **)' +
'\n in R (created by Q)' +
'\n in Q (at **)'
);
renders++;
return null;
}
}
ReactDOM.render(<Q />, document.createElement('div'));
expect(renders).toBe(2);

// Make sure owner is fetched for the top element too.
expect(getAddendum(rOwnedByQ)).toBe(
'\n in R (created by Q)'
);
});
});
Loading

0 comments on commit fef1900

Please sign in to comment.