Skip to content
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

Set per-plugin registration options when registering an array of plugins #1850

Closed
martinheidegger opened this issue Aug 11, 2014 · 6 comments
Closed
Assignees
Labels
Milestone

Comments

@martinheidegger
Copy link

@martinheidegger martinheidegger commented Aug 11, 2014

When I want to set the options and a path prefix for a plugin it looks like this:

server.pack.register({
    plugin: require("my-plugin"),
    options: {
       foo: "bar"
    }
}, {
    route: {
        prefix: "/myprefix"
    }
}, function () {
    ... 
});

It looks like the object containing the route definition seems unnecessary clutter. Would it be possible to offer setting the route in the plugin definition? Somehow like:

server.pack.register({
    plugin: require("my-plugin"),
    options: {
       foo: "bar"
    },
    route: {
        prefix: "/myprefix"
    }
}, function () {
    ... 
});
@hueniverse
Copy link
Contributor

@hueniverse hueniverse commented Aug 11, 2014

I like to keep them separate because they are different options. One for the plugin and one for registration. I worry people will get confused about where to put the registration options (e.g. inside the plugin options which we cannot validate).

@martinheidegger
Copy link
Author

@martinheidegger martinheidegger commented Aug 11, 2014

It is a nuisance when registering one plugin but this makes it hard to register multiple plugins without having all of them prefixed.

For example:

server.pack.register([require("plugin-a"), require("plugin-b")], function () { ... });

Right now if I want to change the path for plugin-a I would have to write

var async = require("async");
async.serial([function (callback) {
    server.pack.register(require("plugin-a"), {
       route: {
            prefix: "/myprefix"
       }
    }, callback);
}, function (callback) {
   server.pack.register(require("plugin-b"), callback);
}], function () { ... });

compared to the proposal

server.pack.register([{
   plugin: require("plugin-a"),
   route: {
       prefix: "/myprefix"
   }
}, require("plugin-b")], function () { ... });

@mwawrusch
Copy link

@mwawrusch mwawrusch commented Aug 11, 2014

@martin +1 for the nuisance factor.

On Mon, Aug 11, 2014 at 10:44 AM, Martin Heidegger <notifications@github.com

wrote:

It is a nuisance when registering one plugin but this makes it hard to
register multiple plugins without having all of them prefixed.

For example:

server.pack.register([require("plugin-a"), require("plugin-b")], function () { ... });

Right now if I want to change the path for plugin-a I would have to write

var async = require("async");async.serial([function (callback) {
server.pack.register(require("plugin-a"), {
route: {
prefix: "/myprefix"
}
}, callback);}, function (callback) {
server.pack.register(require("plugin-b"), callback);}], function () { ... });

compared to the proposal

server.pack.register([{
plugin: require("plugin-a"),
route: {
prefix: "/myprefix"
}}, require("plugin-b")], function () { ... });


Reply to this email directly or view it on GitHub
#1850 (comment).

Martin Wawrusch
p: +1 310 404 1698

modeista.com - Connect With Style
fanignite.com - ALPHA - Create Your Mobile App In 30 Seconds
wawrusch.com - Personal Blog
codedoctor.co - Professional Blog

Follow me on twitter at @MartinWawrusch http://twitter.com/MartinWawrusch

@hueniverse hueniverse self-assigned this Nov 5, 2014
@hueniverse
Copy link
Contributor

@hueniverse hueniverse commented Nov 5, 2014

After much consideration, I much rather this kind of API innovation happen elsewhere first. It should be pretty easy to create utilities that will provide this, and we already have some of this today with the composer() API. We have a new module called glue just for that, and I think this work belongs there. You can easily add helpers there that will give you this functionality.

If later, we see that most people use glue helpers over direct access to register(), we can revisit.

@hueniverse hueniverse closed this Nov 5, 2014
@hueniverse hueniverse added this to the 10.2.0 milestone Oct 2, 2015
@hueniverse hueniverse reopened this Oct 2, 2015
@hueniverse hueniverse changed the title Offer a way to define registerOptions in the options of a plugin. Set per-plugin registration options when registering an array of plugins Oct 2, 2015
@hueniverse
Copy link
Contributor

@hueniverse hueniverse commented Oct 2, 2015

Also requested in #2483 #2478 #2669

@hueniverse hueniverse closed this in 900c913 Oct 2, 2015
@Marsup Marsup added feature and removed request labels Sep 20, 2019
@lock
Copy link

@lock lock bot commented Jan 9, 2020

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.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants