Skip to content

Commit

Permalink
Add warning for unknown properties.
Browse files Browse the repository at this point in the history
  • Loading branch information
@jimfb
jimfb authored and jim committed May 23, 2016
1 parent 799eae2 commit 8e102ef
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 41 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
59 changes: 46 additions & 13 deletions src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,20 @@ 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(console.error.argsForCall[0][0]).toContain(
'Warning: Unknown prop `foo` on <div> tag. Remove this prop from the element. ' +
'For details, see https://fb.me/react-unknown-prop'
);
expect(console.error.argsForCall[0][0]).toContain(
'ReactDOMComponent-test.js'
);
});

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

Expand Down Expand Up @@ -1332,15 +1346,25 @@ 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, '(**:*)')
).toBe(
'Warning: Unknown DOM property class. Did you mean className? (**:*)'
console.error.argsForCall[0][0]
).toContain(
'Warning: Unknown DOM property class. Did you mean className?'
);
expect(
console.error.argsForCall[1][0].replace(/\(.+?:\d+\)/g, '(**:*)')
).toBe(
console.error.argsForCall[0][0]
).toContain(
'ReactDOMComponent-test.js'
);
expect(
console.error.argsForCall[1][0]
).toContain(
'Warning: Unknown event handler property onclick. Did you mean ' +
'`onClick`? (**:*)'
'`onClick`?'
);
expect(
console.error.argsForCall[1][0]
).toContain(
'ReactDOMComponent-test.js'
);
});

Expand All @@ -1354,9 +1378,14 @@ 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, '(**:*)')
).toBe(
'Warning: Unknown DOM property class. Did you mean className? (**:*)'
console.error.argsForCall[0][0]
).toContain(
'Warning: Unknown DOM property class. Did you mean className?'
);
expect(
console.error.argsForCall[0][0]
).toContain(
'ReactDOMComponent-test.js'
);
});

Expand All @@ -1375,10 +1404,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 @@ -1424,10 +1455,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, source, 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(
tagName.indexOf('-') >= 0,
'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.props.is) {
return;
}
for (var key in element.props) {
warnUnknownProperty(key, element._source);
warnUnknownProperty(element.type, key, element._source, 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 8e102ef

Please sign in to comment.