sphinx icon indicating copy to clipboard operation
sphinx copied to clipboard

Clean way to include JavaScript frameworks in themes or extensions

Open humitos opened this issue 3 years ago • 11 comments
trafficstars

Hi! Due to the latest changes about removing Javascript frameworks (#10070), we are thinking what's the best way to require/include jQuery from downstream in a clean way. This is from a Sphinx extension/theme.

Option 1: statically via html_js_files

I started doing this in the simplest way. In my Sphinx extension, I'm adding jQuery as follows:

def setup_jquery(app, exception):
    """
    Inject jQuery if Sphinx>=6.x

    Staring on Sphinx 6.0, jQuery is not included with it anymore.
    As this extension depends on jQuery, we are including it when Sphinx>=6.x
    """

    if sphinx.version_info >= (6, 0, 0):
        # https://jquery.com/download/#using-jquery-with-a-cdn
        jquery_cdn_url = "https://code.jquery.com/jquery-3.6.0.min.js"
        html_js_files = getattr(app.config, "html_js_files", [])
        html_js_files.append((
            jquery_cdn_url,
            {
                'integrity': 'sha256-/xUj+3OJU5yExlq6GSYGSHk7tPXikynS7ogEvDej/m4=',
                'crossorigin': 'anonymous'
            }
        ))
        app.config.html_js_files = html_js_files

def setup(app):
    app.connect('config-inited', setup_jquery)

However, if each extension that requires jQuery does this, we will end up including jQuery multiple times and, probably, with different versions. We could check if there is already something with "jquery" on html_js_files and not include it if it's present, but it's very error-prone since people could use a different method to include it and we will miss those.

Option 2: dynamically by checking if it's not present already

@stsewd worked on a different approach on https://github.com/readthedocs/readthedocs.org/pull/9359/ that checks dynamically if the jQuery is present and if it's not it creates a new <script> tag to include it:

// Inject jQuery if isn't present already.
if (!window.jQuery) {
    console.log("jQuery not found. Injecting...");
    var script = document.createElement("script");
    script.type = 'text/javascript';
    script.src = "https://ajax.googleapis.com/ajax/libs/jquery/3.6.0/jquery.min.js";
    script.onload = function () { domReady(init); };
    document.head.appendChild(script);
}

I'm not an expert on Javascript, and I'm not sure how is the loading order of scripts, but I think this could bring some problems if the extension does call jQuery directly before this function is executed and the script tag loaded. For example, one of my extensions calls $(document).ready(function() {...} as one of the first lines executed.

Option 3: create a helper function at Sphinx

I thought that it could be useful to expose a helper function at Sphinx level to coordinate this. It could work in a similar way as it does Sphinx.set_html_assets_policy('always') where the extension can communicate the Sphinx application whether or not include jQuery (and maybe other frameworks?);

Sphinx.include_js_framework(
    name='jquery',  # required
    version='3.6.0',  # optional?
    url='https://code.jquery.com/jquery-3.6.0.min.js',  # optional?
)

However, after thinking a little about this proposal I immediately realized that it will behave as a "javascript dependency manager" and everything will get complicated too fast:

  • an extension requires jQuery >3.x
  • another extension <2.x
  • another one >2.5,<3.6,!=3.3

That said, what do you think it's the best way to include jQuery from an extension avoiding including it multiple times if several extensions depending on jQuery include it?

References:

  • https://github.com/sphinx-doc/sphinx/pull/10028
  • https://github.com/sphinx-doc/sphinx/issues/10070
  • https://github.com/readthedocs/sphinx_rtd_theme/pull/1299/
  • https://github.com/readthedocs/readthedocs.org/pull/9359/
  • https://github.com/readthedocs/sphinx-hoverxref/pull/204

humitos avatar Jun 27 '22 10:06 humitos

Maybe it should be a config flag on each extension that allows the option to include jQuery (or said framework) or not, allowing the user to pick the best solution based on their needs?

jdillard avatar Jun 27 '22 18:06 jdillard

I do not want to make it easier to include jQuery, theme authors should rewrite in normal JS. We also have a pretty large API around JS assets, and I'm not sure what this would add that can't be achieved by conditional loading.

Are browsers smart enough to only download an asset once if I include a script tag with the same URL multiple times?

A

AA-Turner avatar Jun 27 '22 18:06 AA-Turner

@AA-Turner

I do not want to make it easier to include jQuery, theme authors should rewrite in normal JS

What are the reasons that you don't want to make it easy to include jQuery (or other Javascript frameworks)?

It won't be possible to rewrite jQuery code in multiple cases, mainly in those that do something more complex than just query the DOM to get some elements and change their classes. For example, in one of the extensions that I maintain, I'm using a jQuery plugin (https://www.heteroclito.fr/modules/tooltipster/) that will be impossible to translate into normal js.

Are browsers smart enough to only download an asset once if I include a script tag with the same URL multiple times?

I'm not sure, but I'd guess they will use the cache the second time. However, even if they are, all the themes/extensions author have to use the exact same URL (and version!) to include jQuery. I don't think that will happen since there are multiple CDNs out there and also it can be bundled into the package itself. As an example, on the same day two people from the same team at Read the Docs arrived at two different URLs for including jQuery (as you can see in the description of this issue)

humitos avatar Jun 28 '22 08:06 humitos

It's a tough question. There are many ways to install a JS library to the Sphinx document.

  • app.add_js_file() API from extensions
  • html_js_files setting in conf.py
  • Load it directly via <script> tag from the theme
  • Override the theme partially via template file

Therefore, it's very difficult to detect the conflict.

It's not difficult to detect the JS library is already installed if every component installs it in the same way (ex. API).

def setup(app):
    if not any('jquery' in filename for (filename, attributes) in app.registry.jsfiles):
        app.add_js_file(JQUERY_URL)

But I don't have a good idea to detect if different install methods are used...

tk0miya avatar Jul 03 '22 10:07 tk0miya

It won't be possible to rewrite jQuery code in multiple cases, mainly in those that do something more complex than just query the DOM to get some elements and change their classes. For example, in one of the extensions that I maintain, I'm using a jQuery plugin (heteroclito.fr/modules/tooltipster) that will be impossible to translate into normal js.

jQuery is built on vanilla JS, so I am skeptical of the proposition.

As @tk0miya says, due to the flexibility of including JS files at present (that's before we consider injecting jQuery via another JS file!) I remain unconvinced of the utility. For themes that want to include JS, they would have to add an extension component to use this 'unified' approach.

The suggested workaround in CHANGES is to vendor jQuery, but it is explictly a temporary thing.

Out of interest, what happens if version A and B of jQ are loaded? Later wins, first wins, brower crash?

A

AA-Turner avatar Jul 03 '22 11:07 AA-Turner

It's not good to discuss jQuery vs vanilla JS. The author of extensions or themes can freely choose the JavaScript framework by their purpose, tactics, or preferences. Sphinx should not restrict their choices.

I believe the most important thing about this issue is not only for jQuery but also for other JavaScript frameworks.

tk0miya avatar Jul 03 '22 13:07 tk0miya

@tk0miya

It's not difficult to detect the JS library is already installed if every component installs it in the same way (ex. API).

This was exactly my goal when I opened this issue. Would it be possible to recommend and document a way to include Javascript frameworks so all, or most, of the extensions follow the same pattern? If Sphinx provides this recommended and documented way, theme/extension authors will follow it, and detecting Javascript framework will be more consistent.

Option 3 from the description helps with this. Sphinx could take no responsibility for managing the versions, avoiding collisions, etc. However, providing this API will let extension authors deal with these problems when facing them because they will have the data in a queryable manner allowing them to make better decisions.

humitos avatar Jul 04 '22 10:07 humitos

Just wanted to follow up here. I agree with @humitos that many theme and extension authors will be hitting this issue with the upcoming Sphinx release, and it would be very helpful for these folks if there was a documented way to handle this in the release notes. Otherwise I do expect a few different issues with themes and extensions including multiple conflicting versions, and other odd issues.

I don't have an opinion on what option should be recommended, but do think that it would be very helpful for the Sphinx ecosystem to have the core team recommend one.

ericholscher avatar Jul 11 '22 18:07 ericholscher

Is there a way to integrate this nicely into the current JS/CSS methods? Perhaps a new ‘framework` argument?

A

AA-Turner avatar Jul 11 '22 22:07 AA-Turner

I don't have a fully baked idea yet and that's why I opened this issue, so we can think all together and decide what's the API we want to build if that's possible.

Following the option 3) from my original post I thought about an API with the following features:

  • Allows multiple methods to inject the script (e.g. CDN, vendor)
  • Warn the user when multiple versions are injected
  • Allows specifying that "conflict notification" as ERROR, WARN or None
  • Only accept a list of known frameworks (pre-defined slugs) to avoid collisions (e.g. jquery, knockout, underscore, etc)
  • Allows users to decide the policy if there are multiple versions of the same framework added (e.g. greatest, lowest, or specific one as 3.1.1)

Usage example

Consider a Sphinx project with the following configuration file:

# docs/conf.py
app.add_javascript_framework({
  'jquery': {
    'method': 'cdn',
    'version': '3.6.0',
    'notification': 'warning',
    'policy': 'greatest`,
  },
})

extensions += ['some_extension_that_also_includes_jquery.extension']

The content of some_extension_that_also_includes_jquery.extension, would be something like:

# some_extension_that_also_includes_jquery.extension.py

...

def setup(app):
  app.add_javascript_framework({
    'jquery': {
       'method': 'vendor',
       'version': '2.0.0',
    },
  })

This would end up with jQuery being included only once in their latest version defined (3.6.0) via CDN. The following HTML would be generated:

<script integrity="sha256-/..." crossorigin="anonymous" src="https://code.jquery.com/jquery-3.6.0.min.js"></script>

The output of sphinx-build would show a warning message like:

WARNING: multiple versions of Javascript frameworks are included. jquery 3.6.0 (from "docs/conf.py") and 2.0.0 (from "some_extension_that_also_includes_jquery.extension). Using 3.6.0 as "greatest" policy takes precedence.

Note that this can be done as an extension that could be recommended as the canonical way to include Javascript frameworks from the official Sphinx docs --in case the core team doesn't want to maintain it-- or it can be built at Sphinx core code if we can make it compatible with it.

What do you think about this proposal? What things would you change? Is it aligned in some way with the path we want to move forward? Does it make sense to be implemented as an extension or it would be better to be under Sphinx core?

humitos avatar Jul 13 '22 11:07 humitos

I feel the option 3 is too much. I'm still not sure developers, theme authors, and users really want to control the version of the JS framework. Is it enough to install the latest version (or major version)? So far, Sphinx has provided the latest version of jQuery. But nobody has complained about it.

If my understanding is correct, it's enough to provide a glue package like sphinxcontrib-jquery. Then it's easy to install and use it.

tk0miya avatar Jul 16 '22 16:07 tk0miya

Suggestion here for an update to Sphinx.add_js_file:

def add_js_file(filename: str, framework: str, override: bool, **kwargs):
    if any(framework == js.framework for js in self.script_files):
        if override:
            # INFO LOG ABOUT OVERRIDE
        else:
            # WARN LOG ABOUT FILE IGNORED AS FRAMEWORK ALREADY PRESENT
            return

    if filename and '://' not in filename:
        filename = posixpath.join('_static', filename)

    self.script_files.append(JavaScript(filename, **kwargs))

This allows marking a file as a "framework", and gives an option to override (multiple overrides would just be last-wins).

A

AA-Turner avatar Oct 05 '22 17:10 AA-Turner

I will release 5.3 without this, save any comments overnight.

A

AA-Turner avatar Oct 15 '22 21:10 AA-Turner

Closed with https://pypi.org/project/sphinxcontrib-jquery/, which I believe solves the original issue. Please ping to reopen if needed.

A

AA-Turner avatar Oct 27 '22 09:10 AA-Turner

@AA-Turner Thanks for your work on this. I'd say the last thing to be done here is to update the release notes to suggest using this extension.

ericholscher avatar Oct 31 '22 19:10 ericholscher

Thanks for all the work on this everyone!

I think the sphinxcontrib-jquery extension solves the immediate issue for jQuery and I'm happy that we figured this out before releasing Sphinx 6.x 🎉 . As Eric mentioned, I think it would be good to suggest its usage in the release notes and maybe in the documentation somewhere.

In case we found we have to deal with other frameworks in the future, we can come back to this issue and implement one of the ideas we discussed here.

humitos avatar Nov 03 '22 10:11 humitos