Permalink
Browse files

fix(js): client-side hooks can now handle periods in hook names

Previously, using a hook name with a period would fail, and using a hook type
with a period would create an unexpected data structure within `elgg.config.hooks`.

This allows `elgg.provides()` to handle arbitrary names by giving them as an
array, and uses this new API to handle hook names/types consistently.

Fixes #9160
  • Loading branch information...
mrclay committed Dec 6, 2015
1 parent 2a0b909 commit 9f70099f5ecec3deacd666195cb7751eef77ce17
Showing with 50 additions and 15 deletions.
  1. +21 −4 js/lib/elgglib.js
  2. +11 −11 js/lib/hooks.js
  3. +12 −0 js/tests/ElggHooksTest.js
  4. +6 −0 js/tests/ElggLibTest.js
View
@@ -192,17 +192,34 @@ elgg.require = function(pkg) {
* elgg.package.subpackage = elgg.package.subpackage || {};
* </pre>
*
+ * An array package name can be given if any subpackage names need to contain a period.
+ *
+ * <pre>
+ * elgg.provide(['one', 'two.three']);
+ * </pre>
+ *
+ * is equivalent to
+ *
+ * one = one || {};
+ * one['two.three'] = one['two.three'] || {};
+ *
* @example elgg.provide('elgg.config.translations')
*
- * @param {string} pkg The package name.
+ * @param {String|Array} pkg The package name. Only use an array if a subpackage name needs to contain a period.
+ *
+ * @param {Object} opt_context The object to extend (defaults to this)
*/
elgg.provide = function(pkg, opt_context) {
- elgg.assertTypeOf('string', pkg);
-
- var parts = pkg.split('.'),
+ var parts,
context = opt_context || elgg.global,
part, i;
+ if (elgg.isArray(pkg)) {
+ parts = pkg;
+ } else {
+ elgg.assertTypeOf('string', pkg);
+ parts = pkg.split('.');
+ }
for (i = 0; i < parts.length; i += 1) {
part = parts[i];
View
@@ -19,7 +19,7 @@ elgg.provide('elgg.config.triggered_hooks');
* @param {String} type Type of the event to register for
* @param {Function} handler Handle to call
* @param {Number} priority Priority to call the event handler
- * @return {Bool}
+ * @return {Boolean}
*/
elgg.register_hook_handler = function(name, type, handler, priority) {
elgg.assertTypeOf('string', name);
@@ -30,20 +30,20 @@ elgg.register_hook_handler = function(name, type, handler, priority) {
return false;
}
- var priorities = elgg.config.hooks;
+ var hooks = elgg.config.hooks;
- elgg.provide(name + '.' + type, priorities);
+ elgg.provide([name, type], hooks);
- if (!(priorities[name][type] instanceof elgg.ElggPriorityList)) {
- priorities[name][type] = new elgg.ElggPriorityList();
+ if (!(hooks[name][type] instanceof elgg.ElggPriorityList)) {
+ hooks[name][type] = new elgg.ElggPriorityList();
}
// call if instant and already triggered.
if (elgg.is_instant_hook(name, type) && elgg.is_triggered_hook(name, type)) {
handler(name, type, null, null);
}
- return priorities[name][type].insert(handler, priority);
+ return hooks[name][type].insert(handler, priority);
};
/**
@@ -68,7 +68,7 @@ elgg.register_hook_handler = function(name, type, handler, priority) {
* @param {Object} params Optional parameters to pass to the handlers
* @param {Object} value Initial value of the return. Can be mangled by handlers
*
- * @return {Bool}
+ * @return {Boolean}
*/
elgg.trigger_hook = function(name, type, params, value) {
elgg.assertTypeOf('string', name);
@@ -90,10 +90,10 @@ elgg.trigger_hook = function(name, type, params, value) {
}
};
- elgg.provide(name + '.' + type, hooks);
- elgg.provide('all.' + type, hooks);
- elgg.provide(name + '.all', hooks);
- elgg.provide('all.all', hooks);
+ elgg.provide([name, type], hooks);
+ elgg.provide(['all', type], hooks);
+ elgg.provide([name, 'all'], hooks);
+ elgg.provide(['all', 'all'], hooks);
var hooksList = [];
View
@@ -40,6 +40,18 @@ define(function(require) {
elgg.register_hook_handler('all', 'all', elgg.abstractMethod);
expect(function() { elgg.trigger_hook('pinky', 'winky'); }).toThrow();
});
+
+ it("handles names/types with periods", function() {
+ expect(elgg.trigger_hook("fee.fum", "bar.bang")).toBe(null);
+
+ elgg.register_hook_handler("fee.fum", "bar.bang", function () { return 1; });
+
+ expect(elgg.trigger_hook("fee.fum", "bar.bang")).toBe(1);
+
+ elgg.register_hook_handler("fee.fum", "all", function () { return 2; });
+
+ expect(elgg.trigger_hook("fee.fum", "pow")).toBe(2);
+ });
});
describe("elgg.register_hook_handler()", function() {
View
@@ -71,6 +71,12 @@ define(function(require) {
window.foo = undefined; // cleanup
});
+
+ it("can handle array names with periods", function () {
+ elgg.provide(['foo', 'bar.baz']);
+
+ expect(window.foo['bar.baz']).not.toBe(undefined);
+ });
});
describe("elgg.require()", function() {

0 comments on commit 9f70099

Please sign in to comment.