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

API: use <meta> to define supported API version and trigger CustomEvent #64

Merged
merged 19 commits into from
Apr 15, 2024

Conversation

humitos
Copy link
Member

@humitos humitos commented Apr 27, 2023

Let users to define <meta name="readthedocs-api-version" content="1.0"> to tell Read the Docs client what is the API scheme version supported by them.

When our client request the API data, if the api_version returned does not match with the one expected by the user, another request is done to force a particular API scheme version.

Then, we dispatch a readthedocsdataready custom event and expose window.readthedocs, to let our users know this data is ready to be consumed by their own integrations.

Closes #60
Closes #61
Closes #17
Closes readthedocs/readthedocs.org#9957
Closes #250

@humitos humitos requested a review from a team as a code owner April 27, 2023 19:12
@humitos humitos requested a review from agjohnson April 27, 2023 19:12
src/readthedocs-config.js Outdated Show resolved Hide resolved
public/index.html Outdated Show resolved Hide resolved
src/readthedocs-config.js Outdated Show resolved Hide resolved
Let users to define `<meta name="readthedocs-api-version" content="1.0">` to
tell Read the Docs client what is the API scheme version supported by them.

When our client request the API data, if the `api_version` returned does not
match with the one expected by the user, another request is done to force a
particular API scheme version.

Then, we dispatch a `readthedocsdataready` custom event,
to let our users know this data is ready to be consumed by their own
integrations.

Closes #60
Closes #61
@humitos
Copy link
Member Author

humitos commented Apr 27, 2023

@pradyunsg 👋🏼 -- I'm pinging you in this PR because this is the initial implementation for the pattern that we talked at PyCon to keep the integration stable as much as possible. I'd appreciate any feedback you may have here.

Let me know if connecting to the readthedocsdataready event as a theme author and having access to API response as JSON in event.detail would be enough for you.

Feel free to suggest any change you consider, from naming to full re-designing of the pattern 😉

@pradyunsg
Copy link

Haven't reviewed the logic/code but the description sounds great!

humitos added a commit to readthedocs/readthedocs.org that referenced this pull request Apr 28, 2023
This new header will force the API endpoint to return a specific JSON version
schema without taking into consideration the any other header.

The idea behind this is to allow theme authors guarrantees about the JSON scheme
they are expecting; without stopping us to move forward with potential changes
required to the API response when adding new features/addons/etc.

Related readthedocs/addons#64
humitos added a commit to readthedocs/sphinx_rtd_theme that referenced this pull request Sep 13, 2023
Initial experimentation to use the `CustomEvent` triggered by the addons called
`readthedocsdataready` event (from
readthedocs/addons#64) to build the Read the Docs
flyout being integrated into the theme keeping the original look & feel.

* Related: readthedocs/addons#64
* Closes #1523
src/readthedocs-config.js Outdated Show resolved Hide resolved
@humitos
Copy link
Member Author

humitos commented Sep 14, 2023

@pradyunsg I did a POC on the Read the Docs Sphinx theme using this idea of listening to a CustomEvent as a theme author to integrate the Read the Docs flyout addons in better/nicer/more styled way ✨ . See the PR at readthedocs/sphinx_rtd_theme#1526. This is not ready to be merged nor finished, but as I'm exploring with this and shaping out the final structure for this data and more, I'd love to have your feedback here 🙏🏼 . This will help us to know if we are building something that's going to be useful for theme authors and advanced users that want to have better integrations with Read the Docs.

@humitos humitos changed the title API: use <meta> to define supported API version API: use <meta> to define supported API version and trigger CustomEvent Sep 19, 2023
@humitos humitos mentioned this pull request Sep 19, 2023
src/readthedocs-config.js Outdated Show resolved Hide resolved
humitos added a commit to readthedocs/readthedocs.org that referenced this pull request Sep 21, 2023
* API: accept `X-RTD-Hosting-Integrations-API-Version`

This new header will force the API endpoint to return a specific JSON version
schema without taking into consideration the any other header.

The idea behind this is to allow theme authors guarrantees about the JSON scheme
they are expecting; without stopping us to move forward with potential changes
required to the API response when adding new features/addons/etc.

Related readthedocs/addons#64

* Remove old test

* Use text for `api_version`, since it could be something like `0.1.2`
Copy link
Contributor

@agjohnson agjohnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noted how we could improve the promise flow here, the most important point is probably to be ensuring we stop the promise when we hit an error state though.

src/readthedocs-config.js Outdated Show resolved Hide resolved
src/readthedocs-config.js Outdated Show resolved Hide resolved
src/readthedocs-config.js Outdated Show resolved Hide resolved
humitos added a commit to readthedocs/cpython that referenced this pull request Mar 18, 2024
Integrate the new Read the Docs Addons JavaScript into the Python Docs Sphinx
theme to render versions and languages selector nicely.

References:

* Discuss thread: https://discord.com/channels/935215565872693329/1159601953265942589
* Implementation of Addons JavaScript `CustomEvent`: readthedocs/addons#64
humitos added a commit to readthedocs/cpython that referenced this pull request Mar 18, 2024
Integrate the new Read the Docs Addons JavaScript into the Python Docs Sphinx
theme to render versions and languages selector nicely.

References:

* Discord thread: https://discord.com/channels/935215565872693329/1159601953265942589
* Implementation of Addons JavaScript `CustomEvent`: readthedocs/addons#64
* Conversation about using Read the Docs: python/docs-community#5
@humitos humitos requested a review from agjohnson March 19, 2024 10:13
@humitos
Copy link
Member Author

humitos commented Mar 19, 2024

I updated this PR with the latest changes on main and made some minor adjustments. @agjohnson can you please review it?

I opened a POC on the CPython repository that requires the work from this PR to render the language and version selector on their documentation: python/cpython#116966

I've been talking to them via Discord and they are thinking about merging that PR soon to continue with the experimentation about hosting their docs on Read the Docs.

Copy link
Contributor

@agjohnson agjohnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Overall, I would still point you towards using Promise instances more often, as this pattern does help tame promise chaining and nesting rather well.

humitos added a commit that referenced this pull request Apr 9, 2024
Copy link
Contributor

@agjohnson agjohnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The promise logic change looks good 👍

Edit: oookay, I was confused why the logic didn't seem different here. I saw the commit eb550b above and thought it was in this PR 😅

I just realized this from a commit in #286 instead. I think we can merge this and tune the logic afterwards.

humitos added a commit that referenced this pull request Apr 11, 2024
Implements the changes in the API made in
readthedocs/readthedocs.org#11205. We need to
merge that PR and deploy addons together with the changes in this PR.

In this PR there are mainly no changes in the logic. The most important
thing to mention here is that we are standardizing the usage of the API
response and pinning to `v1` now (we were using `v0-alpha`). This means:

- we will be exposing the `v1` to users via the js custom event (see
#64)
- we are using standard/shared APIv3 serializers for resources for this
`/addons/` endpoint
- any breaking change we have to do in the future it will increase the
API response version


I'm not sure there are too much to review here, but I'd appreciate at
least a quick look at these changes. I'm happy to answer some questions
you may have here or on the underlying PR about the API response
changes.


### Related:
* Closes #132 
* Unblocks #265
* Unblocks theme integration (our theme and CPython's one)
* Requires readthedocs/readthedocs.org#11205

---------

Co-authored-by: Anthony <aj@ohess.org>
@humitos
Copy link
Member Author

humitos commented Apr 15, 2024

🎉

@humitos humitos merged commit 0b06297 into main Apr 15, 2024
4 checks passed
@humitos humitos deleted the humitos/custom-event branch April 15, 2024 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants