docsify icon indicating copy to clipboard operation
docsify copied to clipboard

The toc (with subMaxLevel) is generated twice when there's 2 embedded links in a page

Open julienw opened this issue 3 years ago • 2 comments

Bug Report

Steps to reproduce

  1. Use this configuration:
      window.$docsify = {
        subMaxLevel: 2,
        loadSidebar: true
      };
  1. Use a _sidebar.md, it could be as simple as:
- [Home](/)
  1. add a file with 2 embedded files, such as:
# Hello

Hello world

## Title 1

Lorem ipsum

[insert something](file.mp4 ":include")

[insert something else](file.mp4 ":include")

You can also see it live in https://codesandbox.io/s/problem-with-docsify-4cyldv

What is current behaviour

The Toc is generated twice, but in a nested way. In the above example, this looks like it:

  • Home
    • Title 1
    • Hello
      • Title 1

See also this screenshot: image

What is the expected behaviour

We have the TOC only once, this should look like this in this case:

  • Home
    • Title 1

This happens when there's only one embedded asset.

Other relevant information

  • [X] Bug does still occur when all/other plugins are disabled?

  • Your OS: not relevant

  • Node.js version: not relevant

  • npm/yarn version: not relevant

  • Browser version: Firefox 103, but I also tried in Chrome stable, so I believei t's not relevant

  • Docsify version: 4.12.2

  • Docsify plugins: none

Please create a reproducible sandbox

https://codesandbox.io/s/problem-with-docsify-zl23lm

Mention the docsify version in which this bug was not present (if any)

I haven't tried earlier versions, it's the first time I try this.

julienw avatar Jun 22 '22 07:06 julienw

I tried to debug, and I think the issue is that the 2 callbacks are called for each of the token:

https://github.com/docsifyjs/docsify/blob/c49c39a4a2d6528ec27a314ab5fdc1e274af685f/src/core/render/embed.js#L75-L78

As a result, done here is called twice: https://github.com/docsifyjs/docsify/blob/c49c39a4a2d6528ec27a314ab5fdc1e274af685f/src/core/render/embed.js#L152

And then this is called twice: https://github.com/docsifyjs/docsify/blob/54cc5f939b9499ae56604f589eef4e3f1c13cdc5/src/core/render/index.js#L340-L341

Which isn't usually a problem (except things may be rendered twice), but in the case of the sidebar toc, this causes the toc to be rendered twice with: https://github.com/docsifyjs/docsify/blob/54cc5f939b9499ae56604f589eef4e3f1c13cdc5/src/core/render/compiler.js#L239

Then the duplicated toc is used in this routine: https://github.com/docsifyjs/docsify/blob/35002c92b762f0d12e51582d7de7914fa380596a/src/core/render/compiler.js#L306-L327

Here only the first toc entry is removed, if it's a depth 1.

Therefore I can see these possible fixes:

  1. ~~Remove all TOC entries with depth==1 when generating the sidebar. This is more a hacky quick fix than a longterm solution IMO, but this could at least fix this issue I believe.~~ No that wouldn't work anyway, because the subheadings are also present, so the Toc would still be duplicated, but not nested.
  2. Make sure that done is called only once for each render. I don't understand enough the logic with step and count that are both incremented, so I don't know what's the intent here...

Hpoe this helps!

julienw avatar Jun 22 '22 07:06 julienw

I think I understood the dance with the incrementations: when the token has a url, then the process is asynchronous, so the condition is there to ensure that we call the callback after everything has been fetched. But this doesn't work when the token is just some html, because in that case the process is synchronous. ~~The fix is to make this case asynchronous as well. I think I can make a patch for that.~~

update: Studying the code some more, I decided to use the length of the embedTokens source instead of using step. This makes things a lot more predictable in my opinion.

julienw avatar Jun 22 '22 14:06 julienw