"deep-linking" to anchor links not always working (proposed solution)

[arikorn]

Have you read the Contributing Guidelines on issues?

• I have read the Contributing Guidelines on issues.

Prerequisites

• I'm using the latest version of Docusaurus.
• I have tried the npm run clear or yarn clear command.
• I have tried rm -rf node_modules yarn.lock package-lock.json and re-installing packages.
• I have tried creating a repro with https://new.docusaurus.io.
• I have read the console error message carefully (if applicable).

Description

Here's a proposed fix for: When linking to an anchor-ref in a docusaurus document, an apparent race condition may cause the in-page scroll to fail. This appears to be an old issue (#8351 for example and is an acknowledged problem in dev -- also mentioned in the CLI documentation). Even though this issue mostly affects dev environments, fixing it would be a boon for developers...and the race condition that causes it can make it a problem with "slow-loading" apps.

The fix is fairly straightforward: here's a link to my code
The same code is copied in the sandbox linked below, as well, but is commented out in the config for the demo (see below). FWIW, the code has been picked apart by code rabbit to cover all sorts of edge cases, see my related Companion PR for details and diagnostics (you'll have to expand resolved issues to see descriptions of the edge cases)...

I think the fix would go here more permanently if you agree with the need?? I'd be happy to give it a shot with your support.

Reproducible demo

https://codesandbox.io/p/devbox/silly-andras-68yl3g?file=%2Fdocusaurus.config.js%3A156%2C5-156%2C18

Steps to reproduce

1. Go to the demo link
2. Try the three different anchor-nav links. Only the second one will take you to the anchor ("Generate a new site"; the other two go to the top of the page)
3. If you uncomment the clientModules line near the end of docusaurus.config.js (make sure it recompiles) and try again everything works.

Expected behavior

all three links should go to "Generate a new site" in the intro page.

Actual behavior

see above

Your environment

• Public source code: code sandbox and GitHub linked above
• Public site URL: see above
• Docusaurus version used: "3.9.2"
• Environment name and version (e.g. Chrome 89, Node.js 16.4): latest chrome, sandbox
• Operating system and version (e.g. Ubuntu 20.04.2 LTS): code sandbox

Self-service

• I'd be willing to fix this bug myself.

[Josh-Cena]

Thanks. I don't think your code is a bad idea, but it's certainly very opinionated and may be surprising to many because of the arbitrary waiting. We try to minimize this kind of waiting pattern because it's extremely fragile to processing speed and computer specifics, so I don't think it's good to be included by the core.

[arikorn]

@Josh-Cena what arbitrary waiting? Did you miss the if (!tryScroll()) {? It only waits if the initial scroll failed. Quite to the contrary, this is extremely robust to processing speed -- that's the whole point of it!

Also, in what way is it opinionated that isn't easily modifiable?

[arikorn]

(just to be clear it will try from 0 - 2000 ms, in 50 ms increments)

[Josh-Cena]

Any waiting is considered arbitrary :) The entire Docusaurus has two timeouts: one in waiting to display the progress bar, one in animating the copy button. A good solution should be able to detect when the page has stabilized and thus be able to scroll. A timeout is not tethered to the timeline of real events and thus opinionated because you have introduced fixed constants.

[arikorn]

It turns out that none of the timing is necessary to solve the problem (in most? all? cases). Take a look at the code sandbox demo linked above and check out /src/hash-nav.js. I commented out all of the timing and uncommented the clientModules line in the config file (see my OP). So it appears that running onRouteDidUpdate already does what you want and the rest was just an overabundance of caution on my part.