From fef190047a66d7f65b6ce6a18377192a7e5bd5e3 Mon Sep 17 00:00:00 2001 From: Ben Alpert Date: Sat, 14 May 2016 02:28:01 -0700 Subject: [PATCH] Show component stack in PropTypes warnings --- src/isomorphic/ReactDebugTool.js | 4 + .../classic/element/ReactElementValidator.js | 20 +++-- .../__tests__/ReactElementClone-test.js | 5 +- .../__tests__/ReactElementValidator-test.js | 16 ++-- .../types/__tests__/ReactPropTypes-test.js | 7 +- .../devtools/ReactComponentTreeDevtool.js | 73 +++++++++++++++++- .../ReactComponentTreeDevtool-test.js | 74 +++++++++++++++++++ .../ReactJSXElementValidator-test.js | 36 ++++++--- src/renderers/dom/shared/ReactDOMComponent.js | 2 + .../reconciler/ReactCompositeComponent.js | 34 ++++++--- .../stack/reconciler/ReactMultiChild.js | 9 +++ .../stack/reconciler/ReactReconciler.js | 4 + .../__tests__/ReactStatelessComponent-test.js | 7 +- 13 files changed, 251 insertions(+), 40 deletions(-) diff --git a/src/isomorphic/ReactDebugTool.js b/src/isomorphic/ReactDebugTool.js index d81ab6ee16ba6..b6e6f98e316fb 100644 --- a/src/isomorphic/ReactDebugTool.js +++ b/src/isomorphic/ReactDebugTool.js @@ -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); diff --git a/src/isomorphic/classic/element/ReactElementValidator.js b/src/isomorphic/classic/element/ReactElementValidator.js index 2051a212e3b95..9467160708914 100644 --- a/src/isomorphic/classic/element/ReactElementValidator.js +++ b/src/isomorphic/classic/element/ReactElementValidator.js @@ -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'); @@ -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; @@ -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) + ); } } } @@ -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 ); } diff --git a/src/isomorphic/classic/element/__tests__/ReactElementClone-test.js b/src/isomorphic/classic/element/__tests__/ReactElementClone-test.js index 20b4089726433..b062870d17e14 100644 --- a/src/isomorphic/classic/element/__tests__/ReactElementClone-test.js +++ b/src/isomorphic/classic/element/__tests__/ReactElementClone-test.js @@ -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' ); }); diff --git a/src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js b/src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js index 3cdcd48e5bb9d..64c7a4fc1ef68 100644 --- a/src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js +++ b/src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js @@ -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' ); }); @@ -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' ); }); @@ -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' ); }); @@ -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( diff --git a/src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js b/src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js index ea8d54035a113..1c01aeee8404a 100644 --- a/src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js +++ b/src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js @@ -891,8 +891,11 @@ describe('ReactPropTypes', function() { var instance = ; 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 **)' ); }); diff --git a/src/isomorphic/devtools/ReactComponentTreeDevtool.js b/src/isomorphic/devtools/ReactComponentTreeDevtool.js index c0347a3246e5c..25d537e2e829b 100644 --- a/src/isomorphic/devtools/ReactComponentTreeDevtool.js +++ b/src/isomorphic/devtools/ReactComponentTreeDevtool.js @@ -11,6 +11,8 @@ 'use strict'; +var ReactCurrentOwner = require('ReactCurrentOwner'); + var invariant = require('invariant'); var tree = {}; @@ -53,7 +55,6 @@ var ReactComponentTreeDevtool = { onSetChildren(id, nextChildIDs) { updateTree(id, item => { - var prevChildIDs = item.childIDs; item.childIDs = nextChildIDs; nextChildIDs.forEach(nextChildID => { @@ -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 + ); }); }); }, @@ -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); }, @@ -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 : []; @@ -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; diff --git a/src/isomorphic/devtools/__tests__/ReactComponentTreeDevtool-test.js b/src/isomorphic/devtools/__tests__/ReactComponentTreeDevtool-test.js index a600c79e59684..ed685741e645d 100644 --- a/src/isomorphic/devtools/__tests__/ReactComponentTreeDevtool-test.js +++ b/src/isomorphic/devtools/__tests__/ReactComponentTreeDevtool-test.js @@ -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(
)).toBe( + '\n in div (at **)' + ); + expect(getAddendum()).toBe( + '\n in Unknown (at **)' + ); + expect(getAddendum()).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
; + } + 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()).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(, 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)' + ); + }); }); diff --git a/src/isomorphic/modern/element/__tests__/ReactJSXElementValidator-test.js b/src/isomorphic/modern/element/__tests__/ReactJSXElementValidator-test.js index 1e2890e9f0fd6..427d2340bb305 100644 --- a/src/isomorphic/modern/element/__tests__/ReactJSXElementValidator-test.js +++ b/src/isomorphic/modern/element/__tests__/ReactJSXElementValidator-test.js @@ -195,10 +195,14 @@ describe('ReactJSXElementValidator', function() { } } ReactTestUtils.renderIntoDocument(); - expect(console.error.argsForCall[0][0]).toBe( + expect( + console.error.argsForCall[0][0].replace(/\(at .+?:\d+\)/g, '(at **)') + ).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 (at **)\n' + + ' in ParentComp (at **)' ); }); @@ -242,9 +246,12 @@ describe('ReactJSXElementValidator', function() { ReactTestUtils.renderIntoDocument(); expect(console.error.calls.length).toBe(1); - expect(console.error.argsForCall[0][0]).toBe( + expect( + console.error.argsForCall[0][0].replace(/\(at .+?:\d+\)/g, '(at **)') + ).toBe( 'Warning: Failed propType: ' + - 'Required prop `prop` was not specified in `RequiredPropComponent`.' + 'Required prop `prop` was not specified in `RequiredPropComponent`.\n' + + ' in RequiredPropComponent (at **)' ); }); @@ -254,9 +261,12 @@ describe('ReactJSXElementValidator', function() { ReactTestUtils.renderIntoDocument(); expect(console.error.calls.length).toBe(1); - expect(console.error.argsForCall[0][0]).toBe( + expect( + console.error.argsForCall[0][0].replace(/\(at .+?:\d+\)/g, '(at **)') + ).toBe( 'Warning: Failed propType: ' + - 'Required prop `prop` was not specified in `RequiredPropComponent`.' + 'Required prop `prop` was not specified in `RequiredPropComponent`.\n' + + ' in RequiredPropComponent (at **)' ); }); @@ -267,15 +277,21 @@ describe('ReactJSXElementValidator', function() { ReactTestUtils.renderIntoDocument(); expect(console.error.calls.length).toBe(2); - expect(console.error.argsForCall[0][0]).toBe( + expect( + console.error.argsForCall[0][0].replace(/\(at .+?:\d+\)/g, '(at **)') + ).toBe( 'Warning: Failed propType: ' + - 'Required prop `prop` was not specified in `RequiredPropComponent`.' + 'Required prop `prop` was not specified in `RequiredPropComponent`.\n' + + ' in RequiredPropComponent (at **)' ); - expect(console.error.argsForCall[1][0]).toBe( + expect( + console.error.argsForCall[1][0].replace(/\(at .+?:\d+\)/g, '(at **)') + ).toBe( 'Warning: Failed propType: ' + 'Invalid prop `prop` of type `number` supplied to ' + - '`RequiredPropComponent`, expected `string`.' + '`RequiredPropComponent`, expected `string`.\n' + + ' in RequiredPropComponent (at **)' ); ReactTestUtils.renderIntoDocument(); diff --git a/src/renderers/dom/shared/ReactDOMComponent.js b/src/renderers/dom/shared/ReactDOMComponent.js index f30fb5796b521..61dc9f87ad6dd 100644 --- a/src/renderers/dom/shared/ReactDOMComponent.js +++ b/src/renderers/dom/shared/ReactDOMComponent.js @@ -253,6 +253,8 @@ if (__DEV__) { var contentDebugID = debugID + '#text'; this._contentDebugID = contentDebugID; ReactInstrumentation.debugTool.onSetDisplayName(contentDebugID, '#text'); + ReactInstrumentation.debugTool.onSetParent(contentDebugID, debugID); + ReactInstrumentation.debugTool.onBeforeMountComponent(contentDebugID); ReactInstrumentation.debugTool.onSetText(contentDebugID, '' + contentToUse); ReactInstrumentation.debugTool.onMountComponent(contentDebugID); ReactInstrumentation.debugTool.onSetChildren(debugID, [contentDebugID]); diff --git a/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js b/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js index badd3e7b45955..8fc797064a7c3 100644 --- a/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js +++ b/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js @@ -471,12 +471,21 @@ var ReactCompositeComponentMixin = { } this._renderedNodeType = ReactNodeTypes.getType(renderedElement); - this._renderedComponent = this._instantiateReactComponent( + var child = this._instantiateReactComponent( renderedElement ); + this._renderedComponent = child; + if (__DEV__) { + if (child._debugID !== 0 && this._debugID !== 0) { + ReactInstrumentation.debugTool.onSetParent( + child._debugID, + this._debugID + ); + } + } var markup = ReactReconciler.mountComponent( - this._renderedComponent, + child, transaction, hostParent, hostContainerInfo, @@ -487,9 +496,7 @@ var ReactCompositeComponentMixin = { if (this._debugID !== 0) { ReactInstrumentation.debugTool.onSetChildren( this._debugID, - this._renderedComponent._debugID !== 0 ? - [this._renderedComponent._debugID] : - [] + child._debugID !== 0 ? [child._debugID] : [] ); } } @@ -1031,12 +1038,21 @@ var ReactCompositeComponentMixin = { ReactReconciler.unmountComponent(prevComponentInstance, false); this._renderedNodeType = ReactNodeTypes.getType(nextRenderedElement); - this._renderedComponent = this._instantiateReactComponent( + var child = this._instantiateReactComponent( nextRenderedElement ); + this._renderedComponent = child; + if (__DEV__) { + if (child._debugID !== 0 && this._debugID !== 0) { + ReactInstrumentation.debugTool.onSetParent( + child._debugID, + this._debugID + ); + } + } var nextMarkup = ReactReconciler.mountComponent( - this._renderedComponent, + child, transaction, this._hostParent, this._hostContainerInfo, @@ -1047,9 +1063,7 @@ var ReactCompositeComponentMixin = { if (this._debugID !== 0) { ReactInstrumentation.debugTool.onSetChildren( this._debugID, - this._renderedComponent._debugID !== 0 ? - [this._renderedComponent._debugID] : - [] + child._debugID !== 0 ? [child._debugID] : [] ); } } diff --git a/src/renderers/shared/stack/reconciler/ReactMultiChild.js b/src/renderers/shared/stack/reconciler/ReactMultiChild.js index 9c10bc9a881f7..8b4c95f0c2bf9 100644 --- a/src/renderers/shared/stack/reconciler/ReactMultiChild.js +++ b/src/renderers/shared/stack/reconciler/ReactMultiChild.js @@ -139,8 +139,14 @@ function processQueue(inst, updateQueue) { ); } +var setParentForInstrumentation = emptyFunction; var setChildrenForInstrumentation = emptyFunction; if (__DEV__) { + setParentForInstrumentation = function(child) { + if (child._debugID !== 0) { + ReactInstrumentation.debugTool.onSetParent(child._debugID, this._debugID); + } + }; setChildrenForInstrumentation = function(children) { ReactInstrumentation.debugTool.onSetChildren( this._debugID, @@ -232,6 +238,9 @@ var ReactMultiChild = { for (var name in children) { if (children.hasOwnProperty(name)) { var child = children[name]; + if (__DEV__) { + setParentForInstrumentation.call(this, child); + } var mountImage = ReactReconciler.mountComponent( child, transaction, diff --git a/src/renderers/shared/stack/reconciler/ReactReconciler.js b/src/renderers/shared/stack/reconciler/ReactReconciler.js index 9c74c9f8f1ef7..149bbedd80e70 100644 --- a/src/renderers/shared/stack/reconciler/ReactReconciler.js +++ b/src/renderers/shared/stack/reconciler/ReactReconciler.js @@ -222,6 +222,10 @@ var ReactReconciler = { internalInstance._debugID, 'performUpdateIfNecessary' ); + ReactInstrumentation.debugTool.onBeforeUpdateComponent( + internalInstance._debugID, + internalInstance._currentElement + ); } } internalInstance.performUpdateIfNecessary(transaction); diff --git a/src/renderers/shared/stack/reconciler/__tests__/ReactStatelessComponent-test.js b/src/renderers/shared/stack/reconciler/__tests__/ReactStatelessComponent-test.js index 8e0c8ac492197..c5b2744cebddc 100644 --- a/src/renderers/shared/stack/reconciler/__tests__/ReactStatelessComponent-test.js +++ b/src/renderers/shared/stack/reconciler/__tests__/ReactStatelessComponent-test.js @@ -175,9 +175,12 @@ describe('ReactStatelessComponent', function() { spyOn(console, 'error'); ReactTestUtils.renderIntoDocument(); expect(console.error.argsForCall.length).toBe(1); - expect(console.error.argsForCall[0][0]).toBe( + expect( + console.error.argsForCall[0][0].replace(/\(at .+?:\d+\)/g, '(at **)') + ).toBe( 'Warning: Failed propType: Invalid prop `test` of type `number` ' + - 'supplied to `Child`, expected `string`.' + 'supplied to `Child`, expected `string`.\n' + + ' in Child (at **)' ); });