Skip to content

Commit

Permalink
Merge pull request facebook#6789 from gaearon/tree-devtool-fixes
Browse files Browse the repository at this point in the history
Make sure element is reported correctly by tree devtool
(cherry picked from commit 7f08961)
  • Loading branch information
gaearon authored and zpao committed Jun 8, 2016
1 parent 05e99eb commit fa735b5
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ describe('ReactComponentTreeDevtool', () => {
var element = <div><Foo /><Baz /><Qux /></div>;
var tree = {
displayName: 'div',
element,
children: [{
displayName: 'Bar',
children: [],
Expand Down Expand Up @@ -203,6 +204,7 @@ describe('ReactComponentTreeDevtool', () => {
var element = <div><Foo /><Baz /><Qux /></div>;
var tree = {
displayName: 'div',
element,
children: [{
displayName: 'Bar',
children: [],
Expand Down Expand Up @@ -233,6 +235,7 @@ describe('ReactComponentTreeDevtool', () => {
var element = <div><Foo /><Baz /><Qux /></div>;
var tree = {
displayName: 'div',
element,
children: [{
displayName: 'Bar',
children: [],
Expand Down Expand Up @@ -275,6 +278,7 @@ describe('ReactComponentTreeDevtool', () => {
}],
}, {
displayName: 'hr',
element: <hr />,
children: [],
}],
};
Expand All @@ -291,8 +295,10 @@ describe('ReactComponentTreeDevtool', () => {
var element = <Foo />;
var tree = {
displayName: 'Foo',
element,
children: [{
displayName: 'div',
element: <div />,
children: [],
}],
};
Expand Down Expand Up @@ -333,12 +339,15 @@ describe('ReactComponentTreeDevtool', () => {
var element = <Baz />;
var tree = {
displayName: 'Baz',
element,
children: [{
displayName: 'div',
children: [{
displayName: 'Foo',
element: <Foo />,
children: [{
displayName: 'Qux',
element: <Qux />,
children: [],
}],
}, {
Expand All @@ -349,18 +358,21 @@ describe('ReactComponentTreeDevtool', () => {
displayName: 'span',
children: [{
displayName: '#text',
element: 'Hi,',
text: 'Hi,',
}],
}, {
displayName: '#text',
text: 'Mom',
element: 'Mom',
}],
}],
}, {
displayName: 'a',
children: [{
displayName: '#text',
text: 'Click me.',
element: 'Click me.',
}],
}],
}],
Expand Down Expand Up @@ -400,6 +412,7 @@ describe('ReactComponentTreeDevtool', () => {
var element = <div>{'1'}{2}</div>;
var tree = {
displayName: 'div',
element,
children: [{
displayName: '#text',
text: '1',
Expand All @@ -415,6 +428,7 @@ describe('ReactComponentTreeDevtool', () => {
var element = <div>{'1'}</div>;
var tree = {
displayName: 'div',
element,
children: [{
displayName: '#text',
text: '1',
Expand All @@ -427,6 +441,7 @@ describe('ReactComponentTreeDevtool', () => {
var element = <div>{42}</div>;
var tree = {
displayName: 'div',
element,
children: [{
displayName: '#text',
text: '42',
Expand All @@ -439,6 +454,7 @@ describe('ReactComponentTreeDevtool', () => {
var element = <div>{0}</div>;
var tree = {
displayName: 'div',
element,
children: [{
displayName: '#text',
text: '0',
Expand All @@ -462,16 +478,21 @@ describe('ReactComponentTreeDevtool', () => {
);
var tree = {
displayName: 'div',
element,
children: [{
displayName: '#text',
text: 'hi',
element: 'hi',
}, {
displayName: '#text',
text: '42',
element: 42,
}, {
displayName: 'Foo',
element: <Foo />,
children: [{
displayName: 'div',
element: <div />,
children: [],
}],
}],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ describe('ReactComponentTreeDevtool', () => {
var element = <View><Foo /><Baz /><Qux /></View>;
var tree = {
displayName: 'View',
element,
children: [{
displayName: 'Bar',
children: [],
Expand Down Expand Up @@ -273,6 +274,7 @@ describe('ReactComponentTreeDevtool', () => {
);
var tree = {
displayName: 'View',
element,
children: [{
displayName: 'View',
children: [{
Expand All @@ -281,12 +283,14 @@ describe('ReactComponentTreeDevtool', () => {
displayName: 'RCText',
children: [{
displayName: '#text',
element: 'Hi!',
text: 'Hi!',
}],
}],
}],
}, {
displayName: 'Image',
element: <Image />,
children: [],
}],
};
Expand All @@ -303,8 +307,10 @@ describe('ReactComponentTreeDevtool', () => {
var element = <Foo />;
var tree = {
displayName: 'Foo',
element,
children: [{
displayName: 'Image',
element: <Image />,
children: [],
}],
};
Expand Down Expand Up @@ -344,12 +350,15 @@ describe('ReactComponentTreeDevtool', () => {
var element = <Baz />;
var tree = {
displayName: 'Baz',
element,
children: [{
displayName: 'View',
children: [{
displayName: 'Foo',
element: <Foo />,
children: [{
displayName: 'Qux',
element: <Qux />,
children: [],
}],
}, {
Expand All @@ -362,13 +371,15 @@ describe('ReactComponentTreeDevtool', () => {
displayName: 'RCText',
children: [{
displayName: '#text',
element: 'Hi,',
text: 'Hi,',
}],
}],
}],
}],
}, {
displayName: 'Image',
element: <Image />,
children: [],
}],
}],
Expand Down Expand Up @@ -481,16 +492,21 @@ describe('ReactComponentTreeDevtool', () => {
);
var tree = {
displayName: 'View',
element,
children: [{
displayName: 'Foo',
element: <Foo />,
children: [{
displayName: 'Image',
element: <Image />,
children: [],
}],
}, {
displayName: 'Foo',
element: <Foo />,
children: [{
displayName: 'Image',
element: <Image />,
children: [],
}],
}],
Expand Down
41 changes: 26 additions & 15 deletions src/renderers/dom/shared/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -247,16 +247,34 @@ function optionPostMount() {

var setContentChildForInstrumentation = emptyFunction;
if (__DEV__) {
setContentChildForInstrumentation = function(contentToUse) {
setContentChildForInstrumentation = function(content) {
var hasExistingContent = this._contentDebugID != null;
var debugID = this._debugID;
var contentDebugID = debugID + '#text';

if (content == null) {
if (hasExistingContent) {
ReactInstrumentation.debugTool.onUnmountComponent(this._contentDebugID);
}
this._contentDebugID = null;
return;
}

this._contentDebugID = contentDebugID;
var text = '' + content;

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]);
ReactInstrumentation.debugTool.onSetText(contentDebugID, text);

if (hasExistingContent) {
ReactInstrumentation.debugTool.onBeforeUpdateComponent(contentDebugID, content);
ReactInstrumentation.debugTool.onUpdateComponent(contentDebugID);
} else {
ReactInstrumentation.debugTool.onBeforeMountComponent(contentDebugID, content);
ReactInstrumentation.debugTool.onMountComponent(contentDebugID);
ReactInstrumentation.debugTool.onSetChildren(debugID, [contentDebugID]);
}
};
}

Expand Down Expand Up @@ -463,7 +481,7 @@ function ReactDOMComponent(element) {
this._flags = 0;
if (__DEV__) {
this._ancestorInfo = null;
this._contentDebugID = null;
setContentChildForInstrumentation.call(this, null);
}
}

Expand Down Expand Up @@ -1035,7 +1053,6 @@ ReactDOMComponent.Mixin = {
if (lastContent !== nextContent) {
this.updateTextContent('' + nextContent);
if (__DEV__) {
this._contentDebugID = this._debugID + '#text';
setContentChildForInstrumentation.call(this, nextContent);
}
}
Expand All @@ -1048,10 +1065,7 @@ ReactDOMComponent.Mixin = {
}
} else if (nextChildren != null) {
if (__DEV__) {
if (this._contentDebugID) {
ReactInstrumentation.debugTool.onUnmountComponent(this._contentDebugID);
this._contentDebugID = null;
}
setContentChildForInstrumentation.call(this, null);
}

this.updateChildren(nextChildren, transaction, context);
Expand Down Expand Up @@ -1113,10 +1127,7 @@ ReactDOMComponent.Mixin = {
this._wrapperState = null;

if (__DEV__) {
if (this._contentDebugID) {
ReactInstrumentation.debugTool.onUnmountComponent(this._contentDebugID);
this._contentDebugID = null;
}
setContentChildForInstrumentation.call(this, null);
}
},

Expand Down
16 changes: 8 additions & 8 deletions src/renderers/shared/stack/reconciler/ReactReconciler.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,14 @@ var ReactReconciler = {
) {
if (__DEV__) {
if (internalInstance._debugID !== 0) {
ReactInstrumentation.debugTool.onBeginReconcilerTimer(
internalInstance._debugID,
'mountComponent'
);
ReactInstrumentation.debugTool.onBeforeMountComponent(
internalInstance._debugID,
internalInstance._currentElement
);
ReactInstrumentation.debugTool.onBeginReconcilerTimer(
internalInstance._debugID,
'mountComponent'
);
}
}
var markup = internalInstance.mountComponent(
Expand Down Expand Up @@ -150,13 +150,13 @@ var ReactReconciler = {

if (__DEV__) {
if (internalInstance._debugID !== 0) {
ReactInstrumentation.debugTool.onBeginReconcilerTimer(
ReactInstrumentation.debugTool.onBeforeUpdateComponent(
internalInstance._debugID,
'receiveComponent'
nextElement
);
ReactInstrumentation.debugTool.onBeforeUpdateComponent(
ReactInstrumentation.debugTool.onBeginReconcilerTimer(
internalInstance._debugID,
internalInstance._currentElement
'receiveComponent'
);
}
}
Expand Down
15 changes: 14 additions & 1 deletion src/test/ReactComponentTreeTestUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,13 @@ function getRegisteredDisplayNames() {
.map(ReactComponentTreeDevtool.getDisplayName);
}

function expectTree(rootID, expectedTree, parentPath = '') {
function expectTree(rootID, expectedTree, parentPath) {
var displayName = ReactComponentTreeDevtool.getDisplayName(rootID);
var ownerID = ReactComponentTreeDevtool.getOwnerID(rootID);
var parentID = ReactComponentTreeDevtool.getParentID(rootID);
var childIDs = ReactComponentTreeDevtool.getChildIDs(rootID);
var text = ReactComponentTreeDevtool.getText(rootID);
var element = ReactComponentTreeDevtool.getElement(rootID);
var path = parentPath ? `${parentPath} > ${displayName}` : displayName;

function expectEqual(actual, expected, name) {
Expand Down Expand Up @@ -62,9 +63,21 @@ function expectTree(rootID, expectedTree, parentPath = '') {
}
if (expectedTree.text !== undefined) {
expectEqual(text, expectedTree.text, 'text');
expectEqual('' + element, expectedTree.text, 'element.toString()');
} else {
expectEqual(text, null, 'text');
}
if (expectedTree.element !== undefined) {
// TODO: Comparing elements makes tests run out of memory on errors.
// For now, compare just types.
expectEqual(
element && element.type,
expectedTree.element && expectedTree.element.type,
'element.type'
);
} else if (text == null) {
expectEqual(typeof element, 'object', 'typeof element');
}
if (expectedTree.children !== undefined) {
expectEqual(
childIDs.length,
Expand Down

0 comments on commit fa735b5

Please sign in to comment.