Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the race condition causing search not to initialize properly #2593

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mortenpi
Copy link
Member

Sometimes the search index takes longer to download than, and the documenter.js script already runs, in which case it will error with:

Uncaught ReferenceError: documenterSearchIndex is not defined
    <anonymous> https://docs.julialang.org/en/v1/assets/documenter.js:603
    execCb https://cdnjs.cloudflare.com/ajax/libs/require.js/2.3.6/require.min.js:1
    check https://cdnjs.cloudflare.com/ajax/libs/require.js/2.3.6/require.min.js:1
    enable https://cdnjs.cloudflare.com/ajax/libs/require.js/2.3.6/require.min.js:1
    bind https://cdnjs.cloudflare.com/ajax/libs/require.js/2.3.6/require.min.js:1
    emit https://cdnjs.cloudflare.com/ajax/libs/require.js/2.3.6/require.min.js:1
    each https://cdnjs.cloudflare.com/ajax/libs/require.js/2.3.6/require.min.js:1
    emit https://cdnjs.cloudflare.com/ajax/libs/require.js/2.3.6/require.min.js:1
    check https://cdnjs.cloudflare.com/ajax/libs/require.js/2.3.6/require.min.js:1
    enable https://cdnjs.cloudflare.com/ajax/libs/require.js/2.3.6/require.min.js:1
    init https://cdnjs.cloudflare.com/ajax/libs/require.js/2.3.6/require.min.js:1
    a https://cdnjs.cloudflare.com/ajax/libs/require.js/2.3.6/require.min.js:1
    completeLoad https://cdnjs.cloudflare.com/ajax/libs/require.js/2.3.6/require.min.js:1
    onScriptLoad https://cdnjs.cloudflare.com/ajax/libs/require.js/2.3.6/require.min.js:1

and the search UI doesn't work.

This effectively adds a loop with a sleep to the search code that will wait until the documenterSearchIndex is actually available, before running any of the search initialization code.

@mortenpi mortenpi added Format: HTML Related to the default HTML output Type: Bugfix labels Oct 29, 2024
@Hetarth02
Copy link
Contributor

@mortenpi

As far as the code looks, it just delays the function call by 1 second if the index is undefined.

Shouldn't we keep looping or something and give a bit more time for the index to load(slow networks/other problems). Then for example after 10 seconds the index doesn't load we could give an alert/console.warn something?

@mortenpi
Copy link
Member Author

It does loop. setTimeout will run waitUntilSearchIndexAvailable again, which will again check for the search index, and delay again if need be.

@Hetarth02
Copy link
Contributor

Hetarth02 commented Oct 30, 2024

It does loop.

Oh, condition based recursion until the index isn't defined.

(Is this been tested?)

Fair enough, the PR looks good to me.

P.S.: Needs a changelog entry though...

@mortenpi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Format: HTML Related to the default HTML output Type: Bugfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants