Are calls to override wanted?
Eomm opened this issue · 1 comments
Eomm commented
Reading the docs (master&next) of the after
function:
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
mcollina commented
Fire a PR against next, I don’t think that’s ok.