Configurably vendor all external JS/CSS

[bollwyvl]

Hi folks! Thanks for the great theme!

I'd like to start using it all over, but #89 is real, for all kinds of reasons, for all kinds of folks.

This is an approach to rooting out the last of the CDN assets.

The big'ns are:

• bootstrap (popper.js is already in the bundle, or we can summon up a smaller one)
  • speaking of popper... is it being used anywhere?
• lato
• open sans
• mathjax
• font-awesome

Here's a little "victory lap" in the browser, with my wifi turned off, and these settings on sphinx<3:

diff --git a/docs/conf.py b/docs/conf.py
index 91a1d08f..e9669d8c 100644
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -82,7 +82,9 @@ html_static_path = ["_static"]
 # -- Auto-convert markdown pages to demo --------------------------------------
 import recommonmark
 from recommonmark.transform import AutoStructify
-
+import pydata_sphinx_theme
 
 def setup(app):
+    app.config.use_public_cdns = False
+    app.config.mathjax_path = pydata_sphinx_theme.get_mathjax_path()

The following caveats:

• i've left use_public_cdns to True by default, to minimize end user changes
  • maybe the vendored assets should be a separate package?
• despite using it for years, the sphinx config stuff makes very little sense to me once more than one thing starts getting involved. no doubt the tack i have taken, and what my warning message suggests to people, is Just Wrong.
• my webpack approach is probably Not Good, but it gets the job done.
  • i'll admit i tried to do it the "right" way, first, burnt a couple hours. But here we are in 2020, and we still can't have nice things involving jquery, bootstrap, popper, fonts, in webpack and frankly, the "script tags on a page" is better for piecewise customizing, anyway.
• speaking of webpack: i don't even know if it works with the dev server, might need another WebpackLoaderPluginFactory for that to work properly. Happy for feedback, or straight up maintainer commits, but i can't get back the person-months i've spent tweaking configs to make that work properly, at the expense of the shipped product/project
• i have no idea if this breaks require.js
• i have added no tests...

Thanks again!

[choldgraf]

I think this all sounds awesome, though I am also not qualified to vet this PR :-)

Maybe @hoetmaaiers has thoughts?

@bollwyvl a bit off-topic, but do you think the changes like the one in this PR could solve jupyter-book/thebe#196 ?

[hoetmaaiers]

First of all, the work of vendoring in more (if not all) dependencies into the project great. I also have the preference of bundling all dependencies into the project and not depend on CDN's for this.
We do should think about fingerprinting every individual output asset for correct browser cache behaviour. Otherwise upgrading e.g. bootstrap could become much harder.

Do we even want the external CDN dependency option to be available? I prefer simplicity and one-way of doing stuff. @choldgraf of @jorisvandenbossche , has this been discussed before?

The webpack approach indeed is somewhat against the core Webpack idea of bundling assets together. Still good work done @bollwyvl. This approach has the advantage of outputting individual javascript & css assets which also keep them visible in the network devtool. The Webpack preffered approach would bundle all vendor assets in a vendor.{js,css}.
Despite this advantage of network tab transparency, I do think bundling into 1 vendor asset has my preference. Fingerprint the file is easy, browsers are able to cache and via source maps it is easy to debug every individual file.

[jorisvandenbossche]

@bollwyvl thanks a lot!

I just had a chat with @hoetmaaiers, and I think there are two main questions:

