Skip to content

Commit

Permalink
Warn on missing event handler properties
Browse files Browse the repository at this point in the history
Fixes facebook#3548. Warns on properties that are case-insensitive matches for
registered event names (e.g. "onclick" instead of "onClick").
  • Loading branch information
ali authored and arkist committed Nov 4, 2015
1 parent a4b5f2f commit 876a0fb
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 1 deletion.
15 changes: 15 additions & 0 deletions src/renderers/dom/shared/DOMPropertyOperations.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
'use strict';

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

var quoteAttributeValueForBrowser = require('quoteAttributeValueForBrowser');
Expand Down Expand Up @@ -87,6 +88,20 @@ if (__DEV__) {
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
);
};
}

Expand Down
17 changes: 16 additions & 1 deletion src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ describe('DOMPropertyOperations', function() {
)).toBe('id="simple"');
});

it('should warn about incorrect casing', function() {
it('should warn about incorrect casing on properties', function() {
spyOn(console, 'error');
expect(DOMPropertyOperations.createMarkupForProperty(
'tabindex',
Expand All @@ -60,6 +60,21 @@ describe('DOMPropertyOperations', function() {
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(
Expand Down
24 changes: 24 additions & 0 deletions src/renderers/shared/event/EventPluginRegistry.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,12 @@ function publishRegistrationName(registrationName, PluginModule, eventName) {
EventPluginRegistry.registrationNameModules[registrationName] = PluginModule;
EventPluginRegistry.registrationNameDependencies[registrationName] =
PluginModule.eventTypes[eventName].dependencies;

if (__DEV__) {
var lowerCasedName = registrationName.toLowerCase();
EventPluginRegistry.possibleRegistrationNames[lowerCasedName] =
registrationName;
}
}

/**
Expand Down Expand Up @@ -157,6 +163,14 @@ var EventPluginRegistry = {
*/
registrationNameDependencies: {},

/**
* Mapping from lowercase registration names to the properly cased version,
* used to warn in the case of missing event handlers. Available
* only in __DEV__.
* @type {Object}
*/
possibleRegistrationNames: __DEV__ ? {} : null,

/**
* Injects an ordering of plugins (by plugin name). This allows the ordering
* to be decoupled from injection of the actual plugins so that ordering is
Expand Down Expand Up @@ -265,6 +279,16 @@ var EventPluginRegistry = {
delete registrationNameModules[registrationName];
}
}

if (__DEV__) {
var possibleRegistrationNames =
EventPluginRegistry.possibleRegistrationNames;
for (var lowerCasedName in possibleRegistrationNames) {
if (possibleRegistrationNames.hasOwnProperty(lowerCasedName)) {
delete possibleRegistrationNames[lowerCasedName];
}
}
}
},

};
Expand Down

0 comments on commit 876a0fb

Please sign in to comment.