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

Coordinating with RtD about upcoming changes #795

Open
1 of 13 tasks
pradyunsg opened this issue May 6, 2024 · 17 comments
Open
1 of 13 tasks

Coordinating with RtD about upcoming changes #795

pradyunsg opened this issue May 6, 2024 · 17 comments
Labels
needs discussion The approach to resolve this isn't immediately clear, and needs discussion

Comments

@pradyunsg
Copy link
Owner

pradyunsg commented May 6, 2024

What's happening?

Read the Docs is going to make changes to both how they're building Sphinx projects as well as how they're injecting functionality into the rendered documentation sets.

https://github.com/readthedocs/addons is the upcoming JS client to enable this functionality, which would be injected server-side when serving HTML pages AFAICT.

Reproducer

N/A

Expectation

This issue would be resolved with Furo making changes to all the spots where it depends on some details related to RtD. As of 2024.04.27, these are:

Code of Conduct

@pradyunsg pradyunsg added the needs discussion The approach to resolve this isn't immediately clear, and needs discussion label May 6, 2024
@pradyunsg
Copy link
Owner Author

See #785 where @humitos reached out about these upcoming changes on their end.

@pradyunsg
Copy link
Owner Author

pradyunsg commented May 6, 2024

Hmm... I think I might need to know the details of how things work on the client-side (readthedocs/addons#59) to make progress here.

@humitos
Copy link
Contributor

humitos commented May 6, 2024

I wasn't able to document the process in a clear way yet, but I did the implementation of what I think is a good pattern to follow using our own theme at readthedocs/sphinx_rtd_theme#1526. I'd appreciate any feedback you may have about it.

I did the same for CPython at python/cpython#116966 that you can also take a look. The usage of the data is the same, but it changes how the flyout/selectors are generated. Let me know. I'm happy to help here.

@pradyunsg
Copy link
Owner Author

pradyunsg commented May 6, 2024

Hmm... http://docs.devthedocs.org/_/addons/?project-slug=docs&version-slug=latest&client-version=0.12.0&api-version=1 from description of readthedocs/sphinx_rtd_theme#1526 is a 404 for me.


Just to make sure my understanding correct:

  • The deprecation is upcoming (i.e. no changes have been made so far to how projects are built / pages are served).
    • The new client is NOT being injected into pages today, when the documentation is being served.
  • Once the migration is complete, the final state will be:
    • No build-time injection, tweaks or Sphinx extensions. Only the various environment variables would be set.
    • Client-side injection of the new addons JS file, which would provide the relevant data to the theme. The theme is expected to handle injecting the information as appropriate.

If that understanding is correct, then I have the following questions:

  1. Is it feasible to have a transition like the following?

    RtD Functionality Now Now + t1 Now + t2 Now + t3
    Existing behaviours 🟢 🟢 🟠 🔴
    New behaviours 🔴 🟢 🟢 🟢

    🟢: Functional for RtD projects built during this period.
    🟠: Deprecated, functional for RtD projects built during this period.
    🔴: Non-functional.

    (this would mean that I could just wait until the new client is being injected-when-serving in all newly-built docs and also would have confidence that any new RtD build would work correctly)

  2. Is there any way for the theme to check whether the new functionality is going to be enabled / the new client would be injected etc during the build process?

  3. Could you help me figure out what would be equivalent to the things I'm doing in Furo's conf.py gated behind a dev-only conditional + some static files, which functionally is trying to inject all the pieces that RtD's current build-process injection would inject?

    furo/docs/conf.py

    Lines 113 to 132 in 750fcd7

    RTD_TESTING = False
    if RTD_TESTING or "FURO_RTD_TESTING" in os.environ:
    del html_theme_options["footer_icons"]
    html_css_files += [
    "https://assets.readthedocs.org/static/css/readthedocs-doc-embed.css",
    "https://assets.readthedocs.org/static/css/badge_only.css",
    ]
    html_js_files += [
    "readthedocs-dummy.js",
    "https://assets.readthedocs.org/static/javascript/readthedocs-doc-embed.js",
    ]
    html_context["READTHEDOCS"] = True
    html_context["current_version"] = "latest"
    html_context["conf_py_path"] = "/docs/"
    html_context["display_github"] = True
    html_context["github_user"] = "pradyunsg"
    html_context["github_repo"] = "furo"
    html_context["github_version"] = "main"
    html_context["slug"] = "furo"

    var READTHEDOCS_DATA = {
    project: "furo",
    version: "latest",
    language: "en",
    proxied_api_host: "https://readthedocs.org",
    features: {
    docsearch_disabled: true,
    },
    };

    Ideally, I'd prefer to not need to change browser configuration to make things work (I need to test against multiple browsers on multiple platforms somewhat regularly, and the current workflow involves doing so with a network-local server that is accessed from different machines on the network 😅).

@pradyunsg
Copy link
Owner Author

Oh, and thanks for reaching out! It's appreciated! ^>^

@humitos
Copy link
Contributor

humitos commented May 6, 2024

Hmm... http://docs.devthedocs.org/_/addons/?project-slug=docs&version-slug=latest&client-version=0.12.0&api-version=1 from description of readthedocs/sphinx_rtd_theme#1526 is a 404 for me.

Ups... That's a development instance URL. The one from production is https://docs.readthedocs.io/_/addons/?project-slug=docs&version-slug=latest&client-version=0.12.0&api-version=1


We've already made the changes for this pattern. Following your table, we are at Now + t1, were the old behavior is still working and the new behavior is deployed as well.

Projects using build.commands on their configuration file and/or opting-in into beta addons from project's setting are getting the new behavior: the JS client is automatically injected on each page.

Related blog posts:

I think this answers most of your questions and gives you a way to play around with this. You can see a live example of the implementation from our theme in our own documentation (it uses the PR that I shared previously): https://docs.readthedocs.io/en/stable/

3. Could you help me figure out what would be equivalent to the things I'm doing in Furo's conf.py gated behind a dev-only conditional + some static files, which functionally is trying to inject all the pieces that RtD's current build-process injection would inject?

Yes, but I'm leaving now 😄 . I will come to this issue tomorrow's morning.

humitos added a commit to humitos/furo that referenced this issue May 8, 2024
@humitos
Copy link
Contributor

humitos commented May 8, 2024

3. Could you help me figure out what would be equivalent to the things I'm doing in Furo's conf.py gated behind a dev-only conditional + some static files, which functionally is trying to inject all the pieces that RtD's current build-process injection would inject?

I opened #797 to talk more about this directly in the code, since I think it will be easier to know exactly what we are doing and where. Let me know if this is helpful as a first step.

I will come to this issue tomorrow's morning.

It wasn't tomorrow, but close enough 😄

@humitos
Copy link
Contributor

humitos commented Jun 3, 2024

I just want to check here if you were able to take a look at the PR that I opened that explains a little how to integrate the new Read the Docs Addons. I'm happy to help here with this work, but I would need to understand how do you run the tests so I can experiment with that. Let me know if there is anything else here I can do to help with this.

@humitos
Copy link
Contributor

humitos commented Jul 3, 2024

I wanted to share an update about our deprecation plan here 1:

  1. Soon, we will disable the Sphinx context injection to all new projects created on Read the Docs. That means that a bunch of variables that were available on Sphinx templates won't be available anymore.
  2. New projects will have all Read the Docs Addons enabled by default.
  3. We've created a Sphinx extension (https://github.com/readthedocs/sphinx-build-compatibility/) for users that want to opt-in for the old behavior --this extension will inject all those variables that we are removing. We will be recommending this extension as temporal solution while authors/themes migrate their code. However, we won't maintain this extension for too long.
  4. After some time testing this new implementation, we will disable the Sphinx context injection on all projects and enable addons on all projects at the same time.

I'd appreciate any feedback you may have about this plan. Also, I'm happy to help you with any code modification required on furo or on any other theme you are involved. Let me know 👍🏼

Footnotes

  1. https://github.com/readthedocs/addons/issues/72#issuecomment-2200390705

@humitos
Copy link
Contributor

humitos commented Jul 9, 2024

🗞️ We are about to publish a post in our blog about this plan: readthedocs/website#308

@pradyunsg
Copy link
Owner Author

This has been on my radar but free time hasn't been coming around often enough to get to this. Pesky things like cutting pip releases keep getting in the way.

@flying-sheep
Copy link

@humitos looks like you already went forward with this for new projects, as a project I created today doesn’t integrate anymore (html_context doesn‘t seem to be manipulated).

You say you’re suggesting sphinx-build-compatibility, but that’s not on PyPI.

@humitos
Copy link
Contributor

humitos commented Aug 2, 2024

Hi @flying-sheep. New projects are using the new behavior, as you already noticed. We announced this in our blog at https://about.readthedocs.com/blog/2024/07/addons-by-default/

Regarding the Sphinx extension, you can find it at https://github.com/readthedocs/sphinx-build-compatibility/ and installing it directly from GitHub.

I'd appreciate any feedback you may have.

(from the phone 📱)

@flying-sheep
Copy link

flying-sheep commented Aug 2, 2024

That’s pretty clumsy, I basically have to add this to conf.py

from importlib.util import find_spec

_sbc = (
    ["sphinx_build_compatibility.extension"]
    if find_spec("sphinx_build_compatibility")
    else []
)

extensions = [..., *_sbc]

then add a file docs.requirements-sbc.txt:

sphinx-build-compatibility @ git+https://github.com/readthedocs/sphinx-build-compatibility.git

then reference that in .readthedocs.yml:

 python:
   install:
     - method: pip
       path: .
       extra_requirements:
         - docs
+    - requirements: docs/requirements-sbc.txt

If you publish it on PyPI, @pradyunsg could at least do a hotfix where Furo would depend on that extension until Furo has migrated.

And it’s still not clean:

image

@humitos
Copy link
Contributor

humitos commented Aug 2, 2024

So, even installing that extension you are not getting the flyout properly integrated? If so, can you please open an issue on that extension that I mentioned in my previous comment so we can track it there, please.

Also, mention your read the docs project and the build link.

@pradyunsg
Copy link
Owner Author

Oh, right. I'll try to make time this weekend for this (sorry, dealing with some family stuff that's needed attention so I've been slipping on a few things like this). 😅

@flying-sheep
Copy link

I think by now the changes have rolled out …

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs discussion The approach to resolve this isn't immediately clear, and needs discussion
Projects
None yet
Development

No branches or pull requests

3 participants