• Do we keep this configurable (the use_public_cdns option), or do we simply always vendor?

  • I understood that CDNs can be faster, so that might be a reason people prefer to keep this as an option.
  • On the other hand, having both of course adds complexity. Not that much in the layout itself (it's a rather simple if/else), but if having the option, then we should in principle test both with the preview / when making changes locally, etc.

• Bundling the separate external packages separately (as now done in the PR) or combining them all in a single vendor.js/css bundle.

  • Like Chris, "I know nothing" on those aspects, so gladly defer this to @hoetmaaiers and @bollwyvl . My main worry is that I like being able, when inspecting the site locally with Firefox web developer tools, to see where eg certain css is coming from (is it something we defined ourselves, or is it inherited from bootstrap, or from ..). So as long as this is possible, I am fine with any solution.
    @bollwyvl do you have thoughts on this?
    (and @hoetmaaiers can look into updating this for the single bundle, if that's what we prefer)

[jorisvandenbossche]

@bollwyvl can you explain the mathjax situation a bit more?

So sphinx has an option to set the mathjax path (https://www.sphinx-doc.org/en/master/usage/extensions/math.html#confval-mathjax_path). But if we use our vendored assets (which included mathjax), we override what a user might have configured for mathjax?

[bollwyvl]

Again, it's all rather mysterious to me as well!

The math extension adds this configurable and then at env-updated, it adds the mathjax link. I don't know how to intercept it doing that, hence this whole rube goldberg setup.

In sphinx <3 (which i assume needs to be supported) the only way to control the order is the actual extensions order in setup.py, but themes are "special" and don't go there. So it checks whether mathjax_path is set to the default cloudflare value (which is luckily kept around), and will replace only that.

As a user, one can set it to whatever is wanted (e.g. for SVG output) in conf.py and it would still be respected. But vendoring all of MathJax is 75mb, so i just tried to bring in everything that was related to the default output, so if one did want SVG, they'd have to bring all the assets into their _static. This would probably need documentation.

[bollwyvl]

Do we keep this configurable

I would prefer vendor-only, as well, was just trying to keep it backwards compatible, since I don't like surprises in my docs build chain. This also would also remove any version matching issues in the urls vs what's coming from yarn.lock. Some of the complexity will remain (about overloading mathjax_path). I'll work up an alternate PR without the configurability.

combining them all in a single vendor.js/css bundle.

The workarounds I tried for bootstrap/jquery/popper vs webpack are not great, and require lots of shims, etc. I will not be working up an alternate PR with that approach 😬.

MathJax will basically not go in your bundle, I don't really recommend trying. Also, see 75mb.

[bollwyvl]

We do should think about fingerprinting every individual output asset for correct browser cache behaviour.

While a good idea, I am not sure if it is within the scope of this PR, as it seems like something for which a jinja/sphinx solution should already exist, and may require additional configuration.

[bollwyvl]

but do you think the changes like the one in this PR could solve jupyter-book/thebe#196 ?

Unfortunately, I don't really see how this would help there...

[bollwyvl]

This approach has the advantage of outputting individual javascript & css assets

Further, to my understanding, it would be possible for a theme user to overload an individual asset by just putting a same-named one in place. For example, this loads the entire bootstrap.bundle.min.js, but someone especially size-conscious could put a smaller one in without all the extra bells and whistles.

[hoetmaaiers]

This approach has the advantage of outputting individual javascript & css assets

Further, to my understanding, it would be possible for a theme user to overload an individual asset by just putting a same-named one in place. For example, this loads the entire bootstrap.bundle.min.js, but someone especially size-conscious could put a smaller one in without all the extra bells and whistles.

I dare to say we can also be more size-conscious, since we only use a small part of Bootstrap?

[bollwyvl]

I dare to say we can also be more size-conscious, since we only use a small part of Bootstrap?

Sure. By default, using bootstrap.bundle and bootstrap.scss load/parses/watches the whole shooting match of carousels, spinners, modals, etc. on both the js and css side before anything drawing on the page. Trimming it down to just the ones needed, by specifically importing those files, can result in fairly significant savings, and quicker time to first load. However, it may require documenting this for downstream users that may want those extra pieces, ideally with a path to getting them back... with "dumb" imports, this is fairly straightforward, but with webpack, this becomes.... much harder.

Similarly for font-awesome, you can subset just the icons you need... but again, if downstreams expect everything there, there would need to be some form of escape hatch, even if it's to load everything.

[hoetmaaiers]

I agree on you thoughts on including the full bootstrap and fontawesome packages for now.

This PR can be closed in favour of #169 I think?

[bollwyvl]

This has bit-rotted, and I don't intend to fix it, closing!