Drop JavaScript Frameworks #10028

AA-Turner · 2021-12-29T09:43:30Z

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

Refactoring

A

xref https://bugzilla.mozilla.org/show_bug.cgi?id=645075#c49

…rchWords

TimKam · 2022-01-08T19:46:24Z

Note that I added an issue for the final removal of the dependencies, which will require some coordination: #10070

pradyunsg

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.

doc/_themes/sphinx13/layout.html

AA-Turner · 2022-01-21T17:43:58Z

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

pradyunsg

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

AA-Turner · 2022-01-28T21:19:45Z

Fixed merge conflicts, sorry it took a while.

@tk0miya or @TimKam would you be able to advise on next steps please? Pradyun gave the all clear above.

A

TimKam · 2022-01-28T21:53:41Z

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.
@AA-Turner and @pradyunsg could briefly mention to what extent you have manually tested the build result?

AA-Turner · 2022-01-28T21:55:47Z

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

pradyunsg · 2022-01-29T08:05:23Z

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!

pradyunsg · 2022-01-29T08:14:14Z

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

pradyunsg · 2022-01-29T08:24:41Z

CHANGES

@@ -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.


Suggested change

EMCAScript 2018.

EMCAScript 2015 (ES6).

pradyunsg · 2022-01-29T08:26:32Z

could briefly mention to what extent you have manually tested the build result?

Do a search.
- Check that the behaviour is same as regular Sphinx (the period handling, etc). You can slow the network + disable the cache via the browser's development tools to be able to test/check some of this.
Click a search result.
Hide search matches.
Use the console to invoke Documentation.initOnKeyListeners and then try those out.
Open a domain index, and click to expand/collapse stuff.

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.

tk0miya · 2022-01-30T13:39:23Z

+1 for merging.

TimKam · 2022-01-30T18:44:30Z

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: Uncaught TypeError: Search.title.text is not a function. This only happens with RTD, not with 'vanilla' Sphinx. But because RTD hosts a lot of Sphinx docs for many projects and readers, it is important to at least inform them. Presumably, they can replace Search.title.text by Search.title.innerText (here). Still, considering that they need to support all kinds of Sphinx versions, I am wondering if it isn't more pragmatic to stick to Search.title.text on Sphinx's side. @AA-Turner: what do you think?

AA-Turner · 2022-01-30T19:16:33Z

I believe RTD could do $(Search.title).text(...) which would work in any version, as it wraps Search.title in a new jQuery object instead of assuming it is one.

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

pradyunsg · 2022-01-30T19:19:40Z

Let's move that discussion over to readthedocs/readthedocs.org#8862? I'm pretty sure @humitos et al will be looking there! :)

TimKam · 2022-01-30T19:27:05Z

Okay. I'll merge then. After all, the error does not break the search for RTD.

TimKam · 2022-01-30T19:29:50Z

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.

AA-Turner added 30 commits December 29, 2021 09:41

Delete jQuery and underscore.js

eb0d4dd

Move underscores.js setup to searchtools.js

65986dc

Update jQuery.url(en|de)code

d89d8ac

Update jQuery.getQueryParameters

96d1e3e

Firefox bug is no longer present

432fe75

xref https://bugzilla.mozilla.org/show_bug.cgi?id=645075#c49

Update jQuery.fn.highlightText

bea87c0

Use enum instead of magic number

2838d58

Update test descriptions to remove obsolete jQuery reference

f0d5e35

Update Documentation.getCurrentURL

1d80c6c

Revert accidental fix of Documentation.getCurrentURL

d3f6265

Update Documentation.initOnKeyListeners

5598948

Update Documentation.hideSearchWords

6e20b72

Update Documentation.highlightSearchWords

aa27809

Update Documentation.initDomainIndexTable

f4d2f2e

Use arrow functions and const

aca673b

Replace $(document).ready

0064cfe

Strict mode

5bee0d4

Move Documentation.hideSearchWords next to Documentation.highlightSea…

7beb14d

…rchWords

Update translation functions in Documentation

c1ef40d

Replace $(document).ready in searchtools.js

804e295

Update Scorer

8a27fa0

