Skip to content

Commit

Permalink
fix: delay plugin loading until next tick (#177)
Browse files Browse the repository at this point in the history
* fix: delay plugin loading until next tick

* move `loadPluginNextTick` function alongside `loadPlugin` and add comment

* refactor loadPluginNextTick to use a bound function rather than an anonymous closure

* remove top-level FS read from unit test that was preventing the bug from occurring

* add additional test case
  • Loading branch information
chrskrchr authored Mar 3, 2022
1 parent af141de commit a8dcf0e
Show file tree
Hide file tree
Showing 4 changed files with 107 additions and 1 deletion.
9 changes: 8 additions & 1 deletion plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ function Plugin (parent, func, optsOrFunc, isAfter, timeout) {
this.timeout = timeout === undefined ? parent._timeout : timeout
this.name = getName(func, optsOrFunc)
this.isAfter = isAfter
this.q = fastq(parent, loadPlugin, 1)
this.q = fastq(parent, loadPluginNextTick, 1)
this.q.pause()
this._error = null
this.loaded = false
Expand Down Expand Up @@ -245,6 +245,13 @@ Plugin.prototype.finish = function (err, cb) {
this.q.resume()
}

// delays plugin loading until the next tick to ensure any bound `_after` callbacks have a chance
// to run prior to executing the next plugin
function loadPluginNextTick (toLoad, cb) {
const parent = this
process.nextTick(loadPlugin.bind(parent), toLoad, cb)
}

// loads a plugin
function loadPlugin (toLoad, cb) {
if (typeof toLoad.func.then === 'function') {
Expand Down
70 changes: 70 additions & 0 deletions test/await-after.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ const { test } = require('tap')
const boot = require('..')
const { promisify } = require('util')
const sleep = promisify(setTimeout)
const fs = require('fs').promises
const path = require('path')

test('await after - nested plugins with same tick callbacks', async (t) => {
const app = {}
Expand Down Expand Up @@ -306,6 +308,74 @@ test('await after complex scenario', async (t) => {
}
})

test('without autostart and sync/async plugin mix', async (t) => {
const app = {}
boot(app, { autostart: false })
t.plan(21)

let firstLoaded = false
let secondLoaded = false
let thirdLoaded = false
let fourthLoaded = false

app.use(first)
await app.after()
t.ok(firstLoaded, 'first is loaded')
t.notOk(secondLoaded, 'second is not loaded')
t.notOk(thirdLoaded, 'third is not loaded')
t.notOk(fourthLoaded, 'fourth is not loaded')

app.use(second)
await app.after()
t.ok(firstLoaded, 'first is loaded')
t.ok(secondLoaded, 'second is loaded')
t.notOk(thirdLoaded, 'third is not loaded')
t.notOk(fourthLoaded, 'fourth is not loaded')

await sleep(10)

app.use(third)
await app.after()
t.ok(firstLoaded, 'first is loaded')
t.ok(secondLoaded, 'second is loaded')
t.ok(thirdLoaded, 'third is loaded')
t.notOk(fourthLoaded, 'fourth is not loaded')

app.use(fourth)
t.ok(firstLoaded, 'first is loaded')
t.ok(secondLoaded, 'second is loaded')
t.ok(thirdLoaded, 'third is loaded')
t.notOk(fourthLoaded, 'fourth is not loaded')

await app.after()
t.ok(firstLoaded, 'first is loaded')
t.ok(secondLoaded, 'second is loaded')
t.ok(thirdLoaded, 'third is loaded')
t.ok(fourthLoaded, 'fourth is loaded')

await app.ready()

async function first () {
firstLoaded = true
}

async function second () {
const contents = await fs.readFile(path.join(__dirname, 'fixtures', 'dummy.txt'), 'utf-8')
t.equal(contents, 'hello, world!')
secondLoaded = true
}

async function third () {
await sleep(10)
thirdLoaded = true
}

function fourth (server, opts, done) {
fourthLoaded = true
done()
}
})

test('without autostart', async (t) => {
const app = {}
boot(app, { autostart: false })
Expand Down
28 changes: 28 additions & 0 deletions test/await-use.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
'use strict'

const { test } = require('tap')
const { promisify } = require('util')
const sleep = promisify(setTimeout)
const boot = require('..')

test('await use - nested plugins with same tick callbacks', async (t) => {
Expand Down Expand Up @@ -242,3 +244,29 @@ test('mixed await use and non-awaited use ', async (t) => {
fourthLoaded = true
}
})

test('await use - mix of same and future tick callbacks', async (t) => {
const app = {}
boot(app, { autostart: false })
let record = ''

t.plan(4)

await app.use(async function plugin0 () {
t.pass('plugin0 init')
record += 'plugin0|'
})
await app.use(async function plugin1 () {
t.pass('plugin1 init')
await sleep(500)
record += 'plugin1|'
})
await sleep(1)
await app.use(async function plugin2 () {
t.pass('plugin2 init')
await sleep(500)
record += 'plugin2|'
})
record += 'ready'
t.equal(record, 'plugin0|plugin1|plugin2|ready')
})
1 change: 1 addition & 0 deletions test/fixtures/dummy.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
hello, world!

0 comments on commit a8dcf0e

Please sign in to comment.