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

Use inside use doesn't work anymore #30

Closed
allevo opened this issue Oct 14, 2017 · 14 comments
Closed

Use inside use doesn't work anymore #30

allevo opened this issue Oct 14, 2017 · 14 comments

Comments

@allevo
Copy link
Member

allevo commented Oct 14, 2017

Using fastify, this code works correctly in 0.28 (avvio version 2):

function pluginOk (fastify, opts, next) { next() }

t.test('ok', t => {
  t.plan(2)
  const fastify = Fastify()

  fastify.register(require('./index'), err => {
    t.error(err)
    fastify.register(pluginOk, err => {
      t.error(err)
    })
  })
})

In 0.30.2 (avvio version 3) the second error has this message: Impossible to load "pluginOk" plugin because the parent "" was already loaded

This behaviour is introduced by #29 on avvio version 3.

How can I rewrite the previous code in new avvio version?

@mcollina
Copy link
Member

what is in ./index.js?

@allevo
Copy link
Member Author

allevo commented Oct 14, 2017

index.js is another plugin like

function antoherPlugin(fastify, opts, next) { next() }

@mcollina
Copy link
Member

The problem are nested register calls. Don’t use the callbacks.

@allevo
Copy link
Member Author

allevo commented Oct 14, 2017 via email

@mcollina
Copy link
Member

It’s already handled, you don’t have to do anything. Use another register in the main plugin.

@allevo
Copy link
Member Author

allevo commented Oct 15, 2017

I don't understand. My situation is like this (in fastify):

function plugin1 (fastify, opts, next) {
  setTimeout(function () { // silumate some async
    fastify.decorate('foo', 4) // decorating with the result of the async operation
  }, 100)
}

function plugin2 (fastify, opts, next) {
  console.log(fastify.foo) // using the previous set value
}

How can I register plugin2 underlining the dependency to plugin1 without wrinting a callback hell?

this doesn't work anymore

EDIT:
or

How can I write the dependency betweet plugin2 and fastify-mongodb?

function plugin2 (fastify, opts, next) {
  console.log(fastify.mongo) // use fastify-mongodb
}

@mcollina
Copy link
Member

Just don’t use series. It will work as expected, it will be present in any subsequent plugin that you use. The callback in register is kind of useless.

@wooliet
Copy link

wooliet commented Oct 15, 2017

I'm very confused regarding whatever it is that has changed.

I would imagine developers would want to know that something such as a DB connection is broken before the call to fastify.listen (for example, the mongodb plugin).

My own code is currently split into a collection of plugins, each of which is registered, and then a collection of routes. This now breaks with the "already loaded" error.

Can the expected use of register be addressed somewhere in the fastify repo?

@mcollina
Copy link
Member

@wooliet there is no need to declare the routes after loading all the dependent plugins. This is already done for you, just call get() (etc) after the relevant register(). Error handing is centralized, if a plugin does not boot, everything stops and error.

If you need to wait for whatever reason, wrap it in a plugin.

The previous order was non-deterministic, and it caused some weird bugs. This is very deterministic. Really, you should not use the register() callback. I'll try to remove that in the next release as it's a source of confusion. You might only want to use it if you need to unwind some other state in case of error.

@wooliet
Copy link

wooliet commented Oct 15, 2017

@mcollina I'm sorry to bother you guys with this, and I'll be glad to move this to the fastify repo if you prefer. But I think there's a communication breakdown somewhere.

Copied below is code that is representative of what used to work (fastify 0.29.2) but now (0.30.2) results in:

Error: Impossible to load "bound _after" plugin because the parent "formBodyPlugin" was already loaded

I am interested in what an updated version should look like.

const async = require('async');
const formbody = require('fastify-formbody');
const fastify = require('fastify')();

async.waterfall([
  next => plugins(next),
  next => routes(next)
], (err) => {
  if(err) process.exit(1);
  fastify.listen('9090', '0.0.0.0', err => console.log('running'));
})


function plugins(done) {
  fastify.register(formbody, {}, (err) => {
    done(err);
  });
}

function routes(done) {
  fastify.get('/ping', (req, resp) => {
    resp.code(204).send();
  });
  done();
}

@allevo
Copy link
Member Author

allevo commented Oct 15, 2017

@mcollina we should remove the callback definitely.
The callback makes developer confused.

Anyway,

fastify.register(require('fastify-mongodb'), {})
fastify.register(function (f, o, n) {
  var myColl = f.mongo.db.collection('my-collection') // f.mongo is defined! 
})

works for me.

@wooliet
Copy link

wooliet commented Oct 15, 2017

@allevo at what point in that flow do you call fastify.listen?

@wooliet
Copy link

wooliet commented Oct 15, 2017

Maybe my hangup is that I don't want to open up the port for incoming requests until I know everything has successfully initialized. But if that's not possible anymore then ok, no biggie.

@allevo
Copy link
Member Author

allevo commented Oct 15, 2017

@wooliet

'use strict'

fastify.register(require('fastify-mongodb'), {}) // set up the mongo connection

fastify.register(function (f, o, n) {
  // please don't drop you database in production!!!!
  return f.mongo.db.dropDatabase() // the connection is made
})

fastify.register(function setRoutes(f, o, n) {
  var myColl = f.mongo.db.collection('my-collection') // the connection is made
  f.get('/', function (req, reply) {
    reply.send(myColl.find({}).toArray())
  })
})

fastify.listen(function (err) {
  // server up if err is falsy!
})

I'll close this conversation because the issue is "resolved": the use method (aka register in fastify) keeps the same call order.

If you are other questions/issues please open another one (in fastify or avvio)

@allevo allevo closed this as completed Oct 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants