Skip to content

Commit 346a161

Browse files
committed
Track source more generically in ReactComponentTreeDevtool
Being able to get the source for your parent components seems useful, and ReactComponentTreeDevtool is best poised to be able to do that. I'm also not sure it makes sense to have separate DOM-specific `onMountDOMComponent` and `onUpdateDOMComponent` events, so I removed them for now. Even if we want them, their timing seemed sort of arbitrary. I also made it so DOM devtools can listen to non-DOM events too. Willing to change that if people think it's ugly though.
1 parent 92bebca commit 346a161

7 files changed

Lines changed: 65 additions & 29 deletions

File tree

src/isomorphic/ReactDebugTool.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,10 +241,18 @@ var ReactDebugTool = {
241241
checkDebugID(debugID);
242242
emitEvent('onMountRootComponent', debugID);
243243
},
244+
onBeforeMountComponent(debugID, element) {
245+
checkDebugID(debugID);
246+
emitEvent('onBeforeMountComponent', debugID, element);
247+
},
244248
onMountComponent(debugID) {
245249
checkDebugID(debugID);
246250
emitEvent('onMountComponent', debugID);
247251
},
252+
onBeforeUpdateComponent(debugID, element) {
253+
checkDebugID(debugID);
254+
emitEvent('onBeforeUpdateComponent', debugID, element);
255+
},
248256
onUpdateComponent(debugID) {
249257
checkDebugID(debugID);
250258
emitEvent('onUpdateComponent', debugID);

src/isomorphic/devtools/ReactComponentTreeDevtool.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ var rootIDs = [];
1919
function updateTree(id, update) {
2020
if (!tree[id]) {
2121
tree[id] = {
22+
element: null,
2223
parentID: null,
2324
ownerID: null,
2425
text: null,
@@ -88,6 +89,14 @@ var ReactComponentTreeDevtool = {
8889
updateTree(id, item => item.text = text);
8990
},
9091

92+
onBeforeMountComponent(id, element) {
93+
updateTree(id, item => item.element = element);
94+
},
95+
96+
onBeforeUpdateComponent(id, element) {
97+
updateTree(id, item => item.element = element);
98+
},
99+
91100
onMountComponent(id) {
92101
updateTree(id, item => item.isMounted = true);
93102
},
@@ -141,6 +150,13 @@ var ReactComponentTreeDevtool = {
141150
return item ? item.parentID : null;
142151
},
143152

153+
getSource(id) {
154+
var item = tree[id];
155+
var element = item ? item.element : null;
156+
var source = element != null ? element._source : null;
157+
return source;
158+
},
159+
144160
getText(id) {
145161
var item = tree[id];
146162
return item ? item.text : null;

src/renderers/dom/shared/ReactDOMComponent.js

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ var ReactDOMButton = require('ReactDOMButton');
2929
var ReactDOMComponentFlags = require('ReactDOMComponentFlags');
3030
var ReactDOMComponentTree = require('ReactDOMComponentTree');
3131
var ReactDOMInput = require('ReactDOMInput');
32-
var ReactDOMInstrumentation = require('ReactDOMInstrumentation');
3332
var ReactDOMOption = require('ReactDOMOption');
3433
var ReactDOMSelect = require('ReactDOMSelect');
3534
var ReactDOMTextarea = require('ReactDOMTextarea');
@@ -579,10 +578,6 @@ ReactDOMComponent.Mixin = {
579578
validateDOMNesting.updatedAncestorInfo(parentInfo, this._tag, this);
580579
}
581580

582-
if (__DEV__) {
583-
ReactDOMInstrumentation.debugTool.onMountDOMComponent(this._debugID, this._currentElement);
584-
}
585-
586581
var mountImage;
587582
if (transaction.useCreateElement) {
588583
var ownerDocument = hostContainerInfo._ownerDocument;
@@ -858,10 +853,6 @@ ReactDOMComponent.Mixin = {
858853
context
859854
);
860855

861-
if (__DEV__) {
862-
ReactDOMInstrumentation.debugTool.onUpdateDOMComponent(this._debugID, this._currentElement);
863-
}
864-
865856
if (this._tag === 'select') {
866857
// <select> value update needs to occur after <option> children
867858
// reconciliation

src/renderers/dom/shared/ReactDOMDebugTool.js

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
'use strict';
1313

1414
var ReactDOMUnknownPropertyDevtool = require('ReactDOMUnknownPropertyDevtool');
15+
var ReactDebugTool = require('ReactDebugTool');
1516

1617
var warning = require('warning');
1718

@@ -40,9 +41,11 @@ function emitEvent(handlerFunctionName, arg1, arg2, arg3, arg4, arg5) {
4041

4142
var ReactDOMDebugTool = {
4243
addDevtool(devtool) {
44+
ReactDebugTool.addDevtool(devtool);
4345
eventHandlers.push(devtool);
4446
},
4547
removeDevtool(devtool) {
48+
ReactDebugTool.removeDevtool(devtool);
4649
for (var i = 0; i < eventHandlers.length; i++) {
4750
if (eventHandlers[i] === devtool) {
4851
eventHandlers.splice(i, 1);
@@ -62,12 +65,6 @@ var ReactDOMDebugTool = {
6265
onTestEvent() {
6366
emitEvent('onTestEvent');
6467
},
65-
onMountDOMComponent(debugID, element) {
66-
emitEvent('onMountDOMComponent', debugID, element);
67-
},
68-
onUpdateDOMComponent(debugID, element) {
69-
emitEvent('onMountDOMComponent', debugID, element);
70-
},
7168
};
7269

7370
ReactDOMDebugTool.addDevtool(ReactDOMUnknownPropertyDevtool);

src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1306,8 +1306,17 @@ describe('ReactDOMComponent', function() {
13061306
ReactDOMServer.renderToString(<div class="paladin"/>);
13071307
ReactDOMServer.renderToString(<input type="text" onclick="1"/>);
13081308
expect(console.error.argsForCall.length).toBe(2);
1309-
expect(console.error.argsForCall[0][0]).toMatch(/.*className.*\(.*:\d+\)/);
1310-
expect(console.error.argsForCall[1][0]).toMatch(/.*onClick.*\(.*:\d+\)/);
1309+
expect(
1310+
console.error.argsForCall[0][0].replace(/\(.+?:\d+\)/g, '(**:*)')
1311+
).toBe(
1312+
'Warning: Unknown DOM property class. Did you mean className? (**:*)'
1313+
);
1314+
expect(
1315+
console.error.argsForCall[1][0].replace(/\(.+?:\d+\)/g, '(**:*)')
1316+
).toBe(
1317+
'Warning: Unknown event handler property onclick. Did you mean ' +
1318+
'`onClick`? (**:*)'
1319+
);
13111320
});
13121321

13131322
it('gives source code refs for unknown prop warning for update render', function() {
@@ -1319,7 +1328,11 @@ describe('ReactDOMComponent', function() {
13191328

13201329
ReactDOMServer.renderToString(<div class="paladin" />, container);
13211330
expect(console.error.argsForCall.length).toBe(1);
1322-
expect(console.error.argsForCall[0][0]).toMatch(/.*className.*\(.*:\d+\)/);
1331+
expect(
1332+
console.error.argsForCall[0][0].replace(/\(.+?:\d+\)/g, '(**:*)')
1333+
).toBe(
1334+
'Warning: Unknown DOM property class. Did you mean className? (**:*)'
1335+
);
13231336
});
13241337

13251338
it('gives source code refs for unknown prop warning for exact elements ', function() {

src/renderers/dom/shared/devtools/ReactDOMUnknownPropertyDevtool.js

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,12 @@
1313

1414
var DOMProperty = require('DOMProperty');
1515
var EventPluginRegistry = require('EventPluginRegistry');
16+
var ReactComponentTreeDevtool = require('ReactComponentTreeDevtool');
1617

1718
var warning = require('warning');
1819

1920
if (__DEV__) {
20-
var cachedSource;
21+
var lastDebugID;
2122
var reactProps = {
2223
children: true,
2324
dangerouslySetInnerHTML: true,
@@ -47,14 +48,18 @@ if (__DEV__) {
4748
null
4849
);
4950

51+
var source = ReactComponentTreeDevtool.getSource(lastDebugID);
52+
var formattedSource = source ?
53+
`(${source.fileName.replace(/^.*[\\\/]/, '')}:${source.lineNumber})` : '';
54+
5055
// For now, only warn when we have a suggested correction. This prevents
5156
// logging too much when using transferPropsTo.
5257
warning(
5358
standardName == null,
5459
'Unknown DOM property %s. Did you mean %s? %s',
5560
name,
5661
standardName,
57-
formatSource(cachedSource)
62+
formattedSource
5863
);
5964

6065
var registrationName = (
@@ -70,14 +75,10 @@ if (__DEV__) {
7075
'Unknown event handler property %s. Did you mean `%s`? %s',
7176
name,
7277
registrationName,
73-
formatSource(cachedSource)
78+
formattedSource
7479
);
7580
};
7681

77-
var formatSource = function(source) {
78-
return source ? `(${source.fileName.replace(/^.*[\\\/]/, '')}:${source.lineNumber})` : '';
79-
};
80-
8182
}
8283

8384
var ReactDOMUnknownPropertyDevtool = {
@@ -90,11 +91,13 @@ var ReactDOMUnknownPropertyDevtool = {
9091
onDeleteValueForProperty(node, name) {
9192
warnUnknownProperty(name);
9293
},
93-
onMountDOMComponent(debugID, element) {
94-
cachedSource = element ? element._source : null;
94+
onBeforeMountComponent(debugID, element) {
95+
// TODO: This currently assumes that properties are all set before recursing
96+
// and mounting children, which needn't be the case in the future.
97+
lastDebugID = debugID;
9598
},
96-
onUpdateDOMComponent(debugID, element) {
97-
cachedSource = element ? element._source : null;
99+
onBeforeUpdateComponent(debugID, element) {
100+
lastDebugID = debugID;
98101
},
99102
};
100103

src/renderers/shared/stack/reconciler/ReactReconciler.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,10 @@ var ReactReconciler = {
5050
internalInstance._debugID,
5151
'mountComponent'
5252
);
53+
ReactInstrumentation.debugTool.onBeforeMountComponent(
54+
internalInstance._debugID,
55+
internalInstance._currentElement
56+
);
5357
}
5458
}
5559
var markup = internalInstance.mountComponent(
@@ -150,6 +154,10 @@ var ReactReconciler = {
150154
internalInstance._debugID,
151155
'receiveComponent'
152156
);
157+
ReactInstrumentation.debugTool.onBeforeUpdateComponent(
158+
internalInstance._debugID,
159+
internalInstance._currentElement
160+
);
153161
}
154162
}
155163

0 commit comments

Comments
 (0)