fastify/avvio

Are calls to override wanted?

Eomm opened this issue · 1 comments

Eomm commented

Reading the docs (master&next) of the afterfunction:

If three parameters are given to the callback, the first will be the error object, the second will be the top level context unless you have specified both server and override, in that case the context will be what the override returns, and the third the done callback.

My understanding is that the .after interfaces with one or two params don't call the override
Instead override is always called:

'use strict'

const server = { count: 0 }
const avvio = require('.')(server)

avvio.override = function (s, fn, opts) {
  console.log('override', s.count)
  const res = Object.create(s)
  res.count = res.count + 1
  return res
}

avvio
  .use(function hello (server, opts, done) {
    console.log('hello server', server.count)
    done()
  })
  .after(function (err, done) {
    if (err) throw err
    done()
  })
  .after(function (err, context, done) {
    if (err) throw err
    console.log('context', context.count)
    done()
  })

avvio.ready(() => { console.log('ready') })

Show:

override 0
hello server 1
override 0
override 0
context 0
ready
start

is this wanted/needed?
Does not this behaviour add unuseful overhead?

With this (fast) code on plugin.js all our tests are green and also on fastify.next all is green:

if (!this.isAfter || func.length === 3 || (func.length === 2 && func.then)) {
  this.server = this.parent.override(server, func, this.opts)
}

Doubt born by fastify/fastify#2166

Fire a PR against next, I don’t think that’s ok.