Proposal for sphinxcontrib-jquery

[benjaoming]

This is a proposal for a new Sphinx extension, as elaborated in the proposal itself.

Direct comments and suggestions are welcome through GitHub's code review.

A more general discussion or questions are welcome as comments to the Pull Request

Rendered version: https://dev--9665.org.readthedocs.build/en/9665/design/sphinx-jquery.html

Discussions about jQuery removal in Sphinx and extensions:

• Remove dependency on jQuery sphinx-doc/sphinx#7405
• Removing JavaScript Dependencies sphinx-doc/sphinx#10070
• Include jQuery as dependency sphinx-hoverxref#160
• Remove jQuery sphinx_rtd_theme#1253
• Remove dependence on JQuery or explicitly include it in the theme pydata/pydata-sphinx-theme#764

---

📚 Documentation previews 📚

• User's documentation (docs): https://docs--9665.org.readthedocs.build/en/9665/

• Developer's documentation (dev): https://dev--9665.org.readthedocs.build/en/9665/

[humitos]

I think this is simple enough and it's a good start. I would reduce the scope a little bit and and remove one of the method proposed to avoid handling Sphinx<6 since it's not the target of this extension.

[AA-Turner]

I've been directed to this discussion by Ben.

As opposed to the method, why not suggest app.setup_extension("sphinxcontrib.jquery")?

A

[benjaoming]

