-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow plugins to register handler types #1521
Conversation
…r via server pack.
Conflicts: test/server.js
@@ -7,7 +7,6 @@ var Utils = require('./utils'); | |||
var Response = require('./response'); | |||
var Ext = require('./ext'); | |||
var File = require('./file'); | |||
var Directory = require('./directory'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leave this here along with the server handler registration. Instead of the server constructor having the logic of adding the built-ins, create a function here and have the server call it. Keeps the inter dependencies cleaner.
Please use the excellent work in #1511 for the missing documentation (the relevant parts, modified to this style). |
@hueniverse updated |
I didn't mean to create a new file like methods.js which made sense because it had a lot of other logic. Just mean that the handlers definitions should be owned by the pack. The docs still show passing the schema as a third argument to Also, cannot override the built-ins. |
OK - will pull it back into Just so we're clear, this means that any new handlers will use The initial issue called for the ability to override the built-ins. Is it safe to assume that no handlers can be overridden once they are defined? |
Yes, no overrides. First win. New handlers will need to verify the config using whatever they want, but the route handler config must still be an object with a single key that is Joi.object(). |
@hueniverse one more time |
var server = Hapi.createServer('localhost', 8000); | ||
|
||
// Defines new handler for routes on this server | ||
server.handler('proxy', function (route, options) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bad example since overriding existing is not allowed.
@@ -400,3 +397,15 @@ exports.invoke = function (request, event, callback) { | |||
}); | |||
}; | |||
|
|||
|
|||
exports.registerDefaultHandlers = function (server) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shorter name. Like: register()
Closed? |
The way my code is currently set up, moving the schema validation into the handler generator is going to take some restructuring. If you look at my tests, I was expecting the user to pass in the handler generator. By making this change, non-builtins need to be wrapped at runtime. Also, by eliminating the third |
Ok. I'll give it a pass tonight so you can see what I mean. |
@hueniverse is this what you meant? |
Allow plugins to register handler types
Defines
server.handler(name, method [, schema])
andplugin.handler(name, method [, schema])
Closes #1385