trailsjs / trailpack-router

:package: Router. Aggregate and Configure Application Routes.
MIT License
12 stars 7 forks source link

Route Handler Allow JS Objects #51

Closed smythey21 closed 8 years ago

smythey21 commented 8 years ago

Using the inert addon for hapi, it is possible to define a directory handler to serve static assets like this:

module.exports = [
  {
    method: 'GET',
    path: '/{param*}',
    handler: {
        directory: {
            path: 'public'
        }
    }
  }
]

But doing this returns an error:

error:
 ValidationError: child "handler" fails because ["handler" must be a Function, "handler" must be a string]

I believe adding joi.object() here should fix the issue. Thoughts? I'm happy to send a PR.

jaumard commented 8 years ago

Did you try to edit the file locally directly under your node_modules folder to see if it's works ? I do like this to see if it's fix my problem ^^ But I think it will work to add object on schema :)

smythey21 commented 8 years ago

I did, turns out it's not quite that simple. Looks like the router is trying to parse the handler input here, but since I'm no longer passing in a string calling .split on a JS object fails:

error:
 TypeError: route.handler.split is not a function
    at Object.getRoutePrerequisites (/Users/smythey21/workspace/kwivr-trails/node_modules/trailpack-router/lib/util.js:55:41)

I suppose I could write some logic to not parse the input if its a JS object, but this is becoming a little bit more in depth than I had hoped...

jaumard commented 8 years ago

Hum I suppose in this case if you put a function it will crash too... And the schema allow string or func so look like a bug to me... I didn't work a lot on this module but handler should be string, func or object if hapi allow it. Any thoughts @weyj4 @tjwebb ?

weyj4 commented 8 years ago

@smythey21 Yeah, the router splits the string, which limits what you can do with hapi through config/routes. @tjwebb what do you think? This is an example where having a generic interface gets in the way of native functionality.

tjwebb commented 8 years ago

I think trailpack-router in this case is muddling the actual issue. This is a Developer Experience issue that we definitely want to solve. Luckily, I think the solution is straightforward.

Hapi natively supports strings and objects as values for handler. http://hapijs.com/api#route-configuration:

If set to a string, the value is parsed the same way a prerequisite server method string shortcut is processed. Alternatively, handler can be assigned an object with a single key using the name of a registered handler type and value with the options passed to the registered handler.

The validation fails because the trailpack-router validation is wrong, not because hapi doesn't support certain things. Currently the validator is expecting to see Controller.handler-formatted strings. When it doesn't, it fails. Instead, when the value is something other than a Controller.handler string, it should forward the value directly into hapi.

weyj4 commented 8 years ago

Good idea. I can look into implementing this, that should solve our problem.