@AA-Turner [edit: sorry I now remember you don't like the additional @-mention]

That's interesting, I haven't considered that at all. However, one thing that would require an extra configuration value then would be force=True in case an extension needs more control. We consider this case for the "unexpected", i.e. an extension or theme that we haven't considered needs for and that want to load jQuery based on some other set of conditions. I'm not sure how to do that, I'm also not sure how realistic this case is. Would it be possible to register a config value jquery_force_enable and jquery_force_disable so that an additional layer of control is yielded to both other extensions and conf.py?

Maybe having config values is a better approach. It could ultimately mean that someone who wants to load jQuery by themselves (from their own CDN etc) is free to do so.

[AA-Turner]

Dear Ben,

I am not sure I understand the rationale for the force keyword, please may you briefly explain?

A

[benjaoming]

We introduce sphinxcontrib-jquery for other themes and extensions to be able to provide jQuery through the extension. The general case is clear: Themes and extensions should have an easy way to continue using jQuery.

However, do we want an additional layer of control for themes, extensions and projects to specify exactly when they want jQuery loaded?

That's the ultimate question. I think that the proposal might have chosen a wrong mechanism for that.

• If I write a theme or extension that has its own jQuery and I know that another extension might also load it, should I be able to switch off sphinxcontrib-jquery with a config value or some other mechanism?
• If I have a project with its own CDN, should I be able to switch off sphinxcontrib-jquery? (config value seems obvious here)
• If I have a theme or extension that doesn't even want Sphinx 5.x's jQuery but is still compatible with Sphinx 5.x? (not sure about this one, but it also seems unlikely)

[AA-Turner]

Dear Ben,

Thank you for your explanation; perhaps my understanding was too simple.

I had envisioned the extension as providing a way to pretend Sphinx had never removed jQuery, such that the jQuery library would be included as if it was provided by core Sphinx.

This would mean that extensions providing jQuery themselves would deal with the duplication similar to how they have in the past.

I do see some value in a global disable flag set somehow.

A

[benjaoming]

I had envisioned the extension as providing a way to pretend Sphinx had never removed jQuery, such that the jQuery library would be included as if it was provided by core Sphinx.

That's also the main objective. That should be the default behavior.

I think we should throw other scenarios over board as long as no one is weighing in on them... or just do a design that can accommodate for them later. So it's all just pure speculation with potential for over-engineering.

@humitos what do you think about using config values instead of the force=True/False kwarg? I like how it pushes the configuration into a global layer, since it makes very little sense that Extension A wants jQuery and Extension B doesn't and the project owner maybe wants to include it from somewhere else or put a debugging version in place or whatevs... at the end of the day, there should be just 1 jQuery version included, hence the global configuration layer seems the right place, even though we may not know how/if it's going to used.

I'd like to change the proposal to use config values and then I think it's starting to look good 👍

[humitos]

@benjaoming

@humitos what do you think about using config values instead of the force=True/False kwarg?

I'm 👍🏼 on that. However, I'd use only one config value. Just jquery_force=True/False (default in `False) instead of two variables as mentioned in a previous comment.

[benjaoming]

I noticed that you already released a 1.0.0, great work. I noticed the Python requirement set to 3.7, and I wanted to add that we aim to have this package perhaps work for 3.6.. I can't tell what is out there building "in the wild", nor how much we are willing to break with an update of sphinx-rtd-theme. At this stage, when there is nothing technically in the way of supporting older Python and Sphinx releases, I think we shouldn't impose minimal version requirements until we know exactly what the aim is.

I don't know exactly what Sphinx and Python versions that we'll remove from sphinx-rtd-theme 2.0. I can imagine that we will remove Python 2.7 and Sphinx<4.

[AA-Turner]

Dear Ben,

Sphinx 6 -- no earlier than December, though probably before Christmas.

Why support Python 2.7? It has been end of life for nearly 3 years (April 2020). Are you proposing to also support Python 3.5 & 3.6 (EOL 2020, or 3.2 onwards (2.7's original EOL date)

Thanks,
Adam

[benjaoming]

Hi AA-Turner

I see our comments have crossed each other. Your question should be answered already in the former comment.

Thanks for clarifying the release!

We need to do this if we want "zero downtime" for users that upgrade to Sphinx 6:

1. sphinxcontrib-jquery is ready and tested with Sphinx 6 beta releases
2. sphinx-rtd-theme 2.0 is ready with Sphinx 6 beta releases and sphinxcontrib-jquery
3. other important extensions with jquery needs are ready with sphinxcontrib-jquery
4. Sphinx 6.0.0 is released.

[benjaoming]

I should add that I think we are in good shape time-wise if Sphinx is coming out in December and sphinxcontrib-jquery is already in the pipeline!

[AA-Turner]

Dear Ben,

It is too painful to test on pre-3.7 (I tried for around 2 hours to get it to work), but I have declared suppport for 2.7 and later in v2.0.0.

Thanks,
Adam

[AA-Turner]

I have invited Ben and Manuel to the sphinxcontrib-jquery repo.

Thanks,
Adam

[benjaoming]

AA-Turner, thanks for adding us.

It is too painful to test on pre-3.7

I think that the pain of the developer is considerable and that we should not spend too much time on old stacks. My suggestion was to get rid of it if it comes for free.. but when it doesn't, then get rid of it 😄 👍

[benjaoming]

Dear AA-Turner,

Since you are already starting to implement and release stuff, do you mind if we do work to have the full proposal implemented and would you release that or grant access for us to release updates?

Thanks,
Ben

[AA-Turner]

Dear Ben,

Sounds good, hopefully I'll have time to review PRs etc.

A

[humitos]

The proposal looks good to me. Considering that the repository and code is already done and the package already released, we need to make some adjustments here in the docs. Also, we need to update the code to reflect this document.

I'll approve these changes and ping @ericholscher and @agjohnson who were the other folks discussing this to do a final review, but I think we are good moving forward here.

[humitos]

This also makes references to 1.x, but it should be updated.

[agjohnson]

👍 This all seems reasonable, it's still pretty close to what we discussed previously.

[benjaoming]

+1 This all seems reasonable, it's still pretty close to what we discussed previously.

@agjohnson yes definitely - the intention was to move an internal effort into a public scope here. Hopefully everything from the previous discussions got reflected and duplicated 😄

[benjaoming]

Thanks for all the comments! I'd suggest to keep the PR open for any further commenting, but that we move forwards with implementing the remaining part of the proposal. I'll open issues in https://github.com/sphinx-contrib/jquery

[benjaoming]

@AA-Turner I'm slightly "under the weather", but I'll try to get to the freshly added issues in the repo and create tests where meaningful.

If anyone wants to jump in, please feel free to self-assign the issue 👍

[humitos]

I think we can just merge this PR as-is since it already worked as a canonical place to discuss the proposal implementation and the work is already happening. If anything, you can just add a small paragraph mentioning the repository where this work is happening and then merge it.

[benjaoming]

Yes, I think we are also out of time for processing more feedback 👍