Update Search.hasIndex, Search.deferQuery, Search.stopPulse

bc63172

Prefer window.location

2abdd0d

Update Search.init

e1d23ef

Update Search.loadIndex

60a6a78

Update Search.setIndex

9a2569a

Update Search.startPulse

46ceda7

Add _escapeRegExp

c34700d

Update Search.makeSearchSummary

562b6ed

Update Search.htmlToText

dbc9cc9

pradyunsg reviewed Jan 14, 2022

View reviewed changes

doc/_themes/sphinx13/layout.html Outdated Show resolved Hide resolved

Remove missed jQuery in sphinx13 theme

8a0a3a0

pradyunsg approved these changes Jan 25, 2022

View reviewed changes

Merge branch 'master' into drop-javascript-frameworks

33d84b5

# Conflicts: # CHANGES # package-lock.json # sphinx/themes/basic/static/doctools.js

pradyunsg reviewed Jan 29, 2022

View reviewed changes

TimKam mentioned this pull request Jan 30, 2022

RtD's embed JS calls Sphinx's Search.init unnecessarily readthedocs/readthedocs.org#8862

Closed

TimKam merged commit 3b01fbe into sphinx-doc:master Jan 30, 2022

This was referenced Jan 31, 2022

Search: correctly remove Search.init on Sphinx >= 5.0 readthedocs/readthedocs-sphinx-ext#100

Merged

Incompatibilities with search and Sphinx 5.x readthedocs/readthedocs.org#8871

Closed

Searchtools: don't assume that all themes define some elements #10153

Merged

AA-Turner mentioned this pull request Feb 5, 2022

Drop jQuery and UnderscoreJS #9874

Closed

7 tasks

has2k1 mentioned this pull request Feb 17, 2022

Documentation loads jquery.js twice has2k1/plydata#26

Open

github-actions bot locked as resolved and limited conversation to collaborators Mar 2, 2022

AA-Turner deleted the drop-javascript-frameworks branch May 2, 2022 17:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Drop JavaScript Frameworks #10028

Drop JavaScript Frameworks #10028

AA-Turner commented Dec 29, 2021 •

edited by TimKam

TimKam commented Jan 8, 2022

pradyunsg left a comment

AA-Turner commented Jan 21, 2022

pradyunsg left a comment

AA-Turner commented Jan 28, 2022

TimKam commented Jan 28, 2022

AA-Turner commented Jan 28, 2022

pradyunsg commented Jan 29, 2022 •

edited

pradyunsg commented Jan 29, 2022

pradyunsg Jan 29, 2022 •

edited

pradyunsg commented Jan 29, 2022 •

edited

tk0miya commented Jan 30, 2022

TimKam commented Jan 30, 2022

AA-Turner commented Jan 30, 2022

pradyunsg commented Jan 30, 2022

TimKam commented Jan 30, 2022

TimKam commented Jan 30, 2022

Drop JavaScript Frameworks #10028

Drop JavaScript Frameworks #10028

Conversation

AA-Turner commented Dec 29, 2021 • edited by TimKam

Feature or Bugfix

TimKam commented Jan 8, 2022

pradyunsg left a comment

Choose a reason for hiding this comment

AA-Turner commented Jan 21, 2022

pradyunsg left a comment

Choose a reason for hiding this comment

AA-Turner commented Jan 28, 2022

TimKam commented Jan 28, 2022

AA-Turner commented Jan 28, 2022

pradyunsg commented Jan 29, 2022 • edited

pradyunsg commented Jan 29, 2022

pradyunsg Jan 29, 2022 • edited

Choose a reason for hiding this comment

pradyunsg commented Jan 29, 2022 • edited

tk0miya commented Jan 30, 2022

TimKam commented Jan 30, 2022

AA-Turner commented Jan 30, 2022

pradyunsg commented Jan 30, 2022

TimKam commented Jan 30, 2022

TimKam commented Jan 30, 2022

AA-Turner commented Dec 29, 2021 •

edited by TimKam

pradyunsg commented Jan 29, 2022 •

edited

pradyunsg Jan 29, 2022 •

edited

pradyunsg commented Jan 29, 2022 •

edited