add plugin-specific registration options to schema

[dylancwood]

Background

Glue allows users to specify which plugins to be loaded via the registrations property. Glue enforces the schema for each registration provided.

Problem

Glue's registration schema does not allow for select, once, or routes to be added at a plugin-specific level. These options are supported and documented by Hapi.

Solution

Borrow Hapi's schema for the aforementioned options and add it to Glue's schema.

Discussion

As Hapi's options change, do we want to have to keep Glue's Joi schemas up to date? Perhaps we could remove validation from Glue altogether and let Hapi handle it?

[gergoerdosi]

Those options are already supported: https://github.com/hapijs/glue/blob/v3.2.0/test/index.js#L332-L347

[mtharrison]

Also see https://github.com/hapijs/glue/blob/master/API.md#usage. The third item in registrations show both option types supplied (plugin level options and register options).

[dylancwood]

Thanks @gergoerdosi and @mtharrison. Sorry for not reading the docs closely enough!

I am happy to close this request, but wonder what you think about supporting the current Hapi syntax, where select, once and routes can be specified as peers of the options and registration properties. IMO, this will make if more seamless for folks to convert their Hapi configs to Glue manifests.

Thanks again!

[mtharrison]

I know manifest format has been changed recently. It seems pretty seamless now to me. My code normally looks like this:

const plugin = { 
    register: require('plugin'),
    options: {
        option1: 'something',
        option2: 'otherThing'
    }
};

const options = {
    select: ['api'],
    routes: {
        prefix: '/prefix'
    }
};

// with normal hapi

server.register(plugin, options, (err) => {

   ...
});

// with glue

...
"registrations": [
    {
        plugin: plugin,
        options: options
    },
    ...
]
...

[mtharrison]

Oh, my apologies, I didn't notice that hapi now supports those extra options per-plugin. I'll bow out of this conversation and leave it to @csrl to decide whether to support it or not.

Personally I don't see the point because registrations are per-plugin in Glue anyway.

[dylancwood]

Thanks @mtharrison.

@csrl Here's an example hapi registration that illustrates the syntax I would like to support:

server.register([
  { register: 'my-plugin', options: {...}, select: ['api'], routes: { prefix: '/myplugin' } },
  { register: 'other-plugin', options: {...}, select: ['private'] }
]);

Here's how it would look in a Glue manifest:

{
  registrations: [
    {
      plugin: {
        register: 'my-plugin',
        options: {...},
        select: ['api'],
        routes: { prefix: '/myplugin' }
      }
    },
    {
      plugin: {
        register: 'other-plugin',
        select: ['private']
      }
    }
  ]
}

[gergoerdosi]

@dylancwood: Please see the test case I linked above. It's possible to specify select, routes, and once for each plugin. In short:

{
  registrations: [
    {
      plugin: {
        register: 'my-plugin',
        options: {...},
      },
      options: {
        select: ['api'],
        routes: { prefix: '/myplugin' }
      }
    }
  ]
}

[dylancwood]

@gergoerdosi: Thanks for the link and the example.

When I first submitted this request, I did not understand that Glue supported plugin-specific select, routes and once options. Thanks to you, I now know that it does :+1:

However, I think it would be nice if the Glue manifest matched the Hapi syntax as closely as possible: Hapi allows the aforementioned options to be specified as peers of the register and options plugin properties. I suspect that the current Glue syntax for plugin-specific select, etc... options was implemented before Hapi supported its current syntax for the same functionality. This is supported by the fact that Glue calls server.register() separately for each registration, rather than passing an array of plugins to a single call to server.register().

Let me know if I am still not being clear.

[csrl]

@dylancwood Thanks for the pull request.

If I understand the hapi documentation of server.register, the point of adding those 3 options to the plugin object is meaningful only to the array form of the plugins parameter. Since glue does not support the array form, there is no additional value in supporting those 3 new options.

I think an update to the glue documentation to clearly call out that they are not supported would be the preferred commit to merge. Or am I missing something to why you still want this supported?

[dylancwood]

@csrl : Thanks for your response. I think that we are on the same page for the most part.

I'll make one more attempt to convince you all, but I am also fine with just updating the documentation as @csrl suggests.

The Glue config API allows us to specify plugins in array form. Though Glue does not use hapi's array form for plugin registration, the Glue config format for plugins is very similar to Hapi's plugin registration API.

If you don't agree that Glue should implement the Hapi API as closely as possible, then I'll happily update the docs :-)

[csrl]

@dylancwood i see the difference now. Glue actually does not have an array of plugins. Glue has an array of registrations (since 3.0). I actually find it odd that hapi's registration function supports an array of plugins. Rather I think hapi should leave that type of abstraction to helper code, such as glue.

Of course, now that hapi has added the ability to pass registration options inside the plugin object, glue could be updated to just have a single registration call and an array of plugins to pass to it.

But like I said, I'm of the opinion that hapi should not support that registration form in the first place. However, I am in agreement to you that glue should follow the hapi API as closely as possible - that is why I created the glue 3.0 manifest format.

@hueniverse I think it'd be best to drop the multi-plugin array format for server.register in a future version of hapi. I would suggest that the introduction of the registration options to the plugin object in server.register is like a snake eating its own tail. Probably best to avoid.

[lock[bot]]

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.