outmoded/university

Convert to a plugin

hueniverse opened this issue · 23 comments

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

Nice git guide!

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

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?

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

OHHHHHH I think it is from my rebasing

aaah probably yes

Btw

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

FTW!!!

for adding to an existing commit?

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

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

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

@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.

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

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) { /*...*/ });

@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

@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.

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

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)

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

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.

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

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.