Skip to content

Commit

Permalink
Add warning for unknown properties. (#6800)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jimfb
jimfb committed May 25, 2016
1 parent 4338c8d commit de1de9e
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 38 deletions.
14 changes: 13 additions & 1 deletion src/addons/transitions/ReactTransitionGroup.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -256,9 +256,21 @@ var ReactTransitionGroup = React.createClass({
));
}
}

// Do not forward ReactTransitionGroup props to primitive DOM nodes
var props = Object.assign({}, this.props);
delete props.transitionLeave;
delete props.transitionName;
delete props.transitionAppear;
delete props.transitionEnter;
delete props.childFactory;
delete props.transitionLeaveTimeout;
delete props.transitionEnterTimeout;
delete props.component;

return React.createElement(
this.props.component,
this.props,
props,
childrenToRender
);
},
Expand Down
40 changes: 30 additions & 10 deletions src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ describe('ReactDOMComponent', function() {
var ReactDOMServer;
var inputValueTracking;

function normalizeCodeLocInfo(str) {
return str.replace(/\(at .+?:\d+\)/g, '(at **)');
}

beforeEach(function() {
jest.resetModuleRegistry();
React = require('React');
Expand Down Expand Up @@ -148,6 +152,17 @@ describe('ReactDOMComponent', function() {
expect(console.error.argsForCall.length).toBe(2);
});

it('should warn for unknown prop', function() {
spyOn(console, 'error');
var container = document.createElement('div');
ReactDOM.render(<div foo="bar" />, container);
expect(console.error.argsForCall.length).toBe(1);
expect(normalizeCodeLocInfo(console.error.argsForCall[0][0])).toBe(
'Warning: Unknown prop `foo` on <div> tag. Remove this prop from the element. ' +
'For details, see https://fb.me/react-unknown-prop\n in div (at **)'
);
});

it('should warn about styles with numeric string values for non-unitless properties', function() {
spyOn(console, 'error');

Expand Down Expand Up @@ -1334,15 +1349,15 @@ describe('ReactDOMComponent', function() {
ReactDOMServer.renderToString(<input type="text" onclick="1"/>);
expect(console.error.argsForCall.length).toBe(2);
expect(
console.error.argsForCall[0][0].replace(/\(.+?:\d+\)/g, '(**:*)')
normalizeCodeLocInfo(console.error.argsForCall[0][0])
).toBe(
'Warning: Unknown DOM property class. Did you mean className? (**:*)'
'Warning: Unknown DOM property class. Did you mean className?\n in div (at **)'
);
expect(
console.error.argsForCall[1][0].replace(/\(.+?:\d+\)/g, '(**:*)')
normalizeCodeLocInfo(console.error.argsForCall[1][0])
).toBe(
'Warning: Unknown event handler property onclick. Did you mean ' +
'`onClick`? (**:*)'
'`onClick`?\n in input (at **)'
);
});

Expand All @@ -1356,10 +1371,11 @@ describe('ReactDOMComponent', function() {
ReactDOMServer.renderToString(<div class="paladin" />, container);
expect(console.error.argsForCall.length).toBe(1);
expect(
console.error.argsForCall[0][0].replace(/\(.+?:\d+\)/g, '(**:*)')
normalizeCodeLocInfo(console.error.argsForCall[0][0])
).toBe(
'Warning: Unknown DOM property class. Did you mean className? (**:*)'
'Warning: Unknown DOM property class. Did you mean className?\n in div (at **)'
);

});

it('gives source code refs for unknown prop warning for exact elements ', function() {
Expand All @@ -1377,10 +1393,12 @@ describe('ReactDOMComponent', function() {

expect(console.error.argsForCall.length).toBe(2);

var matches = console.error.argsForCall[0][0].match(/.*className.*\(.*:(\d+)\)/);
expect(console.error.argsForCall[0][0]).toContain('className');
var matches = console.error.argsForCall[0][0].match(/.*\(.*:(\d+)\).*/);
var previousLine = matches[1];

matches = console.error.argsForCall[1][0].match(/.*onClick.*\(.*:(\d+)\)/);
expect(console.error.argsForCall[1][0]).toContain('onClick');
matches = console.error.argsForCall[1][0].match(/.*\(.*:(\d+)\).*/);
var currentLine = matches[1];

//verify line number has a proper relative difference,
Expand Down Expand Up @@ -1426,10 +1444,12 @@ describe('ReactDOMComponent', function() {

expect(console.error.argsForCall.length).toBe(2);

var matches = console.error.argsForCall[0][0].match(/.*className.*\(.*:(\d+)\)/);
expect(console.error.argsForCall[0][0]).toContain('className');
var matches = console.error.argsForCall[0][0].match(/.*\(.*:(\d+)\).*/);
var previousLine = matches[1];

matches = console.error.argsForCall[1][0].match(/.*onClick.*\(.*:(\d+)\)/);
expect(console.error.argsForCall[1][0]).toContain('onClick');
matches = console.error.argsForCall[1][0].match(/.*\(.*:(\d+)\).*/);
var currentLine = matches[1];

//verify line number has a proper relative difference,
Expand Down
74 changes: 47 additions & 27 deletions src/renderers/dom/shared/devtools/ReactDOMUnknownPropertyDevtool.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

var DOMProperty = require('DOMProperty');
var EventPluginRegistry = require('EventPluginRegistry');
var ReactComponentTreeDevtool = require('ReactComponentTreeDevtool');

var warning = require('warning');

Expand All @@ -22,10 +23,19 @@ if (__DEV__) {
dangerouslySetInnerHTML: true,
key: true,
ref: true,

defaultValue: true,
valueLink: true,
defaultChecked: true,
checkedLink: true,
innerHTML: true,
suppressContentEditableWarning: true,
onFocusIn: true,
onFocusOut: true,
};
var warnedProperties = {};

var warnUnknownProperty = function(name, source) {
var warnUnknownProperty = function(tagName, name, debugID) {
if (DOMProperty.properties.hasOwnProperty(name) || DOMProperty.isCustomAttribute(name)) {
return;
}
Expand All @@ -48,16 +58,6 @@ if (__DEV__) {
null
);

// For now, only warn when we have a suggested correction. This prevents
// logging too much when using transferPropsTo.
warning(
standardName == null,
'Unknown DOM property %s. Did you mean %s? %s',
name,
standardName,
formatSource(source)
);

var registrationName = (
EventPluginRegistry.possibleRegistrationNames.hasOwnProperty(
lowerCasedName
Expand All @@ -66,36 +66,56 @@ if (__DEV__) {
null
);

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

var formatSource = function(source) {
return source ? `(${source.fileName.replace(/^.*[\\\/]/, '')}:${source.lineNumber})` : '';
if (standardName != null) {
warning(
standardName == null,
'Unknown DOM property %s. Did you mean %s?%s',
name,
standardName,
ReactComponentTreeDevtool.getStackAddendumByID(debugID)
);
} else if (registrationName != null) {
warning(
registrationName == null,
'Unknown event handler property %s. Did you mean `%s`?%s',
name,
registrationName,
ReactComponentTreeDevtool.getStackAddendumByID(debugID)
);
} else {
// We were unable to guess which prop the user intended.
// It is likely that the user was just blindly spreading/forwarding props
// Components should be careful to only render valid props/attributes.
warning(
false,
'Unknown prop `%s` on <%s> tag. Remove this prop from the element. ' +
'For details, see https://fb.me/react-unknown-prop%s',
name,
tagName,
ReactComponentTreeDevtool.getStackAddendumByID(debugID)
);
}
};

}

function handleElement(element) {
function handleElement(debugID, element) {
if (element == null || typeof element.type !== 'string') {
return;
}
if (element.type.indexOf('-') >= 0 || element.props.is) {
return;
}
for (var key in element.props) {
warnUnknownProperty(key, element._source);
warnUnknownProperty(element.type, key, debugID);
}
}

var ReactDOMUnknownPropertyDevtool = {
onBeforeMountComponent(debugID, element) {
handleElement(element);
handleElement(debugID, element);
},
onBeforeUpdateComponent(debugID, element) {
handleElement(element);
handleElement(debugID, element);
},
};

Expand Down

0 comments on commit de1de9e

Please sign in to comment.