Fix backport event order in master
Eomm opened this issue · 1 comments
I'm merging master in next, and the alpha-2
showed this issue
This script:
const avvio = require('.')()
avvio.ready().then(() => {
console.log('ready')
})
// avvio.ready(() => {
// console.log('ready')
// })
avvio.on('start', () => {
console.log('start')
})
Produce:
Branch | Promise | Callback |
---|---|---|
next | ready then start | ready then start |
master | start then ready | ready then start |
Because of the change, this test is failing:
https://github.com/fastify/fastify/blob/9b533f5062356e27d997eeee076375deba37cb69/test/decorator.test.js#L712
Because the fastify's server is flaged as started
in the start
event here:
https://github.com/fastify/fastify/blob/9b533f5062356e27d997eeee076375deba37cb69/fastify.js#L287
I think ready->start
is correct execution order, but if I think in all my application's tests, await server.ready()
is quite the standard 😭
I would say that "start then ready" is definitely incorrect and not the intention. I'm happy to fix it in avvio master
.
Overall, the implementation of that behavior is wrong :/. I would recommend to fix it in master as well.