Declarative dependencies of a route

arty-name commented 3 years ago

Support plan

is this issue currently blocking your project? (yes/no): no
is this issue affecting a production system? (yes/no): no

Context

node version: 16
module version: 20.1.5
environment (e.g. node, browser, native): node
used with (e.g. hapi application, another framework, standalone, ...): hapi application
any other relevant information:

What problem are you trying to solve?

One of the selling points on the hapi homepage is pretty exciting:

Plugin dependencies — plugins can safely rely on other plugins, including the order in which they must be execute, no matter the order in which you register them.

Explicit dependencies are great! If you try to use a plugin without providing its dependencies, the server won’t even start. So I tried to declare that certain routes in my application depend on certain plugins for static files or logging.

I could not find a way to declare dependencies for routes though, and @devinivy on Slack has confirmed that it is not possible at the moment.

The benefits of adding this feature for routes would be the same as when it was added for plugins. Since plugins decorate the standard Request object, it is not obvious where they are used, so during a refactoring it currently not clear whether a certain plugin is still needed. Explicit dependencies will help with that, as well as with reusing routes from other projects.

Do you have a new or modified API suggestion to solve the problem?

I think the declaration could be the same as for the plugins:

export const route = {
  method: 'GET',
  path: '/',
  handler,
  dependencies: ['inert', 'hapi-pino']
};

kanongil commented 3 years ago

Even if we wanted to do this, it is not possible with the current architecture.

Routes are procedurally defined when a plug-in is loaded. By then it is too late to register new plugins that a route depends on.

arty-name commented 3 years ago

I am not really asking to load plugins when adding a route. Just checking that the required plugins are registered would be very nice already. Is that possible with the current architecture?

kanongil commented 3 years ago

Ah, yes – I forgot how it works.

I guess you can do this by calling server.dependency() before declaring your route.

devinivy commented 3 years ago

I'm curious what you think of this case. If you run this code to define a route that relies on inert, you'll get an error Unknown handler: directory:

const server = Hapi.server();

server.route({
    method: 'get',
    path: '/',
    handler: {
        directory: { path: './' }
    }
});

In this case declaring a dependency to be checked during server initialization (as plugin dependencies behave) isn't going to help us very much: we'd need to defer that Unknown handler error until server init for the dependency to be useful here. I suppose plugin dependencies have analogous behavior, though, e.g. if you try to use a dependee's server decoration during plugin registration, plugin dependencies can't help you there.

Also want to point out that it's easy to satisfy dependencies on plugins like inert because you can register inert multiple times without issue thanks to its use of once: https://github.com/hapijs/inert/blob/0dc3a7e4d5f7d508dea79bf9e7fe9f4ff2c2092d/lib/index.js#L32-L34. So if you need it then you can often satisfy that dependency yourself by registering it.

Finally, I think you can get plugins to express dependencies on behalf of their routes with a little code using the 'route' event: https://runkit.com/60c18db503676e001ce17d66/612fb828a39a910007c885dd

arty-name commented 3 years ago

I guess you can do this by calling server.dependency() before declaring your route.

@kanongil, I don’t think this would really work. I want the dependency declaration to be colocated at least with the handler. The file containing the handler and potentially the path and other route options does not have access to the server instance.

// routes/user.js
// server instance not in scope
export const userRoute = {
  method: 'GET',
  path: '/user',
  dependencies: ['fancy-logger'], // must be really close to the handler calling fancyLogger()
  handler: (request) => request.fancyLogger('PASS')
}

// server.js
import userRoute from './routes/user.js'
const server = Hapi.server();
server.route(userRoute); 
await server.initialize(); // throws, because 'fancy-logger' has not been registered

@devinivy, looks like I should not have chosen inert as an example. I will most likely use many other plugins, and some of them will be unhappy about being registered multiple times. Also I must frankly say I could not really follow most of your comment. The only thing that stood out: I do not mean routes created by plugins. I mean basic routes, as in the example above.

Being very new to hapi, I might be doing things in a very wrong way, so if I really should not be organizing my code in this way, do tell me.

kanongil commented 3 years ago

You can enable what you want in your own project, by creating your own helper that extracts the dependencies from your route definition and calls server.dependency() with it before registering the route. Or maybe export it as something like this for simpler handling?

export const user = {
  route: {
    method: 'GET',
    path: '/user',
    handler: (request) => request.fancyLogger('PASS')
  },
  dependencies: ['fancy-logger'], // must be really close to the handler calling fancyLogger()
}

This should work as long as you only register your routes before the server is initialized.

I don't think it makes sense to include the logic in hapi itself, as this seems quite specific to how you have organised your project.

arty-name commented 3 years ago

This is fair! Thank you for the discussion! Feel free to close this issue as you see fit.

I started asking about it only because declaring route dependencies seemed really obvious to me, and only created the issue at the @devinivy’s suggestion.

Having read a bit more on the topic, I think the same could be also achieved by wrapping every route with dependencies in a plugin, but that seems like too much boilerplate.

devinivy commented 3 years ago

I appreciate the discussion, I found it useful and others may find good information in here too— thanks for bringing it up!

hapijs / hapi