@samitouri / QOS-React-2 / commits / d80723b237

Check for missing debugID in ReactDebugTool

TopLevelWrapper and empty components have ID of 0. These are implementation details and we don't want to leak them to devtools.

Dan Abramov committed May 12, 2016 at 00:45 UTC d80723b237dfb37356c9eed241c45c1fd7938adb
8 files changed +68 -28
src/isomorphic/ReactDebugTool.js
+17
@@ -90,6 +90,10 @@ function resetMeasurements() {
90 }
91 }
92
93 +function checkDebugID(debugID) {
94 + warning(debugID, 'ReactDebugTool: debugID may not be empty.');
95 +}
96 +
97 var ReactDebugTool = {
98 addDevtool(devtool) {
99 eventHandlers.push(devtool);
@@ -143,6 +147,7 @@ var ReactDebugTool = {
147 emitEvent('onEndFlush');
148 },
149 onBeginLifeCycleTimer(debugID, timerType) {
150 + checkDebugID(debugID);
151 emitEvent('onBeginLifeCycleTimer', debugID, timerType);
152 if (__DEV__) {
153 if (isProfiling && currentFlushNesting > 0) {
@@ -162,6 +167,7 @@ var ReactDebugTool = {
167 }
168 },
169 onEndLifeCycleTimer(debugID, timerType) {
170 + checkDebugID(debugID);
171 if (__DEV__) {
172 if (isProfiling && currentFlushNesting > 0) {
173 warning(
@@ -186,9 +192,11 @@ var ReactDebugTool = {
192 emitEvent('onEndLifeCycleTimer', debugID, timerType);
193 },
194 onBeginReconcilerTimer(debugID, timerType) {
195 + checkDebugID(debugID);
196 emitEvent('onBeginReconcilerTimer', debugID, timerType);
197 },
198 onEndReconcilerTimer(debugID, timerType) {
199 + checkDebugID(debugID);
200 emitEvent('onEndReconcilerTimer', debugID, timerType);
201 },
202 onBeginProcessingChildContext() {
@@ -198,33 +206,42 @@ var ReactDebugTool = {
206 emitEvent('onEndProcessingChildContext');
207 },
208 onNativeOperation(debugID, type, payload) {
209 + checkDebugID(debugID);
210 emitEvent('onNativeOperation', debugID, type, payload);
211 },
212 onSetState() {
213 emitEvent('onSetState');
214 },
215 onSetDisplayName(debugID, displayName) {
216 + checkDebugID(debugID);
217 emitEvent('onSetDisplayName', debugID, displayName);
218 },
219 onSetChildren(debugID, childDebugIDs) {
220 + checkDebugID(debugID);
221 emitEvent('onSetChildren', debugID, childDebugIDs);
222 },
223 onSetOwner(debugID, ownerDebugID) {
224 + checkDebugID(debugID);
225 emitEvent('onSetOwner', debugID, ownerDebugID);
226 },
227 onSetText(debugID, text) {
228 + checkDebugID(debugID);
229 emitEvent('onSetText', debugID, text);
230 },
231 onMountRootComponent(debugID) {
232 + checkDebugID(debugID);
233 emitEvent('onMountRootComponent', debugID);
234 },
235 onMountComponent(debugID) {
236 + checkDebugID(debugID);
237 emitEvent('onMountComponent', debugID);
238 },
239 onUpdateComponent(debugID) {
240 + checkDebugID(debugID);
241 emitEvent('onUpdateComponent', debugID);
242 },
243 onUnmountComponent(debugID) {
244 + checkDebugID(debugID);
245 emitEvent('onUnmountComponent', debugID);
246 },
247 onTestEvent() {
src/isomorphic/devtools/__tests__/ReactNativeOperationHistoryDevtool-test.js
+14 -7
@@ -72,13 +72,17 @@ describe('ReactNativeOperationHistoryDevtool', () => {
72 var node = document.createElement('div');
73 ReactDOM.render(<Foo />, node);
74 var inst = ReactDOMComponentTree.getInstanceFromNode(node.firstChild);
75 - assertHistoryMatches([{
76 - instanceID: inst._debugID,
77 - type: 'mount',
78 - payload: ReactDOMFeatureFlags.useCreateElement ?
79 - '#comment' :
80 - '<!-- react-empty: 1 -->',
81 - }]);
75 +
76 + if (ReactDOMFeatureFlags.useCreateElement) {
77 + // Empty DOM components should be invisible to devtools.
78 + assertHistoryMatches([]);
79 + } else {
80 + assertHistoryMatches([{
81 + instanceID: inst._debugID,
82 + type: 'mount',
83 + payload: '<!-- react-empty: 1 -->',
84 + }]);
85 + }
86 });
87
88 it('gets recorded when a native is mounted deeply instead of null', () => {
@@ -95,6 +99,9 @@ describe('ReactNativeOperationHistoryDevtool', () => {
99 element = <span />;
100 ReactDOM.render(<Foo />, node);
101 var inst = ReactDOMComponentTree.getInstanceFromNode(node.firstChild);
102 +
103 + // Since empty components should be invisible to devtools,
104 + // we record a "mount" event rather than a "replace with".
105 assertHistoryMatches([{
106 instanceID: inst._debugID,
107 type: 'mount',
src/renderers/dom/client/ReactMount.js
+8 -5
@@ -701,11 +701,14 @@ var ReactMount = {
701 }
702
703 if (__DEV__) {
704 - ReactInstrumentation.debugTool.onNativeOperation(
705 - ReactDOMComponentTree.getInstanceFromNode(container.firstChild)._debugID,
706 - 'mount',
707 - markup.toString()
708 - );
704 + var nativeNode = ReactDOMComponentTree.getInstanceFromNode(container.firstChild);
705 + if (nativeNode._debugID !== 0) {
706 + ReactInstrumentation.debugTool.onNativeOperation(
707 + nativeNode._debugID,
708 + 'mount',
709 + markup.toString()
710 + );
711 + }
712 }
713 },
714 };
src/renderers/dom/client/__tests__/ReactDOMIDOperations-test.js
+1 -1
@@ -18,7 +18,7 @@ describe('ReactDOMIDOperations', function() {
18
19 it('should update innerHTML and preserve whitespace', function() {
20 var stubNode = document.createElement('div');
21 - var stubInstance = {};
21 + var stubInstance = {_debugID: 1};
22 ReactDOMComponentTree.precacheNode(stubInstance, stubNode);
23
24 var html = '\n \t <span> \n testContent \t </span> \n \t';
src/renderers/dom/client/utils/DOMChildrenOperations.js
+8 -5
@@ -142,11 +142,14 @@ if (__DEV__) {
142 markup.toString()
143 );
144 } else {
145 - ReactInstrumentation.debugTool.onNativeOperation(
146 - ReactDOMComponentTree.getInstanceFromNode(markup.node)._debugID,
147 - 'mount',
148 - markup.toString()
149 - );
145 + var nextInstance = ReactDOMComponentTree.getInstanceFromNode(markup.node);
146 + if (nextInstance._debugID !== 0) {
147 + ReactInstrumentation.debugTool.onNativeOperation(
148 + nextInstance._debugID,
149 + 'mount',
150 + markup.toString()
151 + );
152 + }
153 }
154 };
155 }
src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js
+9 -5
@@ -174,10 +174,12 @@ describe('DOMPropertyOperations', function() {
174
175 describe('setValueForProperty', function() {
176 var stubNode;
177 + var stubInstance;
178
179 beforeEach(function() {
180 stubNode = document.createElement('div');
180 - ReactDOMComponentTree.precacheNode({}, stubNode);
181 + stubInstance = {_debugID: 1};
182 + ReactDOMComponentTree.precacheNode(stubInstance, stubNode);
183 });
184
185 it('should set values as properties by default', function() {
@@ -226,7 +228,7 @@ describe('DOMPropertyOperations', function() {
228
229 it('should not remove empty attributes for special properties', function() {
230 stubNode = document.createElement('input');
229 - ReactDOMComponentTree.precacheNode({}, stubNode);
231 + ReactDOMComponentTree.precacheNode(stubInstance, stubNode);
232
233 DOMPropertyOperations.setValueForProperty(stubNode, 'value', '');
234 // JSDOM does not behave correctly for attributes/properties
@@ -348,10 +350,12 @@ describe('DOMPropertyOperations', function() {
350
351 describe('deleteValueForProperty', function() {
352 var stubNode;
353 + var stubInstance;
354
355 beforeEach(function() {
356 stubNode = document.createElement('div');
354 - ReactDOMComponentTree.precacheNode({}, stubNode);
357 + stubInstance = {_debugID: 1};
358 + ReactDOMComponentTree.precacheNode(stubInstance, stubNode);
359 });
360
361 it('should remove attributes for normal properties', function() {
@@ -367,7 +371,7 @@ describe('DOMPropertyOperations', function() {
371
372 it('should not remove attributes for special properties', function() {
373 stubNode = document.createElement('input');
370 - ReactDOMComponentTree.precacheNode({}, stubNode);
374 + ReactDOMComponentTree.precacheNode(stubInstance, stubNode);
375
376 stubNode.setAttribute('value', 'foo');
377
@@ -379,7 +383,7 @@ describe('DOMPropertyOperations', function() {
383
384 it('should not leave all options selected when deleting multiple', function() {
385 stubNode = document.createElement('select');
382 - ReactDOMComponentTree.precacheNode({}, stubNode);
386 + ReactDOMComponentTree.precacheNode(stubInstance, stubNode);
387
388 stubNode.multiple = true;
389 stubNode.appendChild(document.createElement('option'));
src/renderers/shared/stack/reconciler/instantiateReactComponent.js
+7 -5
@@ -139,11 +139,13 @@ function instantiateReactComponent(node) {
139 var debugID = isEmpty ? 0 : nextDebugID++;
140 instance._debugID = debugID;
141
142 - var displayName = getDisplayName(instance);
143 - ReactInstrumentation.debugTool.onSetDisplayName(debugID, displayName);
144 - var owner = node && node._owner;
145 - if (owner) {
146 - ReactInstrumentation.debugTool.onSetOwner(debugID, owner._debugID);
142 + if (debugID !== 0) {
143 + var displayName = getDisplayName(instance);
144 + ReactInstrumentation.debugTool.onSetDisplayName(debugID, displayName);
145 + var owner = node && node._owner;
146 + if (owner) {
147 + ReactInstrumentation.debugTool.onSetOwner(debugID, owner._debugID);
148 + }
149 }
150 }
151
src/test/ReactTestUtils.js
+4
@@ -376,9 +376,12 @@ ReactShallowRenderer.prototype.getMountedInstance = function() {
376 return this._instance ? this._instance._instance : null;
377 };
378
379 +var nextDebugID = 0;
380 +
381 var NoopInternalComponent = function(element) {
382 this._renderedOutput = element;
383 this._currentElement = element;
384 + this._debugID = nextDebugID++;
385 };
386
387 NoopInternalComponent.prototype = {
@@ -404,6 +407,7 @@ NoopInternalComponent.prototype = {
407 };
408
409 var ShallowComponentWrapper = function(element) {
410 + this._debugID = nextDebugID++;
411 this.construct(element);
412 };
413 Object.assign(