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

Convert to a plugin #43

Closed
hueniverse opened this issue Mar 18, 2015 · 23 comments · Fixed by #44
Closed

Convert to a plugin #43

hueniverse opened this issue Mar 18, 2015 · 23 comments · Fixed by #44
Milestone

Comments

@hueniverse
Copy link

The right way to work with hapi is to build your application using plugins. Plugins provide a way to organize your code into logical components and then put them together in different combinations or deploy them in different configurations. While some plugins are published as general purpose utilities (e.g. adding authentication), you should think of plugins as a way to break your code into pieces.

Now that we have a basic server with one endpoint, we want to move the creation of the endpoint to a plugin, and then register that plugin with the server. Creating a new file lib/version.js and move the /version endpoint there, wrapped in the plugin register() function.

Then change our current lib/index.js to require the new file and register the version plugin with our server.

Remember to follow the style guide, and ask any questions right here in the comments. If you are not sure how to get your fork back in sync with the current updated code, use the git guide.

Assignment due: 3/24

@hueniverse hueniverse added this to the 0.0.2 milestone Mar 18, 2015
@AdriVanHoudt
Copy link
Contributor

Nice git guide!

MylesBorins pushed a commit to MylesBorins/hueniversity that referenced this issue Mar 18, 2015
@AdriVanHoudt
Copy link
Contributor

How far do we need to go with the plugin? e.g. add its own package.json and tests?

MylesBorins pushed a commit to MylesBorins/hueniversity that referenced this issue Mar 18, 2015
MylesBorins pushed a commit to MylesBorins/hueniversity that referenced this issue Mar 18, 2015
MylesBorins pushed a commit to MylesBorins/hueniversity that referenced this issue Mar 18, 2015
MylesBorins pushed a commit to MylesBorins/hueniversity that referenced this issue Mar 18, 2015
MylesBorins pushed a commit to MylesBorins/hueniversity that referenced this issue Mar 18, 2015
MylesBorins pushed a commit to MylesBorins/hueniversity that referenced this issue Mar 18, 2015
@MylesBorins
Copy link
Contributor

I was under the impression that since we were asked to make lib/version.js that would be the extent of what we are to do for a plugin. @hueniverse had previously mentioned testing would be coming in a future assignment (although I do long for tests!).

I was also noticing the amount of noise that my pr has created referencing this issue... is this a problem?

@AdriVanHoudt
Copy link
Contributor

yeah I guess and I noticed the noise to. How come? Looks like the same commit every time?

@MylesBorins
Copy link
Contributor

OHHHHHH I think it is from my rebasing

@AdriVanHoudt
Copy link
Contributor

aaah probably yes

@MylesBorins
Copy link
Contributor

Btw

plus = commit -a --amend --no-edit

FTW!!!

@AdriVanHoudt
Copy link
Contributor

for adding to an existing commit?

@MylesBorins
Copy link
Contributor

mmhmmm. That alias will take all staged chages and squash them down into the last commit.

@hueniverse
Copy link
Author

No need for a standalone plugin module, just the one file.

MylesBorins pushed a commit to MylesBorins/hueniversity that referenced this issue Mar 18, 2015
@chyld chyld mentioned this issue Mar 18, 2015
@AdriVanHoudt
Copy link
Contributor

👍

@ghost
Copy link

ghost commented Mar 19, 2015

I have now this.

index.js :


// Load modules

var Hapi = require('hapi');
var Hoek = require('hoek');
var Package = require('../package.json');


// Declare internals

var internals = {};


internals.init = function () {

  var server = new Hapi.Server();
  server.connection({ port: 8000 });

  server.register({register: require('version')}, function (err) {

    if (err) {
        console.error('Failed to load plugin:', err);
    }
   });

  server.start(function (err) {

  Hoek.assert(!err, err);
  console.log('Server started at: ' + server.info.uri);
  });
};

internals.init();

version.js


exports.register = function (server, options, next) {

  server.route({

    method: 'GET',
    path: '/version',
        config: {
          description: 'Returns the version of the server',
          handler: function (request, reply) {

                return reply({ version: Package.version });
            }
        }
    });

  next();
};

exports.register.attributes = {
  name: "version",
  version: "0.0.1"
};

but still I see this error message :

Error: Cannot find module 'version'

Roelof

@Couto
Copy link

Couto commented Mar 19, 2015

@roelof1967 you should do require('./version.js') when registering the plugin, as far as I know, if you don't refer to a file name, node just tries to require the module from the node_modules folder.

@ghost
Copy link

ghost commented Mar 19, 2015

Thanks, I had to change require (version) to require(./version) and the error message disappear.

@Couto
Copy link

Couto commented Mar 19, 2015

Quick questions:

  1. Should we bump the patch in the version on package.json? Meaning that this assignment will generate the version 0.0.2?
  2. How should we handle the callbacks with server.register/server.start ? Do we need to wait for the plugins to be registered to start the server, or is it ok to start the server right after calling the server.register function?
    e.g:
server.register({ register: plugin }, function (err) {
    server.start(function (err) { /*...*/ })
})

vs

server.register({ register: plugin }, function (err) {
    /*...*/
});
server.start(function (err) { /*...*/ });

@AdriVanHoudt
Copy link
Contributor

@Couto

  1. I assume yes since the issue is tagged as 0.0.2
  2. You should wait for all your plugins to be registered otherwise the route might not have added to the connection when starting the server

@Couto
Copy link

Couto commented Mar 19, 2015

@AdriVanHoudt

  1. you're right, I didn't notice the 0.0.2 tag. :)
  2. It makes sense, I was just wondering, since I've seen both ways.

@AdriVanHoudt
Copy link
Contributor

@Couto as far as I'm concerned the later is just plain wrong :P

@idanwe
Copy link
Contributor

idanwe commented Mar 19, 2015

Should server.start be called after server.register?

    server.register({ register: Version }, function (err) {

        Hoek.assert(!err, err);
        server.start(function (err) {

            Hoek.assert(!err, err);
            console.log('Server started at: ' + server.info.uri);
        });
    });
    server.register({ register: Version }, function (err) {

        Hoek.assert(!err, err);
    });

    server.start(function (err) {

        Hoek.assert(!err, err);
        console.log('Server started at: ' + server.info.uri);
    });

Both seems to work (also the second option when switching the register and the start)

@AdriVanHoudt
Copy link
Contributor

Better to start the server in the register callback so you are sure all the plugins are loaded correctly before starting

@hueniverse
Copy link
Author

General feedback:

  • require('./version') should be declared at the top and assigned to Version.
  • no need to wrap the plugin in { register: Version }. It is enough to just pass Version directly to the register() function.
  • no need for specifying the plugin version in the attributes. This is an internal plugin, not a publicly published one.
  • server.start() must be inside the callback to server.register() otherwise the server will start before all the routes are added.

@MylesBorins
Copy link
Contributor

@hueniverse is there a preferred pattern for handling async when you are registering multiple plugins?

MylesBorins pushed a commit to MylesBorins/hueniversity that referenced this issue Mar 22, 2015
@hueniverse
Copy link
Author

You can pass an array if they all share the same (or lack of) options. Otherwise, you can use the glue module or create your own utility.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.