adding requirejs dependency

[choldgraf]

I was trying to get requirejs to work with Bootstrap, and I found that the only way I could do so was to load requirejs as a part of this theme, and then to disable loading it dynamically by any downstream plugins.

This was partially inspired by this issue: https://stackoverflow.com/questions/15371918/mismatched-anonymous-define-module/23467090#23467090

It seems like things behave unpredictably when requirejs is loaded by extensions etc, so this PR just loads it up-front and before it loads bootstrap, which seemed to be what the SO post recommended. I also added a little plotly example using jupyter-sphinx to test that this actually works (though feel free to recommend a more lightweight example if you can)

closes #30

[choldgraf]

this needs to be set to null so that jupyter-sphinx doesn't load its own version of requirejs

[choldgraf]

actually, now that I think about it I think it'd be better if we added all of the scripts using add_js_file instead of linking directly in the header. That way other projects could check whether the libraries are going to be loaded first (e.g. they could do a check in builder.script_files to see if they need to load requirejs or not)

[choldgraf]

I tried getting this to work with add_js_file entries, but was running into some weird bug where the entries were being added twice which was causing requirejs to fail. That said, I tried out this behavior and it does seem that as long as requirejs is loaded before bootstrap we should be OK. So, as long as a Sphinx extension doesn't load a javascript library that will cause RequireJS to break when it is loaded, then we should be fine.

I am curious if @jorisvandenbossche is OK with bundling RequireJS as a part of this theme by default (about 18kb). Another option would be to make it an option, and to document the fact that if people need to run something that uses RequireJS, then they should enable it at the theme level and disable it at the extension level so we don't trigger the bootstrap bug.

[jorisvandenbossche]

Thanks a lot for looking into this.
Since I basically don't understand anything of the details what is going on with requirejs .. ;), I am happy to defer judgment, and if this approach seems to work, that's good for me. It's in any case a problem that we need to solve some way.

I would at least add the option to turn if off? (similarly as nbsphinx and jupyter-sphinx do)
For the default, I am not sure. You still need to turn it off for nbsphinx or jupyter-sphinx as well to get it working? (which means people will need to find out about it anyhow?)
In any case, a small section to the user guide about how to enable extensions that use requirejs and the need to let them not load requirejs would be useful I think?

[choldgraf]

OK, more experimenting, more weirdness:

• If I put requirejs before bootstrapjs
  • And use async, then it lodas OK
  • And don't use async, then we get the error
• If I put requirejs after bootstrapjs
  • Then it works in both cases

So I concluded that we actually want requirejs after bootstrap loads

So, this PR now adds requirejs after the bootstrapjs. I guess that was the problem - if you add js libraries with add_js_file, it puts them before the extrahead section, so the requirejs from jupyter-sphinx was added before this theme's bootstrap and that was causing errors (I guess it makes sense, since the error was within requirejs rather than bootstrap)

TBH I don't really know why this is a problem because it sounds like a javascript thing (maybe @hoetmaaiers has an idea?)

Either way it seems to work, along with the docs section about using requirejs (also added in this PR)

I also switched the example to use plotly instead of ipywidgets, because I realized that if you don't specify a require.js path then ipywidgets will use a bundle that doesn't need requirejs. Here is the check

[choldgraf]

Is there anything blocking this PR currently?

[hoetmaaiers]

I am not following the need for require.js inside this theme setup. What part of Sphinx requires 'require.js'?

[choldgraf]

That's the tricky thing. This theme doesn't require any requirejs, but many extensions and packages in the pydata community (eg, jupyter-sphinx, plotly, and ipywidgets) do need it, and if they add it in the standard way to their sphinx build, it puts requirejs before bootstrap in the load order, which causes things to break

[choldgraf]

I think that the cleaner solution here would be to get bootstrap loaded with "add_js_file" so we know it loads before require, but when I did that, it caused sphinx to load it twice and I have no idea why

[hoetmaaiers]

I am not familiar with require.js. It also is an older way of importing and bundling javascripts. The only thing add_js_file appears to solve is loading order for the javascripts?

I am also wondering why requirejs loaded before bootstraps breaks things. Is this a name collision (or maybe something global?) Is it possible to provide an example with the requires + bootstrap going wrong?

[jorisvandenbossche]

I am also wondering why requirejs loaded before bootstraps breaks things.

I also goes far beyond my js world knowledge, but from googling, it seems relatively common that people run into problems with boostrap and requirejs ...

So when requirejs is loaded first, you get "Error: Mismatched anonymous define() module:" errors in the browser console (and certain features of bootstrap don't work anymore). According to https://stackoverflow.com/questions/15371918/mismatched-anonymous-define-module/23467090#23467090, the solution is to either load the other js code before loading requirejs, or either to load it using requirejs.

Is it possible to provide an example with the requires + bootstrap going wrong?

Yes, will push a small commit to this branch that will trigger it.

[jorisvandenbossche]

I pushed it to a separate branch to not mess up this PR: #150. If you look at the built demo docs: https://255-130237506-gh.circle-artifacts.com/0/html/index.html, I get the mentioned error in the browser console.
Then the javascript-based features of bootstrap (I assume) don't work anymore. For example, when viewing the site in responsive design mode for a smartphone size, the main menu expansion doesn't work (I think the "hamburger" ?)

[hoetmaaiers]

I pushed it to a separate branch to not mess up this PR

This helps. I might have an idea for combining bootstrap and require.js this way. Maybe making correct use of Webpack and loading Bootstrap that way will help. I try to look into it soon.

[jorisvandenbossche]

@choldgraf I am happy to have this merged in the meantime, even if @hoetmaaiers can find a better solution, if that helps you downstream.

[hoetmaaiers]

Based on @jorisvandenbossche separate branch, I managed to fix the requires error by importing bootstrap dependencies via Webpack and bundle over there.

It is available on https://github.com/hoetmaaiers/pydata-bootstrap-sphinx-theme/tree/pr-150, what is the easiest to test if this works with the example package needing require (was it a plotly graph) , is it included in #150 ?

[choldgraf]

closing this as @hoetmaaiers's fix seems to work quite nicely!!