hook api #6

Closed
wants to merge 1 commit into
from

Projects

None yet

1 participant

@gjohnson

Hook API and necessary test changes. See inline comments for details / discussion.

@gjohnson gjohnson commented on the diff Mar 25, 2013
lib/server.js
@@ -64,6 +65,8 @@ Server.prototype.respondWithMethods = function(reply){
/**
* Handle `msg`.
*
+ * TODO: refactor internal validation to use hooks.
@gjohnson
gjohnson Mar 25, 2013

Will do this later once your cool with all the changes. Probably just add a this.before(this.validate) to the constructor or something.

@gjohnson gjohnson commented on the diff Mar 25, 2013
lib/server.js
args.push(function(err){
if (err) return reply({ error: err.message });
var args = [].slice.call(arguments, 1);
reply({ args: args });
+ self.performHook('after', msg);
@gjohnson
gjohnson Mar 25, 2013

Would nextTick be required to ENSURE reply() flushes? Wouldn't really want the after hooks to disrupt reply()

@gjohnson gjohnson commented on the diff Mar 25, 2013
lib/server.js
+Server.prototype.after = function(name, fn){
+ if ('function' == typeof name) fn = name, name = undefined;
+ this.hook('after', name, fn);
+ return this;
+};
+
+/**
+ * Performs all the `hooks` registered for `type`.
+ *
+ * @api private
+ * @param {String} type
+ * @param {Object} msg
+ * @param {Function} fn
+ */
+
+Server.prototype.performHook = function(type, msg, done){
@gjohnson
gjohnson Mar 25, 2013

Yanked this from builder.js! Thanks :-)

@gjohnson gjohnson commented on the diff Mar 25, 2013
lib/server.js
@@ -114,6 +123,79 @@ Server.prototype.expose = function(name, fn){
};
/**
+ * Registers a hook `fn` for `type`, optionally filtered by `name`. If
+ * no `name` is given, it will match all method calls.
+ *
+ * @api private
+ * @param {String} type
+ * @param {String} name
+ * @param {Function} fn
+ */
+
+Server.prototype.hook = function(type, name, fn){
@gjohnson
gjohnson Mar 25, 2013

RegExp might be overkill, but seems liked the easiest way to catch the "wildcard" and "named" hooks...

@gjohnson
gjohnson commented Apr 3, 2013

See #5 for why.

@gjohnson

I don't need this anymore :-)

@gjohnson gjohnson closed this Aug 14, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment