Skip to content

Commit

Permalink
Merge pull request facebook#5590 from jimfb/use-devtool-for-unknown-p…
Browse files Browse the repository at this point in the history
…roperty-warning

Use devtool for unknown property warning
  • Loading branch information
jimfb committed Dec 24, 2015
2 parents edf1952 + 26f3785 commit 82fe64a
Show file tree
Hide file tree
Showing 6 changed files with 202 additions and 95 deletions.
70 changes: 10 additions & 60 deletions src/renderers/dom/shared/DOMPropertyOperations.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
'use strict';

var DOMProperty = require('DOMProperty');
var EventPluginRegistry = require('EventPluginRegistry');
var ReactDOMInstrumentation = require('ReactDOMInstrumentation');
var ReactPerf = require('ReactPerf');

var quoteAttributeValueForBrowser = require('quoteAttributeValueForBrowser');
Expand Down Expand Up @@ -50,59 +50,6 @@ function shouldIgnoreValue(propertyInfo, value) {
(propertyInfo.hasOverloadedBooleanValue && value === false);
}

if (__DEV__) {
var reactProps = {
children: true,
dangerouslySetInnerHTML: true,
key: true,
ref: true,
};
var warnedProperties = {};

var warnUnknownProperty = function(name) {
if (reactProps.hasOwnProperty(name) && reactProps[name] ||
warnedProperties.hasOwnProperty(name) && warnedProperties[name]) {
return;
}

warnedProperties[name] = true;
var lowerCasedName = name.toLowerCase();

// data-* attributes should be lowercase; suggest the lowercase version
var standardName = (
DOMProperty.isCustomAttribute(lowerCasedName) ?
lowerCasedName :
DOMProperty.getPossibleStandardName.hasOwnProperty(lowerCasedName) ?
DOMProperty.getPossibleStandardName[lowerCasedName] :
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?',
name,
standardName
);

var registrationName = (
EventPluginRegistry.possibleRegistrationNames.hasOwnProperty(
lowerCasedName
) ?
EventPluginRegistry.possibleRegistrationNames[lowerCasedName] :
null
);

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

/**
* Operations for dealing with DOM properties.
*/
Expand Down Expand Up @@ -139,6 +86,9 @@ var DOMPropertyOperations = {
* @return {?string} Markup string, or null if the property was invalid.
*/
createMarkupForProperty: function(name, value) {
if (__DEV__) {
ReactDOMInstrumentation.debugTool.onCreateMarkupForProperty(name, value);
}
var propertyInfo = DOMProperty.properties.hasOwnProperty(name) ?
DOMProperty.properties[name] : null;
if (propertyInfo) {
Expand All @@ -156,8 +106,6 @@ var DOMPropertyOperations = {
return '';
}
return name + '=' + quoteAttributeValueForBrowser(value);
} else if (__DEV__) {
warnUnknownProperty(name);
}
return null;
},
Expand All @@ -184,6 +132,9 @@ var DOMPropertyOperations = {
* @param {*} value
*/
setValueForProperty: function(node, name, value) {
if (__DEV__) {
ReactDOMInstrumentation.debugTool.onSetValueForProperty(node, name, value);
}
var propertyInfo = DOMProperty.properties.hasOwnProperty(name) ?
DOMProperty.properties[name] : null;
if (propertyInfo) {
Expand Down Expand Up @@ -218,8 +169,6 @@ var DOMPropertyOperations = {
}
} else if (DOMProperty.isCustomAttribute(name)) {
DOMPropertyOperations.setValueForAttribute(node, name, value);
} else if (__DEV__) {
warnUnknownProperty(name);
}
},

Expand All @@ -241,6 +190,9 @@ var DOMPropertyOperations = {
* @param {string} name
*/
deleteValueForProperty: function(node, name) {
if (__DEV__) {
ReactDOMInstrumentation.debugTool.onDeleteValueForProperty(node, name);
}
var propertyInfo = DOMProperty.properties.hasOwnProperty(name) ?
DOMProperty.properties[name] : null;
if (propertyInfo) {
Expand All @@ -262,8 +214,6 @@ var DOMPropertyOperations = {
}
} else if (DOMProperty.isCustomAttribute(name)) {
node.removeAttribute(name);
} else if (__DEV__) {
warnUnknownProperty(name);
}
},

Expand Down
66 changes: 66 additions & 0 deletions src/renderers/dom/shared/ReactDOMDebugTool.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/**
* Copyright 2013-2015, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
* @providesModule ReactDOMDebugTool
*/

'use strict';

var ReactDOMUnknownPropertyDevtool = require('ReactDOMUnknownPropertyDevtool');

var warning = require('warning');

var eventHandlers = [];
var handlerDoesThrowForEvent = {};

function emitEvent(handlerFunctionName, arg1, arg2, arg3, arg4, arg5) {
if (__DEV__) {
eventHandlers.forEach(function(handler) {
try {
if (handler[handlerFunctionName]) {
handler[handlerFunctionName](arg1, arg2, arg3, arg4, arg5);
}
} catch (e) {
warning(
!handlerDoesThrowForEvent[handlerFunctionName],
'exception thrown by devtool while handling %s: %s',
handlerFunctionName,
e.message
);
handlerDoesThrowForEvent[handlerFunctionName] = true;
}
});
}
}

var ReactDOMDebugTool = {
addDevtool(devtool) {
eventHandlers.push(devtool);
},
removeDevtool(devtool) {
for (var i = 0; i < eventHandlers.length; i++) {
if (eventHandlers[i] === devtool) {
eventHandlers.splice(i, 1);
i--;
}
}
},
onCreateMarkupForProperty(name, value) {
emitEvent('onCreateMarkupForProperty', name, value);
},
onSetValueForProperty(node, name, value) {
emitEvent('onSetValueForProperty', node, name, value);
},
onDeleteValueForProperty(node, name) {
emitEvent('onDeleteValueForProperty', node, name);
},
};

ReactDOMDebugTool.addDevtool(ReactDOMUnknownPropertyDevtool);

module.exports = ReactDOMDebugTool;
16 changes: 16 additions & 0 deletions src/renderers/dom/shared/ReactDOMInstrumentation.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/**
* Copyright 2013-2015, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
* @providesModule ReactDOMInstrumentation
*/

'use strict';

var ReactDOMDebugTool = require('ReactDOMDebugTool');

module.exports = {debugTool: ReactDOMDebugTool};
35 changes: 0 additions & 35 deletions src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,41 +50,6 @@ describe('DOMPropertyOperations', function() {
)).toBe('id="simple"');
});

it('should warn about incorrect casing on properties', function() {
spyOn(console, 'error');
expect(DOMPropertyOperations.createMarkupForProperty(
'tabindex',
'1'
)).toBe(null);
expect(console.error.argsForCall.length).toBe(1);
expect(console.error.argsForCall[0][0]).toContain('tabIndex');
});

it('should warn about incorrect casing on event handlers', function() {
spyOn(console, 'error');
expect(DOMPropertyOperations.createMarkupForProperty(
'onclick',
'1'
)).toBe(null);
expect(DOMPropertyOperations.createMarkupForProperty(
'onKeydown',
'1'
)).toBe(null);
expect(console.error.argsForCall.length).toBe(2);
expect(console.error.argsForCall[0][0]).toContain('onClick');
expect(console.error.argsForCall[1][0]).toContain('onKeyDown');
});

it('should warn about class', function() {
spyOn(console, 'error');
expect(DOMPropertyOperations.createMarkupForProperty(
'class',
'muffins'
)).toBe(null);
expect(console.error.argsForCall.length).toBe(1);
expect(console.error.argsForCall[0][0]).toContain('className');
});

it('should create markup for boolean properties', function() {
expect(DOMPropertyOperations.createMarkupForProperty(
'checked',
Expand Down
23 changes: 23 additions & 0 deletions src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1087,5 +1087,28 @@ describe('ReactDOMComponent', function() {
'See Link > a > ... > Link > a.'
);
});

it('should warn about incorrect casing on properties', function() {
spyOn(console, 'error');
ReactDOMServer.renderToString(React.createElement('input', {type: 'text', tabindex: '1'}));
expect(console.error.argsForCall.length).toBe(1);
expect(console.error.argsForCall[0][0]).toContain('tabIndex');
});

it('should warn about incorrect casing on event handlers', function() {
spyOn(console, 'error');
ReactDOMServer.renderToString(React.createElement('input', {type: 'text', onclick: '1'}));
ReactDOMServer.renderToString(React.createElement('input', {type: 'text', onKeydown: '1'}));
expect(console.error.argsForCall.length).toBe(2);
expect(console.error.argsForCall[0][0]).toContain('onClick');
expect(console.error.argsForCall[1][0]).toContain('onKeyDown');
});

it('should warn about class', function() {
spyOn(console, 'error');
ReactDOMServer.renderToString(React.createElement('div', {class: 'muffins'}));
expect(console.error.argsForCall.length).toBe(1);
expect(console.error.argsForCall[0][0]).toContain('className');
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/**
* Copyright 2013-2015, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
* @providesModule ReactDOMUnknownPropertyDevtool
*/

'use strict';

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

var warning = require('warning');

if (__DEV__) {
var reactProps = {
children: true,
dangerouslySetInnerHTML: true,
key: true,
ref: true,
};
var warnedProperties = {};

var warnUnknownProperty = function(name) {
if (DOMProperty.properties.hasOwnProperty(name) || DOMProperty.isCustomAttribute(name)) {
return;
}
if (reactProps.hasOwnProperty(name) && reactProps[name] ||
warnedProperties.hasOwnProperty(name) && warnedProperties[name]) {
return;
}

warnedProperties[name] = true;
var lowerCasedName = name.toLowerCase();

// data-* attributes should be lowercase; suggest the lowercase version
var standardName = (
DOMProperty.isCustomAttribute(lowerCasedName) ?
lowerCasedName :
DOMProperty.getPossibleStandardName.hasOwnProperty(lowerCasedName) ?
DOMProperty.getPossibleStandardName[lowerCasedName] :
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?',
name,
standardName
);

var registrationName = (
EventPluginRegistry.possibleRegistrationNames.hasOwnProperty(
lowerCasedName
) ?
EventPluginRegistry.possibleRegistrationNames[lowerCasedName] :
null
);

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

var ReactDOMUnknownPropertyDevtool = {
onCreateMarkupForProperty(name, value) {
warnUnknownProperty(name);
},
onSetValueForProperty(node, name, value) {
warnUnknownProperty(name);
},
onDeleteValueForProperty(node, name) {
warnUnknownProperty(name);
},
}

module.exports = ReactDOMUnknownPropertyDevtool;

0 comments on commit 82fe64a

Please sign in to comment.