EPIC: Remove inline JS across all python templates
NOTE: This will enable OpenLibrary's JavaScript to be async and non-render blocking. When transitioning to async JavaScript it's vitally important we review the DOMContentLoaded event inside ol.analytics to make sure it is still firing.
Various templates across openlibrary add inline JavaScript files making it really hard to make changes to HTML or JS that is built in webpack without the potential to break things
The epic can be considered done when there are no matches for the following query:
# using [silver-searcher](https://github.com/ggreer/the_silver_searcher)
ag "(\<script type=\"text\/ja|\<script\>)" --file-search-regex .html openlibrary/(macros|templates) -l --ignore "openlibrary/templates/site/body.html" --ignore openlibrary/templates/site/footer.html
This will allow us to drop the window.q handling code and begin to refactor/rewrite our JavaScript to a more modern framework.
The How
The goal is to move JS out of HTML template files into JS files referenced inside openlibrary/plugins/openlibrary/js/index.js
Typically inline scripts are used when we need to pass data from the server to the client. Historically we've dynamically generated JavaScript
There are several ways to use this
- Use the JS equivalent function
- If a string use a data attribute, and read the value from the DOM in JS.
- If an object use a script tag with type="text/json+graph" following the example in openlibrary/templates/publishers/view.html calling
json_encode
Recommended steps
- Understand how the code works and where the code works before starting. Note some templates may be used on different pages.
- Remove the inline script in the HTML template and move to a file inside openlibrary/plugins/openlibrary/js/
.js - Load
.js inside index.js - Run
make js - Verify the page still works as before without any JS in the error console.
- Make sure the JS is loaded. See note below.
- Replace any references to
window.q.pushwith$(fn) - Document the new code explaining to your best understanding what it does and make sure it passes eslint
- If possible, make a video showing before/after of the code working (something like loom makes it easy)
Note: To avoid large amounts of JS in index.js we will import JS as required based on checks on the HTML. For example
if (document.getElementById('modal-link')) {
import(/* webpackChunkName: "patron_metadata" */ './patron-metadata')
.then((module) => module.initPatronMetadata());
}
Note 2: Some JS is duplicated in other files. If that's the case, the goal should be to simplify the existing code to avoid the duplication.
Acceptance criteria
Pick a file or folder in the list below
- [ ] #9176
- [x] openlibrary/templates/account/loans.html (medium) #8378
- [ ] openlibrary/templates/swagger/swaggerui.html (medium) #8381
- [x] openlibrary/templates/account/ia_thirdparty_logins.html (easy) #8379
- [x] openlibrary/templates/type/i18n/edit.html #8336
- [x] openlibrary/templates/type/i18n/view.html #8336
- [x] openlibrary/templates/lib/edit_head.html (very easy) #8383
- [x] openlibrary/templates/books/breadcrumb_select.html (easy) #8380
- [x] openlibrary/templates/account/readinglog_stats.html (medium) (use data-config on div.readinglog-charts element instead) #8382
- [x] openlibrary/macros/TypeChanger.html #8374
- [x] openlibrary/macros/LoanStatus.html @jimchamp #8275
- [x] openlibrary/templates/lists/lists.html #2468
- [x] openlibrary/templates/lists/widget.html (#6159)
- [x] openlibrary/templates/merge/authors.html
- [x] openlibrary/templates/covers/change.html
- [x] openlibrary/templates/covers/saved.html
- [x] openlibrary/templates/covers/manage.html @drakene
- [x] openlibrary/templates/covers/add.html
- [x] openlibrary/templates/admin/case.html @RayBB #8260
- [x] openlibrary/templates/subjects.html @Yashs911
- [x] openlibrary/templates/admin/people/edits.html @drakene
- [x] openlibrary/templates/admin/people/view.html @drakene
- [x] openlibrary/templates/books/check.html @nileshtrivedi @CliftonMcCallum
- [x] openlibrary/templates/books/author-autocomplete.html @RayBB
- [x] openlibrary/templates/books/edit/addfield.html @RayBB #8261 (previously @cdrini #2700)
- [x] openlibrary/templates/books/edit/edition.html @RayBB
- [x] openlibrary/templates/books/edit.html @lephemere
- [x] openlibrary/templates/books/edit/web.html @lephemere
- [x] openlibrary/templates/books/edit/excerpts.html
- [x] openlibrary/templates/history.html
- [x] openlibrary/templates/type/author/view.html @Yashs911 #5262
- [x] openlibrary/templates/type/user/edit.html @RayBB
- [x] openlibrary/templates/type/user/view.html @RayBB
- [x] openlibrary/templates/type/list/view_body.html #2468
- [x] openlibrary/templates/work_search.html
- [x] openlibrary/templates/lib/history.html
- [x] openlibrary/templates/site/stats.html @Yashs911
- [x] openlibrary/templates/publishers/view.html
- [x] openlibrary/templates/account/borrow.html #5644
- [ ] Write a script to run in CI on PRs to prevent us from adding anymore inline js!
Stakeholders
@jdlrobson
If you want to help please comment issue for the file you'd like to work on.
Hi @jdlrobson , Can I work on books folder?
May I work on /lib? looks fun :D
Hi @jdlrobson , Can I work on books folder?
Perhaps openlibrary/templates/books/edit/ - hopefully that will only equate to the edit interface for books.
May I work on /lib? looks fun :D
lib/analytics runs on every page, so should be relatively straightforward to test so perhaps start there?
@jdlrobson, I would like to give it a shot.
Can I work on openlibrary/templates/subjects.html?
I can give the /covers folder a try if you'd like @jdlrobson. Let me know if there's any particular file I would want to start with!
I think manage would be a good place to start.
@jdlrobson , may I work on this issue? I would like to help clean up the inline JS for openlibrary/templates/books/check.html.
Just Curios about this Issue, Would not it will be messier if all the js file will be put inside one file, Though I want to help in one of the fie as well, can I work on covers folder
The motivation is to get all the javascript in one place to make us less confident making changes there. Having code in one place also allows us to reuse and refactor our code.
In javascript we can import JavaScript so the intention is for large blocks of JavaScript they will live in their own file and be imported asynchronously where needed.
Now that covers is done, I can move to work on the admins folder next @jdlrobson. I don't think I see anyone who has jumped on it yet.
@yashs911 @nynaalekhya how are you getting on with your parts of this task?
@Yashs911 @nynaalekhya how are you getting on with your parts of this task?
@jdlrobson sorry for the late reply. How should I go about removing this:
<script type="text/json+graph" id="graph-json-chartPubHistory">$:json_encode(page.publishing_history)</script>
@Yashs911 @nynaalekhya how are you getting on with your parts of this task?
@jdlrobson sorry for the late reply. How should I go about removing this:
<script type="text/json+graph" id="graph-json-chartPubHistory">$:json_encode(page.publishing_history)</script>
There is no need to remove this. This is not inline Javascript. Inline JSON is fine.
@jdlrobson, I'll have a look at openlibrary/templates/work_search.html.
I'll take openlibrary/templates/type/user/edit.html
@Yashs911 how are you getting on with the subjects page?
Is anyone interested in tackling the other files? We are closing in on our goal of one JavaScript entry point to rule them all :) !
I'll have a look at openlibrary/templates/books/edit/web.html.
I'll continue on the books edit templates: openlibrary/templates/books/edit/addfield.html
I am working on openlibrary/templates/books/edit/edition.html as part of #395 .
The autocomplete stuff is quite a pain so far
I'll also take on openlibrary/templates/books/author-autocomplete.html since it's related to work I did for the edition edit page. But I'll wait until I get some feedback on the above three PRs.
Thanks for pushing on this one!
I started working on openlibrary/templates/books/edit.html.
I am working on openlibrary/templates/site/stats.html
I'll have a look at openlibrary/templates/type/author/view.html.
openlibrary/templates/publishers/view.html can be marked as done via #5188
We are getting very close to our goal! @lephemere is looking at openlibrary/templates/books/edit/addfield.html and openlibrary/templates/type/author/view.html
I'm looking for help with the remaining tasks here.
Beginner/good first tasks:
- openlibrary/macros/LoanStatus.html (move code into index.js verbatim)
- openlibrary/macros/TypeChanger.html (move global into index.js along with the thinginput onchange event registration)
- openlibrary/templates/admin/case.html (possibly dead code that can be deleted after a few discussions in Slack/testing)
- openlibrary/templates/lib/edit_head.html (conditional check for existence of #editHead element, inside index.js)
- openlibrary/templates/account/borrow.html (requires communicating from server to client when 5 loans are present via a data-loan-total attribute. The date enhancement can be moved into index.js)
Intermediate tasks:
- ~~openlibrary/templates/history.html (Will require investigation into what it's doing and a possible no JS equivalent)~~
- i18n: (openlibrary/templates/type/i18n/edit.html, openlibrary/templates/type/i18n/view.html - requires moving some onclick attributes to a conditional javaScript file that progressively enhances the elements using jQuery)
Harder (but rewarding!) tasks for those who want it!:
- #2468 - challenging but should be very rewarding! ( openlibrary/templates/type/list/view.html , openlibrary/templates/lists/widget.html , openlibrary/templates/lists/lists.html)
- openlibrary/templates/account/readinglog_stats.html (use data-config on div.readinglog-charts element instead)
- openlibrary/templates/site/head.html (this must be done last, and will also allow us to add the
asyncattribute to our JavaScript which should result in a huge performance win. Person who works on this task is also responsible for kicking off the Slack celebration thread :)
openlibrary/templates/history.html (Will require investigation into what it's doing and a possible no JS equivalent)
It basically selects two latest revision radio inputs (a10 and b11) and if only one revision is present then it selects (a1, b1)

Does anybody have bandwidth to look at openlibrary/templates/account/readinglog_stats.html or openlibrary/templates/type/i18n/edit.html and openlibrary/templates/type/i18n/view.html ?
@jdlrobson I can do, I'll share the results
@jdlrobson looks like these can be checked off:
- openlibrary/templates/lists/lists.html via #2468
- openlibrary/templates/type/list/view_body.html via #2468
- openlibrary/templates/type/author/view.html via #5262
- openlibrary/templates/account/borrow.html via #5644
And soon:
- openlibrary/templates/admin/case.html via #8260
- openlibrary/templates/books/edit/addfield.html via #8261
Curious if you or anyone has an idea of what this does: https://github.com/internetarchive/openlibrary/blob/f7388d603bb748771529c68c0558b6718211fc3b/openlibrary/templates/lib/edit_head.html#L3-L9
We don't seem to have similar code anywhere else in the codebase. and I'm not sure what would trigger it. My only guess is that it's somehow related to iframes.
Edit: discussion here said maybe it's an old security measure.