-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Build: support build time injected logic via generic HTML injection #9063
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
Comments
A few notes regarding this issue from a meeting today:
|
We have already shipped the search parsing for |
The next step here is documenting how to implement these features with |
@ericholscher yes, I'd say that's the first step. We can later improve the integration if that makes sense, but communicating to people how to get the same result they get with the regular builder is a good step that gives users a solution without involving us in writing a lot of that integration yet. I think we have two options here:
|
Note here: we probably should have a design discussion on how to accomplish injection of these pieces. We may be able to do this at proxito, injection at build time is a possibility, and there is also fancier versions of this with web workers, but that is likely too technical of an implementation. |
We also have https://github.com/readthedocs/meta/issues/71 for doing the documentation as a first step. |
@agjohnson @ericholscher the more I think about this, the closer I get to the idea of "creating a Read the Docs javascript library that handles all of them". Similar to what we do we EthicalAds, but for the the warning banners and other hosting integrations. I think that injecting a blob of HTML at build time, include a js/css file as well, and then (maybe) add extra js/css at serving time it's the wrong approach. I'd like to have all of this standardized and it seems that javascript seems to be the right place to do this making it doctool agnostic. What do you think? |
Yeah, I probably lean that way, assuming we have a slightly more established JS library pattern and break out all of this to a standalone library. I still don't know if I want this at build time or injection time though. That could be a good discussion to have. |
Yea, that is already what we do with the I'm definitely on the side of injecting it at serving time, since that gives us a lot more options going forward. It would work similarly to the flyout now with the user being able to tell us where to inject it, or we inject it at the bottom of the |
I'm more torn, because there is a tradeoff to injecting at build time so that the built artifact never changes in the future (we don't have to support old documentation implementations with new client JS releases) and injecting at build time so that our implementation is guaranteed to be the same (but we do have to worry about backwards compatibility). There is a middle ground solution, and it's probably what I'm describing when I say I want the embed client JS to be versioned, and so every build refers to a specific embed client API version or release. We could even backport changes at that point. In this case I lean towards injection at display time. This is a good deal of package maintenance work however. So, this is a conversation we need to discuss a bit more, in my opinion. |
Yea, the current solution we have is injection at build time, but to an unversioned file that changes over time. Versioning that file is definitely an interesting use case, but I do think we need the ability to roll things out across all hosted docs at once in some fashion. |
Aye, that middle ground solution I'm describing would be maintaining multiple versions of the client library package, and also hosting multiple major versions in a shared place. It's a bit more maintenace work, but I doubt we'd have too many cases where we have to backport changes. The multiple hosted versions would mostly be for snapshotting the major versions so we can stop worrying about historical support. That is:
I suppose at that point, build time injection could add these includes in |
If we version the library and we have metadata about the project like That would allow us to inject an old version of our library, for projects building today with Sphinx 2.x for example. Also, in the case we inject these files dynamically at CF, we can make the same decision based on this data. |
Notes from our call on this here: https://docs.google.com/document/d/1Cs5uW7TD5jaUkrFsVMz5YtKF0iAlnpktxxc-FhSHCQc/edit#heading=h.srhzjalnqm6z (RTD Internal only, but I can copy/paste here if folks are curious) |
The work described originally in the description is happening in #10127. Besides, in this issue we start taking about where to inject the JS, which is also happening in #10127. Finally, we detoured to talk about versioning the flyout JavaScript, which that conversation is happening in https://github.com/readthedocs/meta/issues/96 So, I'm closing this issue aiming for simplification and easy to follow conversations/discussions for each topic in just one place while keeping all the issues/PR/discussion linked between each of them in case we want come back to review the context. |
In #8190 we described generalizing our build process with generalized HTML manipulation instead of Sphinx-specific solutions for injection.
I can't summarize here, so might leave this to @humitos to expand more. TBD
readthedocs-sphinx-ext
at this file https://github.com/rtfd/readthedocs-sphinx-ext/blob/7cc1e60f7dcdeb7af35e3479509a621d5bac0976/readthedocs_ext/readthedocs.py)This sounds complex enough that we should have two issues to track probably.
Refs #9062
The text was updated successfully, but these errors were encountered: