Skip to content
This repository was archived by the owner on Apr 8, 2019. It is now read-only.

Discrepancies in registering plugins #218

Closed
garfbargle opened this issue May 23, 2016 · 7 comments
Closed

Discrepancies in registering plugins #218

garfbargle opened this issue May 23, 2016 · 7 comments

Comments

@garfbargle
Copy link

garfbargle commented May 23, 2016

In /lib/start.js, our manifest sets us up and we register Inert and Vision.

In /lib/home.js, we set server dependencies for Inert and Vision. We wait for them before setting up the home plugin.

However, in /lib/lout.js, we re-register Inert and Vision along with the 'Lout' plugin.

Why does /lib/home.js not register the plugins instead? Why is there a difference between //lib/home.js and /lib/lout.js?

@zoe-1
Copy link
Contributor

zoe-1 commented May 23, 2016

@garfbargle good observation.
Will look into it and write a response.

zoe-1 added a commit that referenced this issue May 24, 2016
* lout now registers using dependencies.after logic
  similar to other project plugins.
* previously lout registered vision and inert inside
  of using dependency.after logic.
@zoe-1
Copy link
Contributor

zoe-1 commented May 24, 2016

@garfbargle
cleaned up /lib/lout.js to follow similar logic as /lib/home.js and other plugins. The project uses Glue to register plugins so registering inert
and vision in the lout plugin or other plugin is not good style.
Instead plugins that have dependencies like inert or vision use dependency.after logic to ensure dependencies are loaded first. Thanks for pointing out the issue.

@garfbargle
Copy link
Author

@zoe-1 Awesome, that's what I was thinking should be the right approach after studying the project. Thanks for clearing that up!

@zoe-1 zoe-1 closed this as completed May 25, 2016
@ferrao
Copy link

ferrao commented Oct 17, 2016

@zoe-1 I belive you did something wrong with this:

server.dependency(['inert', 'vision'], internals.after(server, next));

as you seem to be calling internals.after() immediately. I belive it should be:

server.dependency(['inert', 'vision'], internals.after);
return next();

But then i can not escape this:

Error: Cannot add onPreStart (after) extension after the server was initialized

@zoe-1
Copy link
Contributor

zoe-1 commented Oct 28, 2016

@ferrao I have not seen your code, but most likely you are describing an asynchronous issue.
In short, internals.after() is not executed immediately.
server.dependency(['dependencies'], after) first registers dependencies, then the after function is executed "after" dependencies are registered.

Plugins are loaded at the initialization stage of a hapi server start up. The next() execution tells hapi that the plugin registration completed causing hapi to move on to other registrations. But, in your code, next() is called before the completion of the internals.after() registration logic and you get the error message. This happens because of JavaScripts' asynchronous execution of code. JavaScript executes return next() before your server.dependency() completes.

See ./lib/lout.js in master branch of university repo for example.

server.dependency(['inert', 'vision'], internals.after(server, options, next));
return next();  // incorrect use of next();

If we did the above in ./lib/lout.js, the registration of ./lib/lout.js would exit prematurely (before the server.dependency() completes). However, if we drop return next() as below, next() would execute at the completion of registering the lout plugin. Notice below: next() is passed to the internals.after() to be executed later.
Correct example below.

server.dependency(['inert', 'vision'], internals.after(server, options, next));

Usually, the error message is:

Error: Cannot start server while it is in initializing state

Relevant Documentation:
plugin tutorial
dependencies after docs

@ferrao
Copy link

ferrao commented Oct 28, 2016

@zoe-1 both ./lib/lout.js and your example

server.dependency(['inert', 'vision'], internals.after(server, options, next));

are not correct. The after argument needs to be a function and not a function invocation.

Look at the docs example:

const after = function (server, next) {
    // Additional plugin registration logic
    return next();
};

exports.register = function (server, options, next) {
    server.dependency('yar', after);
    return next();
};

It is quite clear for me that the code is wrong as it is, but i am lost as to what is the proper way to set this up using server.dependecy(). I now that i can avoid it by declaring decencies in attributes, but sometimes we need to do additional stuff after dependencies are registered.

zoe-1 added a commit to zoe-1/university-dev that referenced this issue Oct 31, 2016
zoe-1 added a commit to zoe-1/university-dev that referenced this issue Oct 31, 2016
zoe-1 added a commit that referenced this issue Oct 31, 2016
zoe-1 added a commit that referenced this issue Oct 31, 2016
fix lout loading issue #394 #218.
@zoe-1
Copy link
Contributor

zoe-1 commented Oct 31, 2016

You are right.
made corrections with #235 and #234.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants