- Sponsor
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Drop JavaScript Frameworks #10028
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
Drop JavaScript Frameworks #10028
Conversation
Note that I added an issue for the final removal of the dependencies, which will require some coordination: #10070 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still haven’t found time to checkout this branch and run things, so… here’s another round of me reviewing this PR from a handheld device with a touch screen.
Are there any more blockers on this (beyond review)? There's now a merge conflict but I want to check we've accepted this method of communicating the change etc before putting much more work into this. If you'd prefer I resolve the conflict first though, happy to do so. A |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me overall! If there's something that's slipped through the cracks that both @AA-Turner and I couldn't spot, we can certainly fix that in a follow up!
# Conflicts: # CHANGES # package-lock.json # sphinx/themes/basic/static/doctools.js
Thank you. I would like to click myself through a build in a couple of browsers asap, but I don't see/expect any issues. ECMA 2018 is not (fully) supported by some fringe or very dated browsers. I assume we can live with this, because Sphinx documentation is used primarily by developers or tech-literature folks, so the share of Internet Explorer (and similar) users should be negligible. So if @tk0miya is okay with it, I would most likely merge after I have done some manual testing. |
Sounds good, thanks! Tested on latest Firefox and Chrome, but not much beyond that. Testing in Safari (WebKit) would cover all iOS usages, so I could do that. I assume Google use the same browser engine on mobile and desktop, although not certain from memory. A |
As far as I can tell, there's nothing from ECMAScript 2018 in this PR. There's a decent amount of ECMAScript 2015 (ES6) tho, and that's pretty well supported across the board: https://caniuse.com/es6. See also https://caniuse.com/?compare=ie+11,edge+94,edge+95,edge+96,edge+97,firefox+76,firefox+86,firefox+96,chrome+77,chrome+87,chrome+97,safari+13.1,safari+14.1,safari+15.1,safari+15.2-15.3,ios_saf+15.2-15.3,android+97,and_chr+97,and_ff+95&compareCats=JS I've tried out Chrome and Firefox on MacOS. I considered trying out other browsers, but looking at the patch, nothing stood out as problematic. I realised the ReadTheDocs preview of this PR uses the new JS, so... trying out the basics on other browsers is fairly straightforward: https://sphinx--10028.org.readthedocs.build/en/10028/ -- it won't exercise the translations JS on there tho. I haven't tried, but I won't be surprised if stuff still works on IE 11. I have access to BrowserStack via their OSS plans (and I'm pretty sure Sphinx maintainers can sign up too), so... in theory, we can test whatever browser combinations we want. If there's a specific combination that you'd like to see tested, lemme know! |
Oh, one note about the RtD preview -- they're calling Search.init an extra time in their injected JS, which is why there's a redundant "Searching..." on the search page: readthedocs/readthedocs.org#8862 |
@@ -41,6 +58,9 @@ Features added | |||
* #9075: autodoc: The default value of :confval:`autodoc_typehints_format` is | |||
changed to ``'smart'``. It will suppress the leading module names of | |||
typehints (ex. ``io.StringIO`` -> ``StringIO``). | |||
* #10028: Removed internal usages of JavaScript frameworks (jQuery and | |||
underscore.js) and modernised ``doctools.js`` and ``searchtools.js`` to | |||
EMCAScript 2018. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EMCAScript 2018. | |
EMCAScript 2015 (ES6). |
This tests basically all of doctools, and the main parts of searchtools as far as I can tell. I haven't bothered testing out any of the theme-related changes, especially not the CSS changes to sphinx13/classic. |
+1 for merging. |
There is one -- I assume final -- issue that we may need to address. Open this link: https://sphinx--10028.org.readthedocs.build/en/10028/search.html?q=test and check the console output. We get an error: |
I believe RTD could do We could also add a transition function to enable updating functionally, but this is less elegant. Do you know who the best person would be to ask on the RTD side about this? A |
Let's move that discussion over to readthedocs/readthedocs.org#8862? I'm pretty sure @humitos et al will be looking there! :) |
Okay. I'll merge then. After all, the error does not break the search for RTD. |
Thanks all for the great efforts! |
On sphinx-doc/sphinx#10028 the `Sphinx.init` call was changed to be called with the `_ready` function. We need to update our regex to catch that, otherwise `Sphinx.init` will be called twice. Closes readthedocs/readthedocs.org#8862.
On sphinx-doc/sphinx#10028 the `Sphinx.init` call was changed to be called with the `_ready` function. We need to update our regex to catch that, otherwise `Sphinx.init` will be called twice. Closes readthedocs/readthedocs.org#8862.
xref #7405, #9874 (Closes #7405)
This is another attempt to drop jQuery & underscore.js. Tests pass & everything is converted. I did initially miss @pradyunsg's implementation of this (I started having a go at this back in 2020 and never got around to upstreaming!), so there are differences between the two, Happy to withdraw mine or work on merging so as not to have competing implementations -- whichever is easier for the maintainers (this is why I've opened the PR as draft).
Main differences between mine and his architecturally are I haven't moved to JavaScript classes, although that might be a good idea -- I didn't consider it at all really.
Feature or Bugfix
A