Skip to content

Async afterEach hook breaks ready and doneEach hooks #449

Description

@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.

Activity

  1. jhildenbiddle commented on Apr 16, 2018

    @jhildenbiddle
    Member

    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)
  2. jhildenbiddle commented on May 31, 2018

    @jhildenbiddle
    Member

    @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!

  3. stale commented on Feb 4, 2020

    @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.

  4. added
    bugconfirmed as a bug
    and removed on Jan 23, 2022
  5. trusktr commented on Jan 23, 2022

    @trusktr
    Member

    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).

  6. self-assigned this
    on Jan 23, 2022
  7. jhildenbiddle commented on Mar 13, 2022

    @jhildenbiddle
    Member

    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:

    This fix will not break these types of workarounds.

  8. changed the title [-]Async afterEach hook breaks doneEach-event dependent plugins.[/-] [+]Async afterEach hook breaks ready and doneEach dependent plugins.[/+] on Mar 13, 2022
  9. changed the title [-]Async afterEach hook breaks ready and doneEach dependent plugins.[/-] [+]Async afterEach hook breaks ready and doneEach[/+] on Mar 13, 2022
  10. changed the title [-]Async afterEach hook breaks ready and doneEach[/-] [+]Async afterEach hook breaks ready and doneEach hooks[/+] on Mar 13, 2022
  11. added a commit that references this issue on Mar 13, 2022
    87c602e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugconfirmed as a bug

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions