Skip to content

Commit

Permalink
New approach for 6062 fix : Show source line number on unknown proper…
Browse files Browse the repository at this point in the history
…ty warning
  • Loading branch information
troydemonbreun committed Apr 15, 2016
1 parent 4016e71 commit 55d7007
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 9 deletions.
3 changes: 2 additions & 1 deletion .babelrc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"transform-es2015-modules-commonjs",
"transform-es3-member-expression-literals",
"transform-es3-property-literals",
"./scripts/babel/transform-object-assign-require"
"./scripts/babel/transform-object-assign-require",
"babel-preset-react/node_modules/babel-plugin-transform-react-jsx-source"
]
}
3 changes: 3 additions & 0 deletions src/renderers/dom/shared/ReactDOMDebugTool.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ var ReactDOMDebugTool = {
onDeleteValueForProperty(node, name) {
emitEvent('onDeleteValueForProperty', node, name);
},
onInstantiateReactComponent(instance) {
emitEvent('onInstantiateReactComponent', instance);
},
};

ReactDOMDebugTool.addDevtool(ReactDOMUnknownPropertyDevtool);
Expand Down
9 changes: 9 additions & 0 deletions src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1237,5 +1237,14 @@ describe('ReactDOMComponent', function() {
ReactTestUtils.renderIntoDocument(<div onFocusOut={() => {}} />);
expect(console.error.argsForCall.length).toBe(2);
});

it('gives source code refs for unknown property warnings', function() {
spyOn(console, 'error');
ReactDOMServer.renderToString(<input type="text" onclick="1"/>);
ReactDOMServer.renderToString(<div class="muffins"/>);
expect(console.error.argsForCall.length).toBe(2);
expect(console.error.argsForCall[0][0]).toMatch(/.*\(.*:\d+\)/);
expect(console.error.argsForCall[1][0]).toMatch(/.*\(.*:\d+\)/);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ var EventPluginRegistry = require('EventPluginRegistry');
var warning = require('warning');

if (__DEV__) {
var cachedSource;
var reactProps = {
children: true,
dangerouslySetInnerHTML: true,
Expand All @@ -25,7 +26,7 @@ if (__DEV__) {
};
var warnedProperties = {};

var warnUnknownProperty = function(name) {
var warnUnknownProperty = function(name, source) {
if (DOMProperty.properties.hasOwnProperty(name) || DOMProperty.isCustomAttribute(name)) {
return;
}
Expand All @@ -50,9 +51,10 @@ if (__DEV__) {
// logging too much when using transferPropsTo.
warning(
standardName == null,
'Unknown DOM property %s. Did you mean %s?',
'Unknown DOM property %s. Did you mean %s? %s',
name,
standardName
standardName,
formatSource(source)
);

var registrationName = (
Expand All @@ -65,22 +67,31 @@ if (__DEV__) {

warning(
registrationName == null,
'Unknown event handler property %s. Did you mean `%s`?',
'Unknown event handler property %s. Did you mean `%s`? %s',
name,
registrationName
registrationName,
formatSource(source)
);
};

var formatSource = function(source) {
return source ? `(${source.fileName.replace(/^.*[\\\/]/, '')}:${source.lineNumber})` : '';
};
}

var ReactDOMUnknownPropertyDevtool = {
onCreateMarkupForProperty(name, value) {
warnUnknownProperty(name);
warnUnknownProperty(name, cachedSource);
},
onSetValueForProperty(node, name, value) {
warnUnknownProperty(name);
warnUnknownProperty(name, cachedSource);
},
onDeleteValueForProperty(node, name) {
warnUnknownProperty(name);
warnUnknownProperty(name, cachedSource);
},
onInstantiateReactComponent(instance) {
//Get JSX _source for use in warnings
cachedSource = instance._currentElement ? instance._currentElement._source : null;
},
};

Expand Down
5 changes: 5 additions & 0 deletions src/renderers/shared/reconciler/instantiateReactComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
'use strict';

var ReactCompositeComponent = require('ReactCompositeComponent');
var ReactDOMInstrumentation = require('ReactDOMInstrumentation');
var ReactEmptyComponent = require('ReactEmptyComponent');
var ReactNativeComponent = require('ReactNativeComponent');

Expand Down Expand Up @@ -129,6 +130,10 @@ function instantiateReactComponent(node) {
}
}

if (__DEV__) {
ReactDOMInstrumentation.debugTool.onInstantiateReactComponent(instance);
}

return instance;
}

Expand Down

0 comments on commit 55d7007

Please sign in to comment.