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

Remove jQuery #1253

Open
agjohnson opened this issue Nov 17, 2021 · 9 comments
Open

Remove jQuery #1253

agjohnson opened this issue Nov 17, 2021 · 9 comments
Labels
Accepted Accepted issue on our roadmap Improvement Minor improvement to code

Comments

@agjohnson
Copy link
Collaborator

Sphinx will be moving to remove jQuery as a dependency. We are currently using jQuery in our JS and in the application JS, so we'll want to start thinking about doing this with vanilla JS.

Related: sphinx-doc/sphinx#7405 (comment)

@agjohnson agjohnson added Improvement Minor improvement to code Accepted Accepted issue on our roadmap labels Nov 17, 2021
@davidfischer
Copy link
Contributor

I actually view this as a positive. I do think there's a few things to think about:

  1. Should we make our theme work with old Sphinx versions that include jQuery but our theme only uses vanilla JS?
  2. Should we major rev the theme and require new versions of Sphinx for that version of the theme?

Option 1 might be tricky but I lean in that direction.

@nienn
Copy link
Contributor

nienn commented Nov 18, 2021

Option 1 should be easy enough as using vanilla JS with or without jQuery creates no conflict. It would be even better if we drop support for IE11 at the same time to use ES6 without any combability concerns. It's then just be a matter of including jQuery for old Sphinx versions, but this should not affect us.

@davidfischer
Copy link
Contributor

It's then just be a matter of including jQuery for old Sphinx versions, but this should not affect us.

We won't even need to do this! Sphinx handles this already. If they build with an old Sphinx, it writes jQuery to the output. We just have to make sure our theme works regardless of whether Sphinx added jQuery or not.

@ericholscher
Copy link
Member

Is there anything else we need to do here?

@benjaoming
Copy link
Contributor

Yes, we should add the sphinxcontrib-jquery dependency - we need to firstly add docutils 0.18 support (since it seems to be a requirement of Sphinx 6), on top of that we can add Sphinx 6 support w/ the sphinxcontrib-jquery dependency.

@benjaoming
Copy link
Contributor

I don't think that it's a good idea to remove jQuery, since we probably have projects using sphinx-rtd-theme that will anticipate that it's there. Once gone, they will break silently in bad ways.

We could, though, remove jQuery from the theme itself and replace it with vanilla JS. Any comments on this?

I have added sphinxcontrib-jquery as a dependency here: #1385

@agjohnson
Copy link
Collaborator Author

I don't think that it's a good idea to remove jQuery, since we probably have projects using sphinx-rtd-theme that will anticipate that it's there. Once gone, they will break silently in bad ways.

Yeah, unfortunately I think it's best to add the dependency on sphinxcontrib-jquery either way. We don't have any way of confirming how widely jQuery would be used on derivative themes. This should be a fairly significant deprecation in a fairly long list of deprecations. Moving towards vanilla JS is probably wise just for consistency, but I would probably give jQuery removal a longer grace period.

@benjaoming
Copy link
Contributor

I'd definitely like to remove jQuery and a dependency sphinxcontrib-jquery, maybe we can target the latter for 2.0. So we start by softly removing jQuery, replacing it with vanially JS in 1.2 and then we can announce the deprecation with a bit of buffer until final removal?

@agjohnson
Copy link
Collaborator Author

2.0 is already overloaded, so probably not in that release. I don't think including jQuery is a huge problem, really. It's just debt we want to remove eventually, or is maybe just being Technically Correct.

I say it's fine to continue including the release here via sphinxcontrib-jquery, but we should be moving towards vanilla JS everywhere either way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepted Accepted issue on our roadmap Improvement Minor improvement to code
Projects
None yet
Development

No branches or pull requests

6 participants