Async afterEach hook breaks ready and doneEach hooks

[imyelo]

Async afterEach hooks such as:

$docsify.plugins = [].concat(install, $docsify.plugins)

function install (hook, vm) {
  hook.afterEach(function (html, next) {
    setTimeout(function () {
      next(html)
    }, 1000)
  })
}

breaking doneEach events dependent plugins, such as zoom-image.

Demo:
https://docsify-pagination-issue-1-nzhdhlzryc.now.sh/#/
Source of demo:
https://docsify-pagination-issue-1-nzhdhlzryc.now.sh/_src

(Came from imyelo/docsify-pagination#1)

I guess the next function in this place should run after callback:
https://github.com/QingWei-Li/docsify/blob/e5a263f1bb0909e1629abf76e327f7f2b50340d8/src/core/render/index.js#L149

For the current version, however, if callback is an asynchronous function, this assumption will be broken.

[jhildenbiddle]

It looks like neither the beforeEach() or afterEach() hooks are handling async tasks properly.

For example, here is a synchronous plugin using each hook (drop this in your docsify index.html):

<script>
    function pluginSync(hook, vm) {
        hook.init(function() {
            console.log('init()');
        });

        hook.mounted(function() {
            console.log('mounted()');
        });

        hook.beforeEach(function(content) {
            console.log('beforeEach()');
            return content;
        });

        hook.afterEach(function(html, next) {
            console.log('afterEach()');
            next(html);
        });

        hook.doneEach(function() {
            console.log('doneEach()');
        });

        hook.ready(function() {
            console.log('ready()');
        });
    }

    $docsify.plugins = [].concat($docsify.plugins, pluginSync);
</script>

Here is the output of the synchronous plugin. The order is what one would expect.

init()
mounted()
beforeEach()
afterEach()
doneEach()
ready()

Here is the same plugin but with async beforeEach() and afterEach() tasks:

<script>
    function pluginAsync(hook, vm) {
        hook.init(function() {
            console.log('init()');
        });

        hook.mounted(function() {
            console.log('mounted()');
        });

        hook.beforeEach(function(content) {
            setTimeout(function() {
                console.log('beforeEach()');
                return content;
            }, 2000);
        });

        hook.afterEach(function(html, next) {
            setTimeout(function() {
                console.log('afterEach()');
                next(html);
            }, 1000);
        });

        hook.doneEach(function() {
            console.log('doneEach()');
        });

        hook.ready(function() {
            console.log('ready()');
        });
    }

    $docsify.plugins = [].concat($docsify.plugins, pluginAsync);
</script>

This is what I would expect:

init()
mounted()
ready()
beforeEach()
afterEach()
doneEach()

But this is the actual result:

init()
mounted()
doneEach()   <-- Should fire after afterEach()
ready()
afterEach()  <-- Should fire after beforeEach()
beforeEach() <-- Should fire before afterEach()

Summary: async afterEach() and beforeEach() don't work as they should.

• When beforeEach() is complete, it should call afterEach()
• When afterEach() is complete, it should call doneEach()
• Ideally, a loadedEach() would also be availalble which would trigger when all data is loaded (including embed data loaded via :include)

[jhildenbiddle]

@QingWei-Li -- Any chance we can get this marked as a bug and addressed in a future release? This behavior 1) breaks async plugins which 2) increases the likelihood of errors being thrown that break docsify.

Thanks!

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[trusktr]

I would expect the output to be in the same order. Like this:

init()
mounted()
// async begins
beforeEach()
afterEach()
doneEach()
ready()

The only difference is timing (the last four happen later, but in the same order). If we do otherwise, it may break something else.

But even if we make doneEach async, that might break something. Maybe we should "fix" it, then just increment the docsify version to 5 along with new migration guide with its first entry (an example of an organic change without piling up a huge v5 release, as we chatted on Discord).

[jhildenbiddle]

I was working with the plugin system recently, so I took a some time to review the code and find the issue.

Docsify supports asynchronous beforeEach and afterEach hooks by handling them as synchronous hooks (i.e., sequentially, in the order they exist in the $docsify.plugins array, regardless of whether they are sync or async). Consider the following hook sequence:

init()
beforeEach() - Sync, 0.1 seconds to complete
beforeEach() - Async, 2 seconds to complete
beforeEach() - Async, 1 seconds to complete
beforeEach() - Sync, 0.1 seconds to complete
// Content rendered
doneEach()
ready()

The beforeEach queue is invoked and completed in the order they appear above, taking a total of 3.02 seconds.

Now, let's change the sync/async beforeEach hooks for afterEach:

init()
afterEach() - Sync, 0.1 seconds to complete
doneEach()
ready()
afterEach() - Async, 2 seconds to complete
afterEach() - Async, 1 seconds to complete
afterEach() - Sync, 0.1 seconds to complete
// Content rendered

This is clearly not the intended behavior. Docsify correctly waits for all of the afterEach hooks to complete before rendering content, but the doneEach hook is invoked before the first async afterEach hook and before content has rendered (i.e., before Docsify is "done"). The behavior should be exactly like the first beforeEach example:

init()
afterEach() - Sync, 0.1 seconds to complete
afterEach() - Async, 2 seconds to complete
afterEach() - Async, 1 seconds to complete
afterEach() - Sync, 0.1 seconds to complete
// Content rendered
doneEach()
ready()

Fortunately, this turned out to be a fairly easy fix once I was able to wrap my head around how the plugin system works. I will create PR shortly with the fix.

FWIW, this fix should be considered a bug fix and not a breaking change as it has no effect on how content is process or rendered; it only ensures that ready() and doneEach() hooks are called after all content has been processed and rendered to the DOM (as expected). Several plugins have already had to work around this bug:

• Breaks with docsify "zoom-image" plugin imyelo/docsify-pagination#1
• https://github.com/jhildenbiddle/docsify-themeable/blob/23c699a47e6bebbc25c3d598a48565750979abfc/src/js/plugin-fix-ready-state.js#L9-L14

This fix will not break these types of workarounds.