Skip to content

Commit

Permalink
Faster Listener Deletion
Browse files Browse the repository at this point in the history
Whenever a component is unmounted, we delete all listeners that might have been attached. This sucks because most applications, Facebook included, do not use every listener. There's a lot of wasted computation, especially if many components are mounted and unmounted.

This changes `deleteAllListeners` to more delete listeners more efficiently.
  • Loading branch information
yungsters authored and zpao committed Jul 3, 2013
1 parent c692d9e commit 510146e
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 14 deletions.
17 changes: 14 additions & 3 deletions src/event/CallbackRegistry.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ var CallbackRegistry = {
/**
* Stores `listener` at `listenerBank[registrationName][id]`. Is idempotent.
*
* @param {string} id ID of the DOM node.
* @param {string} id ID of the DOM element.
* @param {string} registrationName Name of listener (e.g. `onClick`).
* @param {?function} listener The callback to store.
*/
Expand All @@ -46,7 +46,7 @@ var CallbackRegistry = {
},

/**
* @param {string} id ID of the DOM node.
* @param {string} id ID of the DOM element.
* @param {string} registrationName Name of listener (e.g. `onClick`).
* @return {?function} The stored callback.
*/
Expand All @@ -58,7 +58,7 @@ var CallbackRegistry = {
/**
* Deletes a listener from the registration bank.
*
* @param {string} id ID of the DOM node.
* @param {string} id ID of the DOM element.
* @param {string} registrationName Name of listener (e.g. `onClick`).
*/
deleteListener: function(id, registrationName) {
Expand All @@ -68,6 +68,17 @@ var CallbackRegistry = {
}
},

/**
* Deletes all listeners for the DOM element with the supplied ID.
*
* @param {string} id ID of the DOM element.
*/
deleteAllListeners: function(id) {
for (var registrationName in listenerBank) {
delete listenerBank[registrationName][id];
}
},

/**
* This is needed for tests only. Do not use!
*/
Expand Down
12 changes: 1 addition & 11 deletions src/event/EventPluginHub.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,17 +112,7 @@ var EventPluginHub = {

deleteListener: CallbackRegistry.deleteListener,

/**
* Deletes all listeners for the DOM element with the supplied ID.
*
* @param {string} domID ID of a DOM element.
*/
deleteAllListeners: function(domID) {
var registrationNamesKeys = EventPluginRegistry.registrationNamesKeys;
for (var ii = 0; ii < registrationNamesKeys.length; ii++) {
CallbackRegistry.deleteListener(domID, registrationNamesKeys[ii]);
}
},
deleteAllListeners: CallbackRegistry.deleteAllListeners,

/**
* Allows registered plugins an opportunity to extract events from top-level
Expand Down

0 comments on commit 510146e

Please sign in to comment.