Skip to content

Commit 82fe64a

Browse files
jimfbjimfb
committed
Merge pull request facebook#5590 from jimfb/use-devtool-for-unknown-property-warning
Use devtool for unknown property warning
2 parents edf1952 + 26f3785 commit 82fe64a

File tree

6 files changed

+202
-95
lines changed

6 files changed

+202
-95
lines changed

‎src/renderers/dom/shared/DOMPropertyOperations.js

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

1414
var DOMProperty = require('DOMProperty');
15-
var EventPluginRegistry = require('EventPluginRegistry');
15+
var ReactDOMInstrumentation = require('ReactDOMInstrumentation');
1616
var ReactPerf = require('ReactPerf');
1717

1818
var quoteAttributeValueForBrowser = require('quoteAttributeValueForBrowser');
@@ -50,59 +50,6 @@ function shouldIgnoreValue(propertyInfo, value) {
5050
(propertyInfo.hasOverloadedBooleanValue && value === false);
5151
}
5252

53-
if (__DEV__) {
54-
var reactProps = {
55-
children: true,
56-
dangerouslySetInnerHTML: true,
57-
key: true,
58-
ref: true,
59-
};
60-
var warnedProperties = {};
61-
62-
var warnUnknownProperty = function(name) {
63-
if (reactProps.hasOwnProperty(name) && reactProps[name] ||
64-
warnedProperties.hasOwnProperty(name) && warnedProperties[name]) {
65-
return;
66-
}
67-
68-
warnedProperties[name] = true;
69-
var lowerCasedName = name.toLowerCase();
70-
71-
// data-* attributes should be lowercase; suggest the lowercase version
72-
var standardName = (
73-
DOMProperty.isCustomAttribute(lowerCasedName) ?
74-
lowerCasedName :
75-
DOMProperty.getPossibleStandardName.hasOwnProperty(lowerCasedName) ?
76-
DOMProperty.getPossibleStandardName[lowerCasedName] :
77-
null
78-
);
79-
80-
// For now, only warn when we have a suggested correction. This prevents
81-
// logging too much when using transferPropsTo.
82-
warning(
83-
standardName == null,
84-
'Unknown DOM property %s. Did you mean %s?',
85-
name,
86-
standardName
87-
);
88-
89-
var registrationName = (
90-
EventPluginRegistry.possibleRegistrationNames.hasOwnProperty(
91-
lowerCasedName
92-
) ?
93-
EventPluginRegistry.possibleRegistrationNames[lowerCasedName] :
94-
null
95-
);
96-
97-
warning(
98-
registrationName == null,
99-
'Unknown event handler property %s. Did you mean `%s`?',
100-
name,
101-
registrationName
102-
);
103-
};
104-
}
105-
10653
/**
10754
* Operations for dealing with DOM properties.
10855
*/
@@ -139,6 +86,9 @@ var DOMPropertyOperations = {
13986
* @return {?string} Markup string, or null if the property was invalid.
14087
*/
14188
createMarkupForProperty: function(name, value) {
89+
if (__DEV__) {
90+
ReactDOMInstrumentation.debugTool.onCreateMarkupForProperty(name, value);
91+
}
14292
var propertyInfo = DOMProperty.properties.hasOwnProperty(name) ?
14393
DOMProperty.properties[name] : null;
14494
if (propertyInfo) {
@@ -156,8 +106,6 @@ var DOMPropertyOperations = {
156106
return '';
157107
}
158108
return name + '=' + quoteAttributeValueForBrowser(value);
159-
} else if (__DEV__) {
160-
warnUnknownProperty(name);
161109
}
162110
return null;
163111
},
@@ -184,6 +132,9 @@ var DOMPropertyOperations = {
184132
* @param {*} value
185133
*/
186134
setValueForProperty: function(node, name, value) {
135+
if (__DEV__) {
136+
ReactDOMInstrumentation.debugTool.onSetValueForProperty(node, name, value);
137+
}
187138
var propertyInfo = DOMProperty.properties.hasOwnProperty(name) ?
188139
DOMProperty.properties[name] : null;
189140
if (propertyInfo) {
@@ -218,8 +169,6 @@ var DOMPropertyOperations = {
218169
}
219170
} else if (DOMProperty.isCustomAttribute(name)) {
220171
DOMPropertyOperations.setValueForAttribute(node, name, value);
221-
} else if (__DEV__) {
222-
warnUnknownProperty(name);
223172
}
224173
},
225174

@@ -241,6 +190,9 @@ var DOMPropertyOperations = {
241190
* @param {string} name
242191
*/
243192
deleteValueForProperty: function(node, name) {
193+
if (__DEV__) {
194+
ReactDOMInstrumentation.debugTool.onDeleteValueForProperty(node, name);
195+
}
244196
var propertyInfo = DOMProperty.properties.hasOwnProperty(name) ?
245197
DOMProperty.properties[name] : null;
246198
if (propertyInfo) {
@@ -262,8 +214,6 @@ var DOMPropertyOperations = {
262214
}
263215
} else if (DOMProperty.isCustomAttribute(name)) {
264216
node.removeAttribute(name);
265-
} else if (__DEV__) {
266-
warnUnknownProperty(name);
267217
}
268218
},
269219

‎src/renderers/dom/shared/ReactDOMDebugTool.js

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
/**
2+
* Copyright 2013-2015, Facebook, Inc.
3+
* All rights reserved.
4+
*
5+
* This source code is licensed under the BSD-style license found in the
6+
* LICENSE file in the root directory of this source tree. An additional grant
7+
* of patent rights can be found in the PATENTS file in the same directory.
8+
*
9+
* @providesModule ReactDOMDebugTool
10+
*/
11+
12+
'use strict';
13+
14+
var ReactDOMUnknownPropertyDevtool = require('ReactDOMUnknownPropertyDevtool');
15+
16+
var warning = require('warning');
17+
18+
var eventHandlers = [];
19+
var handlerDoesThrowForEvent = {};
20+
21+
function emitEvent(handlerFunctionName, arg1, arg2, arg3, arg4, arg5) {
22+
if (__DEV__) {
23+
eventHandlers.forEach(function(handler) {
24+
try {
25+
if (handler[handlerFunctionName]) {
26+
handler[handlerFunctionName](arg1, arg2, arg3, arg4, arg5);
27+
}
28+
} catch (e) {
29+
warning(
30+
!handlerDoesThrowForEvent[handlerFunctionName],
31+
'exception thrown by devtool while handling %s: %s',
32+
handlerFunctionName,
33+
e.message
34+
);
35+
handlerDoesThrowForEvent[handlerFunctionName] = true;
36+
}
37+
});
38+
}
39+
}
40+
41+
var ReactDOMDebugTool = {
42+
addDevtool(devtool) {
43+
eventHandlers.push(devtool);
44+
},
45+
removeDevtool(devtool) {
46+
for (var i = 0; i < eventHandlers.length; i++) {
47+
if (eventHandlers[i] === devtool) {
48+
eventHandlers.splice(i, 1);
49+
i--;
50+
}
51+
}
52+
},
53+
onCreateMarkupForProperty(name, value) {
54+
emitEvent('onCreateMarkupForProperty', name, value);
55+
},
56+
onSetValueForProperty(node, name, value) {
57+
emitEvent('onSetValueForProperty', node, name, value);
58+
},
59+
onDeleteValueForProperty(node, name) {
60+
emitEvent('onDeleteValueForProperty', node, name);
61+
},
62+
};
63+
64+
ReactDOMDebugTool.addDevtool(ReactDOMUnknownPropertyDevtool);
65+
66+
module.exports = ReactDOMDebugTool;

‎src/renderers/dom/shared/ReactDOMInstrumentation.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
/**
2+
* Copyright 2013-2015, Facebook, Inc.
3+
* All rights reserved.
4+
*
5+
* This source code is licensed under the BSD-style license found in the
6+
* LICENSE file in the root directory of this source tree. An additional grant
7+
* of patent rights can be found in the PATENTS file in the same directory.
8+
*
9+
* @providesModule ReactDOMInstrumentation
10+
*/
11+
12+
'use strict';
13+
14+
var ReactDOMDebugTool = require('ReactDOMDebugTool');
15+
16+
module.exports = {debugTool: ReactDOMDebugTool};

‎src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -50,41 +50,6 @@ describe('DOMPropertyOperations', function() {
5050
)).toBe('id="simple"');
5151
});
5252

53-
it('should warn about incorrect casing on properties', function() {
54-
spyOn(console, 'error');
55-
expect(DOMPropertyOperations.createMarkupForProperty(
56-
'tabindex',
57-
'1'
58-
)).toBe(null);
59-
expect(console.error.argsForCall.length).toBe(1);
60-
expect(console.error.argsForCall[0][0]).toContain('tabIndex');
61-
});
62-
63-
it('should warn about incorrect casing on event handlers', function() {
64-
spyOn(console, 'error');
65-
expect(DOMPropertyOperations.createMarkupForProperty(
66-
'onclick',
67-
'1'
68-
)).toBe(null);
69-
expect(DOMPropertyOperations.createMarkupForProperty(
70-
'onKeydown',
71-
'1'
72-
)).toBe(null);
73-
expect(console.error.argsForCall.length).toBe(2);
74-
expect(console.error.argsForCall[0][0]).toContain('onClick');
75-
expect(console.error.argsForCall[1][0]).toContain('onKeyDown');
76-
});
77-
78-
it('should warn about class', function() {
79-
spyOn(console, 'error');
80-
expect(DOMPropertyOperations.createMarkupForProperty(
81-
'class',
82-
'muffins'
83-
)).toBe(null);
84-
expect(console.error.argsForCall.length).toBe(1);
85-
expect(console.error.argsForCall[0][0]).toContain('className');
86-
});
87-
8853
it('should create markup for boolean properties', function() {
8954
expect(DOMPropertyOperations.createMarkupForProperty(
9055
'checked',

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1087,5 +1087,28 @@ describe('ReactDOMComponent', function() {
10871087
'See Link > a > ... > Link > a.'
10881088
);
10891089
});
1090+
1091+
it('should warn about incorrect casing on properties', function() {
1092+
spyOn(console, 'error');
1093+
ReactDOMServer.renderToString(React.createElement('input', {type: 'text', tabindex: '1'}));
1094+
expect(console.error.argsForCall.length).toBe(1);
1095+
expect(console.error.argsForCall[0][0]).toContain('tabIndex');
1096+
});
1097+
1098+
it('should warn about incorrect casing on event handlers', function() {
1099+
spyOn(console, 'error');
1100+
ReactDOMServer.renderToString(React.createElement('input', {type: 'text', onclick: '1'}));
1101+
ReactDOMServer.renderToString(React.createElement('input', {type: 'text', onKeydown: '1'}));
1102+
expect(console.error.argsForCall.length).toBe(2);
1103+
expect(console.error.argsForCall[0][0]).toContain('onClick');
1104+
expect(console.error.argsForCall[1][0]).toContain('onKeyDown');
1105+
});
1106+
1107+
it('should warn about class', function() {
1108+
spyOn(console, 'error');
1109+
ReactDOMServer.renderToString(React.createElement('div', {class: 'muffins'}));
1110+
expect(console.error.argsForCall.length).toBe(1);
1111+
expect(console.error.argsForCall[0][0]).toContain('className');
1112+
});
10901113
});
10911114
});

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

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
/**
2+
* Copyright 2013-2015, Facebook, Inc.
3+
* All rights reserved.
4+
*
5+
* This source code is licensed under the BSD-style license found in the
6+
* LICENSE file in the root directory of this source tree. An additional grant
7+
* of patent rights can be found in the PATENTS file in the same directory.
8+
*
9+
* @providesModule ReactDOMUnknownPropertyDevtool
10+
*/
11+
12+
'use strict';
13+
14+
var DOMProperty = require('DOMProperty');
15+
var EventPluginRegistry = require('EventPluginRegistry');
16+
17+
var warning = require('warning');
18+
19+
if (__DEV__) {
20+
var reactProps = {
21+
children: true,
22+
dangerouslySetInnerHTML: true,
23+
key: true,
24+
ref: true,
25+
};
26+
var warnedProperties = {};
27+
28+
var warnUnknownProperty = function(name) {
29+
if (DOMProperty.properties.hasOwnProperty(name) || DOMProperty.isCustomAttribute(name)) {
30+
return;
31+
}
32+
if (reactProps.hasOwnProperty(name) && reactProps[name] ||
33+
warnedProperties.hasOwnProperty(name) && warnedProperties[name]) {
34+
return;
35+
}
36+
37+
warnedProperties[name] = true;
38+
var lowerCasedName = name.toLowerCase();
39+
40+
// data-* attributes should be lowercase; suggest the lowercase version
41+
var standardName = (
42+
DOMProperty.isCustomAttribute(lowerCasedName) ?
43+
lowerCasedName :
44+
DOMProperty.getPossibleStandardName.hasOwnProperty(lowerCasedName) ?
45+
DOMProperty.getPossibleStandardName[lowerCasedName] :
46+
null
47+
);
48+
49+
// For now, only warn when we have a suggested correction. This prevents
50+
// logging too much when using transferPropsTo.
51+
warning(
52+
standardName == null,
53+
'Unknown DOM property %s. Did you mean %s?',
54+
name,
55+
standardName
56+
);
57+
58+
var registrationName = (
59+
EventPluginRegistry.possibleRegistrationNames.hasOwnProperty(
60+
lowerCasedName
61+
) ?
62+
EventPluginRegistry.possibleRegistrationNames[lowerCasedName] :
63+
null
64+
);
65+
66+
warning(
67+
registrationName == null,
68+
'Unknown event handler property %s. Did you mean `%s`?',
69+
name,
70+
registrationName
71+
);
72+
};
73+
}
74+
75+
var ReactDOMUnknownPropertyDevtool = {
76+
onCreateMarkupForProperty(name, value) {
77+
warnUnknownProperty(name);
78+
},
79+
onSetValueForProperty(node, name, value) {
80+
warnUnknownProperty(name);
81+
},
82+
onDeleteValueForProperty(node, name) {
83+
warnUnknownProperty(name);
84+
},
85+
}
86+
87+
module.exports = ReactDOMUnknownPropertyDevtool;

0 commit comments

Comments
 (0)