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

Synchronous plugin registration #2690

Closed
kanongil opened this issue Aug 11, 2015 · 7 comments
Closed

Synchronous plugin registration #2690

kanongil opened this issue Aug 11, 2015 · 7 comments
Assignees
Labels
non issue Issue is not a problem or requires changes

Comments

@kanongil
Copy link
Contributor

Hapi 9.x will require eg. inert to be registered before its file and directory handlers can be used.

This is fine but the new process seems a bit off, requiring all such routes to be embedded in the registration callback. Alternatively, the user would need to know that the registration is actually synchronous, and that the callback can be ignored and an empty function should be provided.

Ideally, it should just be something like:

var Hapi = require('hapi');
var Inert = require('inert');

var server = new Server();
server.connection();

server.register(Inert);

server.route();

instead of:

server.register(Inert, function(err) {

    if (err) {
        throw err;
    }

    server.route();
});

or:

server.register(Inert, function() {});

server.route();
@hueniverse
Copy link
Contributor

This is already the case for any auth scheme or anything other than the three built-in plugins. I don't see this as a real problem. It might be annoying at first but eventually people will just get use to it (or better yet, glue will add built-in support directly).

@hueniverse hueniverse added the non issue Issue is not a problem or requires changes label Aug 11, 2015
@hueniverse hueniverse self-assigned this Aug 11, 2015
@kanongil
Copy link
Contributor Author

Why not remove the callback requirement from server.register and catch errors from such in the server.start callback? Plugin dependency after errors are already exposed there.

@hueniverse
Copy link
Contributor

Because this is not the right way to solve the issue. hapi is designed to work with plugins and you are optimizing for the most basic use case.

@kanongil
Copy link
Contributor Author

To me, designed to work with plugins, reads as: should be simple to use plugins, which my proposal aims at.

The most basic use case is the one new users are likely to be confronted with. Requiring nesting, or empty function arguments, is somewhat off-putting.

Anyway, I don't intend to fight on this, and the current implementation is fine for my private usage.

@hueniverse
Copy link
Contributor

The only way this doesn't lead to future pain because people are just ignoring the callback is if we add a plugin attribute called sync which when true allows you to skip the callback in register. But this would require publishing all the plugins again. Would that work?

@kanongil
Copy link
Contributor Author

It seems that you miss my point. My suggestion is that errors that should have triggered in a missing register callback are instead triggered in the server.start callback. As far as I can tell, this can be done without any breaking changes, and most of the tooling to implement this is already in place.

@hueniverse
Copy link
Contributor

This is weird. You would call register and if there are errors, you would get them returned when you call start() or initialize()? This will be a nightmare to debug when things go wrong and your tests are not starting the server.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
non issue Issue is not a problem or requires changes
Projects
None yet
Development

No branches or pull requests

2 participants