Merge pull request #6752 from facebook/fix-6742

Fix ReactPerf.printOperations() crash
(cherry picked from commit 027d9a919b)
This commit is contained in:
Dan Abramov
2016-05-12 02:59:46 +01:00
committed by Paul O’Shannessy
parent e15a7d1f7e
commit 95db5bab42
9 changed files with 139 additions and 25 deletions
+17
View File
@@ -90,6 +90,10 @@ function resetMeasurements() {
}
}
function checkDebugID(debugID) {
warning(debugID, 'ReactDebugTool: debugID may not be empty.');
}
var ReactDebugTool = {
addDevtool(devtool) {
eventHandlers.push(devtool);
@@ -143,6 +147,7 @@ var ReactDebugTool = {
emitEvent('onEndFlush');
},
onBeginLifeCycleTimer(debugID, timerType) {
checkDebugID(debugID);
emitEvent('onBeginLifeCycleTimer', debugID, timerType);
if (__DEV__) {
if (isProfiling && currentFlushNesting > 0) {
@@ -162,6 +167,7 @@ var ReactDebugTool = {
}
},
onEndLifeCycleTimer(debugID, timerType) {
checkDebugID(debugID);
if (__DEV__) {
if (isProfiling && currentFlushNesting > 0) {
warning(
@@ -186,9 +192,11 @@ var ReactDebugTool = {
emitEvent('onEndLifeCycleTimer', debugID, timerType);
},
onBeginReconcilerTimer(debugID, timerType) {
checkDebugID(debugID);
emitEvent('onBeginReconcilerTimer', debugID, timerType);
},
onEndReconcilerTimer(debugID, timerType) {
checkDebugID(debugID);
emitEvent('onEndReconcilerTimer', debugID, timerType);
},
onBeginProcessingChildContext() {
@@ -198,33 +206,42 @@ var ReactDebugTool = {
emitEvent('onEndProcessingChildContext');
},
onNativeOperation(debugID, type, payload) {
checkDebugID(debugID);
emitEvent('onNativeOperation', debugID, type, payload);
},
onSetState() {
emitEvent('onSetState');
},
onSetDisplayName(debugID, displayName) {
checkDebugID(debugID);
emitEvent('onSetDisplayName', debugID, displayName);
},
onSetChildren(debugID, childDebugIDs) {
checkDebugID(debugID);
emitEvent('onSetChildren', debugID, childDebugIDs);
},
onSetOwner(debugID, ownerDebugID) {
checkDebugID(debugID);
emitEvent('onSetOwner', debugID, ownerDebugID);
},
onSetText(debugID, text) {
checkDebugID(debugID);
emitEvent('onSetText', debugID, text);
},
onMountRootComponent(debugID) {
checkDebugID(debugID);
emitEvent('onMountRootComponent', debugID);
},
onMountComponent(debugID) {
checkDebugID(debugID);
emitEvent('onMountComponent', debugID);
},
onUpdateComponent(debugID) {
checkDebugID(debugID);
emitEvent('onUpdateComponent', debugID);
},
onUnmountComponent(debugID) {
checkDebugID(debugID);
emitEvent('onUnmountComponent', debugID);
},
};
@@ -67,6 +67,13 @@ describe('ReactPerf', function() {
ReactPerf.start();
fn();
ReactPerf.stop();
// Make sure none of the methods crash.
ReactPerf.getWasted();
ReactPerf.getInclusive();
ReactPerf.getExclusive();
ReactPerf.getOperations();
return ReactPerf.getLastMeasurements();
}
@@ -208,6 +215,32 @@ describe('ReactPerf', function() {
});
});
it('should not count replacing null with a native as waste', function() {
var element = null;
function Foo() {
return element;
}
var container = document.createElement('div');
ReactDOM.render(<Foo />, container);
expectNoWaste(() => {
element = <div />;
ReactDOM.render(<Foo />, container);
});
});
it('should not count replacing a native with null as waste', function() {
var element = <div />;
function Foo() {
return element;
}
var container = document.createElement('div');
ReactDOM.render(<Foo />, container);
expectNoWaste(() => {
element = null;
ReactDOM.render(<Foo />, container);
});
});
it('warns once when using getMeasurementsSummaryMap', function() {
var measurements = measure(() => {});
spyOn(console, 'error');
@@ -65,19 +65,38 @@ describe('ReactNativeOperationHistoryDevtool', () => {
}]);
});
it('gets recorded for composite roots that return null', () => {
it('gets ignored for composite roots that return null', () => {
function Foo() {
return null;
}
var node = document.createElement('div');
ReactDOM.render(<Foo />, node);
// Empty DOM components should be invisible to devtools.
assertHistoryMatches([]);
});
it('gets recorded when a native is mounted deeply instead of null', () => {
var element;
function Foo() {
return element;
}
var node = document.createElement('div');
element = null;
ReactDOM.render(<Foo />, node);
ReactNativeOperationHistoryDevtool.clearHistory();
element = <span />;
ReactDOM.render(<Foo />, node);
var inst = ReactDOMComponentTree.getInstanceFromNode(node.firstChild);
// Since empty components should be invisible to devtools,
// we record a "mount" event rather than a "replace with".
assertHistoryMatches([{
instanceID: inst._debugID,
type: 'mount',
payload: ReactDOMFeatureFlags.useCreateElement ?
'#comment' :
'<!-- react-empty: 1 -->',
payload: 'SPAN',
}]);
});
});
@@ -500,6 +519,27 @@ describe('ReactNativeOperationHistoryDevtool', () => {
}]);
});
it('gets recorded when composite renders to null after a native', () => {
var element;
function Foo() {
return element;
}
var node = document.createElement('div');
element = <span />;
ReactDOM.render(<Foo />, node);
var inst = ReactDOMComponentTree.getInstanceFromNode(node.firstChild);
ReactNativeOperationHistoryDevtool.clearHistory();
element = null;
ReactDOM.render(<Foo />, node);
assertHistoryMatches([{
instanceID: inst._debugID,
type: 'replace with',
payload: '#comment',
}]);
});
it('gets ignored if the type has not changed', () => {
var element;
function Foo() {
+8 -5
View File
@@ -701,11 +701,14 @@ var ReactMount = {
}
if (__DEV__) {
ReactInstrumentation.debugTool.onNativeOperation(
ReactDOMComponentTree.getInstanceFromNode(container.firstChild)._debugID,
'mount',
markup.toString()
);
var nativeNode = ReactDOMComponentTree.getInstanceFromNode(container.firstChild);
if (nativeNode._debugID !== 0) {
ReactInstrumentation.debugTool.onNativeOperation(
nativeNode._debugID,
'mount',
markup.toString()
);
}
}
},
};
@@ -18,7 +18,7 @@ describe('ReactDOMIDOperations', function() {
it('should update innerHTML and preserve whitespace', function() {
var stubNode = document.createElement('div');
var stubInstance = {};
var stubInstance = {_debugID: 1};
ReactDOMComponentTree.precacheNode(stubInstance, stubNode);
var html = '\n \t <span> \n testContent \t </span> \n \t';
@@ -135,11 +135,22 @@ var dangerouslyReplaceNodeWithMarkup = Danger.dangerouslyReplaceNodeWithMarkup;
if (__DEV__) {
dangerouslyReplaceNodeWithMarkup = function(oldChild, markup, prevInstance) {
Danger.dangerouslyReplaceNodeWithMarkup(oldChild, markup);
ReactInstrumentation.debugTool.onNativeOperation(
prevInstance._debugID,
'replace with',
markup.toString()
);
if (prevInstance._debugID !== 0) {
ReactInstrumentation.debugTool.onNativeOperation(
prevInstance._debugID,
'replace with',
markup.toString()
);
} else {
var nextInstance = ReactDOMComponentTree.getInstanceFromNode(markup.node);
if (nextInstance._debugID !== 0) {
ReactInstrumentation.debugTool.onNativeOperation(
nextInstance._debugID,
'mount',
markup.toString()
);
}
}
};
}
@@ -174,10 +174,12 @@ describe('DOMPropertyOperations', function() {
describe('setValueForProperty', function() {
var stubNode;
var stubInstance;
beforeEach(function() {
stubNode = document.createElement('div');
ReactDOMComponentTree.precacheNode({}, stubNode);
stubInstance = {_debugID: 1};
ReactDOMComponentTree.precacheNode(stubInstance, stubNode);
});
it('should set values as properties by default', function() {
@@ -226,7 +228,7 @@ describe('DOMPropertyOperations', function() {
it('should not remove empty attributes for special properties', function() {
stubNode = document.createElement('input');
ReactDOMComponentTree.precacheNode({}, stubNode);
ReactDOMComponentTree.precacheNode(stubInstance, stubNode);
DOMPropertyOperations.setValueForProperty(stubNode, 'value', '');
// JSDOM does not behave correctly for attributes/properties
@@ -348,10 +350,12 @@ describe('DOMPropertyOperations', function() {
describe('deleteValueForProperty', function() {
var stubNode;
var stubInstance;
beforeEach(function() {
stubNode = document.createElement('div');
ReactDOMComponentTree.precacheNode({}, stubNode);
stubInstance = {_debugID: 1};
ReactDOMComponentTree.precacheNode(stubInstance, stubNode);
});
it('should remove attributes for normal properties', function() {
@@ -367,7 +371,7 @@ describe('DOMPropertyOperations', function() {
it('should not remove attributes for special properties', function() {
stubNode = document.createElement('input');
ReactDOMComponentTree.precacheNode({}, stubNode);
ReactDOMComponentTree.precacheNode(stubInstance, stubNode);
stubNode.setAttribute('value', 'foo');
@@ -379,7 +383,7 @@ describe('DOMPropertyOperations', function() {
it('should not leave all options selected when deleting multiple', function() {
stubNode = document.createElement('select');
ReactDOMComponentTree.precacheNode({}, stubNode);
ReactDOMComponentTree.precacheNode(stubInstance, stubNode);
stubNode.multiple = true;
stubNode.appendChild(document.createElement('option'));
@@ -144,11 +144,13 @@ function instantiateReactComponent(node) {
var debugID = isEmpty ? 0 : nextDebugID++;
instance._debugID = debugID;
var displayName = getDisplayName(instance);
ReactInstrumentation.debugTool.onSetDisplayName(debugID, displayName);
var owner = node && node._owner;
if (owner) {
ReactInstrumentation.debugTool.onSetOwner(debugID, owner._debugID);
if (debugID !== 0) {
var displayName = getDisplayName(instance);
ReactInstrumentation.debugTool.onSetDisplayName(debugID, displayName);
var owner = node && node._owner;
if (owner) {
ReactInstrumentation.debugTool.onSetOwner(debugID, owner._debugID);
}
}
}
+4
View File
@@ -376,9 +376,12 @@ ReactShallowRenderer.prototype.getMountedInstance = function() {
return this._instance ? this._instance._instance : null;
};
var nextDebugID = 1;
var NoopInternalComponent = function(element) {
this._renderedOutput = element;
this._currentElement = element;
this._debugID = nextDebugID++;
};
NoopInternalComponent.prototype = {
@@ -404,6 +407,7 @@ NoopInternalComponent.prototype = {
};
var ShallowComponentWrapper = function(element) {
this._debugID = nextDebugID++;
this.construct(element);
};
Object.assign(