Skip to content

Commit

Permalink
Warn on missing event handler properties
Browse files Browse the repository at this point in the history
Fixes #3548. Warns on properties that are case-insensitive matches for
registered event names (e.g. "onclick" instead of "onClick").
  • Loading branch information
ali committed Nov 2, 2015
1 parent 88bae3f commit 9275f87
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 1 deletion.
13 changes: 13 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,18 @@ if (__DEV__) {
standardName
);

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

warning(
registrationName == null,
'%s seems to be an invalid event name. 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.getPossibleRegistrationName[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}
*/
getPossibleRegistrationName: __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 getPossibleRegistrationName = EventPluginRegistry
.getPossibleRegistrationName;
for (var lowerCasedName in getPossibleRegistrationName) {
if (getPossibleRegistrationName.hasOwnProperty(lowerCasedName)) {
delete getPossibleRegistrationName[lowerCasedName];
}
}
}
},

};
Expand Down

0 comments on commit 9275f87

Please sign in to comment.