From 03f4ba260b7cfe78ea15b34d8169e42fea2c874e Mon Sep 17 00:00:00 2001 From: Ben Alpert Date: Sat, 14 May 2016 12:12:12 -0700 Subject: [PATCH] Track source more generically in ReactComponentTreeDevtool (#6765) 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. --- src/isomorphic/ReactDebugTool.js | 8 ++++++ .../devtools/ReactComponentTreeDevtool.js | 16 ++++++++++++ src/renderers/dom/shared/ReactDOMComponent.js | 9 ------- src/renderers/dom/shared/ReactDOMDebugTool.js | 9 +++---- .../__tests__/ReactDOMComponent-test.js | 19 +++++++++++--- .../ReactDOMUnknownPropertyDevtool.js | 25 +++++++++++-------- .../stack/reconciler/ReactReconciler.js | 8 ++++++ 7 files changed, 65 insertions(+), 29 deletions(-) diff --git a/src/isomorphic/ReactDebugTool.js b/src/isomorphic/ReactDebugTool.js index 5145f8ab314f2..7d55bab6dfb1d 100644 --- a/src/isomorphic/ReactDebugTool.js +++ b/src/isomorphic/ReactDebugTool.js @@ -244,10 +244,18 @@ var ReactDebugTool = { checkDebugID(debugID); emitEvent('onMountRootComponent', debugID); }, + onBeforeMountComponent(debugID, element) { + checkDebugID(debugID); + emitEvent('onBeforeMountComponent', debugID, element); + }, onMountComponent(debugID) { checkDebugID(debugID); emitEvent('onMountComponent', debugID); }, + onBeforeUpdateComponent(debugID, element) { + checkDebugID(debugID); + emitEvent('onBeforeUpdateComponent', debugID, element); + }, onUpdateComponent(debugID) { checkDebugID(debugID); emitEvent('onUpdateComponent', debugID); diff --git a/src/isomorphic/devtools/ReactComponentTreeDevtool.js b/src/isomorphic/devtools/ReactComponentTreeDevtool.js index 85902c30d09cc..353b743e1b01e 100644 --- a/src/isomorphic/devtools/ReactComponentTreeDevtool.js +++ b/src/isomorphic/devtools/ReactComponentTreeDevtool.js @@ -19,6 +19,7 @@ var rootIDs = []; function updateTree(id, update) { if (!tree[id]) { tree[id] = { + element: null, parentID: null, ownerID: null, text: null, @@ -88,6 +89,14 @@ var ReactComponentTreeDevtool = { updateTree(id, item => item.text = text); }, + onBeforeMountComponent(id, element) { + updateTree(id, item => item.element = element); + }, + + onBeforeUpdateComponent(id, element) { + updateTree(id, item => item.element = element); + }, + onMountComponent(id) { updateTree(id, item => item.isMounted = true); }, @@ -141,6 +150,13 @@ var ReactComponentTreeDevtool = { return item ? item.parentID : null; }, + getSource(id) { + var item = tree[id]; + var element = item ? item.element : null; + var source = element != null ? element._source : null; + return source; + }, + getText(id) { var item = tree[id]; return item ? item.text : null; diff --git a/src/renderers/dom/shared/ReactDOMComponent.js b/src/renderers/dom/shared/ReactDOMComponent.js index 161a436acbf78..f30fb5796b521 100644 --- a/src/renderers/dom/shared/ReactDOMComponent.js +++ b/src/renderers/dom/shared/ReactDOMComponent.js @@ -29,7 +29,6 @@ var ReactDOMButton = require('ReactDOMButton'); var ReactDOMComponentFlags = require('ReactDOMComponentFlags'); var ReactDOMComponentTree = require('ReactDOMComponentTree'); var ReactDOMInput = require('ReactDOMInput'); -var ReactDOMInstrumentation = require('ReactDOMInstrumentation'); var ReactDOMOption = require('ReactDOMOption'); var ReactDOMSelect = require('ReactDOMSelect'); var ReactDOMTextarea = require('ReactDOMTextarea'); @@ -579,10 +578,6 @@ ReactDOMComponent.Mixin = { validateDOMNesting.updatedAncestorInfo(parentInfo, this._tag, this); } - if (__DEV__) { - ReactDOMInstrumentation.debugTool.onMountDOMComponent(this._debugID, this._currentElement); - } - var mountImage; if (transaction.useCreateElement) { var ownerDocument = hostContainerInfo._ownerDocument; @@ -858,10 +853,6 @@ ReactDOMComponent.Mixin = { context ); - if (__DEV__) { - ReactDOMInstrumentation.debugTool.onUpdateDOMComponent(this._debugID, this._currentElement); - } - if (this._tag === 'select') { // ); expect(console.error.argsForCall.length).toBe(2); - expect(console.error.argsForCall[0][0]).toMatch(/.*className.*\(.*:\d+\)/); - expect(console.error.argsForCall[1][0]).toMatch(/.*onClick.*\(.*:\d+\)/); + expect( + console.error.argsForCall[0][0].replace(/\(.+?:\d+\)/g, '(**:*)') + ).toBe( + 'Warning: Unknown DOM property class. Did you mean className? (**:*)' + ); + expect( + console.error.argsForCall[1][0].replace(/\(.+?:\d+\)/g, '(**:*)') + ).toBe( + 'Warning: Unknown event handler property onclick. Did you mean ' + + '`onClick`? (**:*)' + ); }); it('gives source code refs for unknown prop warning for update render', function() { @@ -1328,7 +1337,11 @@ describe('ReactDOMComponent', function() { ReactDOMServer.renderToString(
, container); expect(console.error.argsForCall.length).toBe(1); - expect(console.error.argsForCall[0][0]).toMatch(/.*className.*\(.*:\d+\)/); + expect( + console.error.argsForCall[0][0].replace(/\(.+?:\d+\)/g, '(**:*)') + ).toBe( + 'Warning: Unknown DOM property class. Did you mean className? (**:*)' + ); }); it('gives source code refs for unknown prop warning for exact elements ', function() { diff --git a/src/renderers/dom/shared/devtools/ReactDOMUnknownPropertyDevtool.js b/src/renderers/dom/shared/devtools/ReactDOMUnknownPropertyDevtool.js index 5518b3b1ecf51..6845a239df058 100644 --- a/src/renderers/dom/shared/devtools/ReactDOMUnknownPropertyDevtool.js +++ b/src/renderers/dom/shared/devtools/ReactDOMUnknownPropertyDevtool.js @@ -13,11 +13,12 @@ var DOMProperty = require('DOMProperty'); var EventPluginRegistry = require('EventPluginRegistry'); +var ReactComponentTreeDevtool = require('ReactComponentTreeDevtool'); var warning = require('warning'); if (__DEV__) { - var cachedSource; + var lastDebugID; var reactProps = { children: true, dangerouslySetInnerHTML: true, @@ -47,6 +48,10 @@ if (__DEV__) { null ); + var source = ReactComponentTreeDevtool.getSource(lastDebugID); + var formattedSource = source ? + `(${source.fileName.replace(/^.*[\\\/]/, '')}:${source.lineNumber})` : ''; + // For now, only warn when we have a suggested correction. This prevents // logging too much when using transferPropsTo. warning( @@ -54,7 +59,7 @@ if (__DEV__) { 'Unknown DOM property %s. Did you mean %s? %s', name, standardName, - formatSource(cachedSource) + formattedSource ); var registrationName = ( @@ -70,14 +75,10 @@ if (__DEV__) { 'Unknown event handler property %s. Did you mean `%s`? %s', name, registrationName, - formatSource(cachedSource) + formattedSource ); }; - var formatSource = function(source) { - return source ? `(${source.fileName.replace(/^.*[\\\/]/, '')}:${source.lineNumber})` : ''; - }; - } var ReactDOMUnknownPropertyDevtool = { @@ -90,11 +91,13 @@ var ReactDOMUnknownPropertyDevtool = { onDeleteValueForProperty(node, name) { warnUnknownProperty(name); }, - onMountDOMComponent(debugID, element) { - cachedSource = element ? element._source : null; + onBeforeMountComponent(debugID, element) { + // TODO: This currently assumes that properties are all set before recursing + // and mounting children, which needn't be the case in the future. + lastDebugID = debugID; }, - onUpdateDOMComponent(debugID, element) { - cachedSource = element ? element._source : null; + onBeforeUpdateComponent(debugID, element) { + lastDebugID = debugID; }, }; diff --git a/src/renderers/shared/stack/reconciler/ReactReconciler.js b/src/renderers/shared/stack/reconciler/ReactReconciler.js index e9c4251514a39..9c74c9f8f1ef7 100644 --- a/src/renderers/shared/stack/reconciler/ReactReconciler.js +++ b/src/renderers/shared/stack/reconciler/ReactReconciler.js @@ -50,6 +50,10 @@ var ReactReconciler = { internalInstance._debugID, 'mountComponent' ); + ReactInstrumentation.debugTool.onBeforeMountComponent( + internalInstance._debugID, + internalInstance._currentElement + ); } } var markup = internalInstance.mountComponent( @@ -150,6 +154,10 @@ var ReactReconciler = { internalInstance._debugID, 'receiveComponent' ); + ReactInstrumentation.debugTool.onBeforeUpdateComponent( + internalInstance._debugID, + internalInstance._currentElement + ); } }