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

Only load Jupyter JS dependencies on pages that need it #377

Closed
choldgraf opened this issue Jan 2, 2022 · 5 comments
Closed

Only load Jupyter JS dependencies on pages that need it #377

choldgraf opened this issue Jan 2, 2022 · 5 comments
Labels

Comments

@choldgraf
Copy link
Member

choldgraf commented Jan 2, 2022

Description / Summary

I did a quick lighthouse audit on Jupyter Book, and discovered that a large chunk of our Javascript loading is coming from loading the JS bundles needed for Jupyter widgets (there are some other improvements we can make too, will open other issues for that):

image

This is something we could load in a more restrictive way. Here are a few ideas from anticipated easiest-to-hardest:

Load this JS bundle only if:

  • The page is an ipynb file
  • The page is known to have notebook content (e.g., also covering MyST Markdown Notebooks)
  • The page is known to have widget content on it (maybe as part of the CellOutputBundle parsing logic?)

Value / benefit

This could save a lot of unnecessary JS loading, and speed up page load times.

Implementation details

I think these are the two libraries we could target:

  • <script src="https://cdnjs.cloudflare.com/ajax/libs/require.js/2.3.4/require.min.js"></script>
  • <script src="https://unpkg.com/@jupyter-widgets/html-manager@^0.20.0/dist/embed-amd.js"></script>

somewhere around here:

MyST-NB/myst_nb/__init__.py

Lines 284 to 301 in 2cc1260

require_url_default = (
REQUIRE_URL_DEFAULT
if "jupyter_sphinx_require_url" not in app.config
else app.config.jupyter_sphinx_require_url
)
embed_url_default = (
None
if "jupyter_sphinx_embed_url" not in app.config
else app.config.jupyter_sphinx_embed_url
)
if require_url_default:
builder.add_js_file(require_url_default)
embed_url = embed_url_default or DEFAULT_EMBED_REQUIREJS_URL
else:
embed_url = embed_url_default or DEFAULT_EMBED_SCRIPT_URL
if embed_url:
builder.add_js_file(embed_url)

Tasks to complete

No response

@choldgraf choldgraf added enhancement New feature or request 🏷️ performance labels Jan 2, 2022
@akhmerov
Copy link
Contributor

akhmerov commented Jan 3, 2022

Switching to a single page app would also diminish the impact of the problem.

@chrisjsewell
Copy link
Member

This is available since sphinx 3.5: sphinx-doc/sphinx#8631

@choldgraf
Copy link
Member Author

choldgraf commented Jan 3, 2022

A single page app would be a pretty substantial change. I've never seen a Sphinx site served as a single page app. Though you could probably do it with something like turbolinks. Maybe something to explore after we've picked off the lower hanging fruit.

@chrisjsewell agree that sounds like the right mechanism to use. Id be fine pinning our minimum sphinx 3 version to take advantage is this. You see any problems with that?

@chrisjsewell
Copy link
Member

You can just change the logic based on the sphinx version, ps don't worry about a PR, I'll add it into https://github.com/executablebooks/MyST-NB/tree/refactor-mystnb, which is on its way

@chrisjsewell
Copy link
Member

Now fixed in #380, although feel free to double-check @choldgraf

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants