hapijs / glue

Server composer for hapi.js
Other
245 stars 62 forks source link

using Hoek.clone makes some plugins stop working #71

Closed paulovieira closed 8 years ago

paulovieira commented 8 years ago

glue will call Hoek.clone in the parsePlugin method. This has the effect that some plugins stop working as expected when the values passed in the plugin options are objects or functions (in particular, streams).

Below is a simple demo that exhibits the problem. It just loads the good plugin and passes an instance of a stream to the options (from good-console). It should output some info about the server load every 1s, but an error is thrown:

Error: no writecb in Transform class

If we comment the Hoek.clone call, it works as expected.

The problem will be present whenever the user needs to access the same value that is given in the plugin options (not a clone).

// npm install glue good good-console
const Glue = require('glue');
const GoodConsole = require('good-console');

const internals = {};

internals.goodOptions = {
    ops: {
        interval: 1000
    },
    reporters: {
        'foo': [
            new GoodConsole(),
            'stdout'
        ]
    }
};

internals.manifest = {
    connections: [{ port: 8888 }],
    registrations: [{
        plugin: {
            register: 'good',
            options: internals.goodOptions
        }
    }]
};

Glue.compose(internals.manifest, (err, server) => {

    server.start();
});
csrl commented 8 years ago

@nlf this seems to be an issue with hoek.clone? Can you confirm?

I can work around it doing a cloneWithShallow, skipping the plugin options.

nlf commented 8 years ago

i'm not sure how to identify this as a bug in clone. is it that clone doesn't work properly on a stream? or is it that in this case cloneWithShallow is what should reasonably be used?

paulovieira commented 8 years ago

Hoek.clone is working fine. I should have been more clear in the issue: sometimes the user of glue needs access to the actual values passed in the options, not a copy. Streams are a good example.

Would it make sense to add a new clone option for glue (besides relativeTo, preConnections and preRegister)?

https://github.com/paulovieira/glue/tree/clone-option

csrl commented 8 years ago

Ok, I'll just use cloneWithShallow to fix this then. Thanks.

lock[bot] commented 4 years ago

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.