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: save a "flag" on window object #474

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

humitos
Copy link
Member

@humitos humitos commented Dec 16, 2024

We need to find a way for people subscribing to the Read the Docs data ready event after it was triggered. This is an idea about how to solve this use case.

It adds a "flag" in the window object that users can check before subscribing to that event to make usage of the data if the event was triggered already.

We need to find a way for people subscribing to the Read the Docs data ready
event _after_ it was triggered. This is an idea about how to solve this use
case.

It adds a "flag" in the `window` object that users can check _before_
subscribing to that event to make usage of the data if the event was triggered
already.

* Reference: readthedocs/readthedocs.org#10648 (comment)
* Example: https://github.com/readthedocs/test-builds/blob/full-feature/docs/static/readthedocs.js
@humitos humitos requested a review from a team as a code owner December 16, 2024 11:56
@humitos humitos requested a review from agjohnson December 16, 2024 11:56
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.

Seems good for now, but I think the context API probably will solve this is a cleaner way later.

@@ -114,6 +114,10 @@ export function getReadTheDocsConfig(sendUrlParam) {
// Expose `dataUser` if available or the `data` already requested.
const dataEvent = dataUser !== undefined ? dataUser : data;

// Add the data to the window so scripts loaded after the initial
// event was fired can still get access to the data
window.ReadTheDocsEventData = new ReadTheDocsEventData(dataEvent);
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid using window as this isn't usable in tests/Node. Use globalThis to always reference the correct global.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/globalThis#description

@DimedS
Copy link

DimedS commented Jan 2, 2025

Hi @humitos, @agjohnson,
Happy New Year! 🎉 Do you have any plans to merge that PR? I believe it would resolve the issue mentioned in readthedocs/readthedocs.org#10648.

@humitos
Copy link
Member Author

humitos commented Jan 2, 2025

@DimedS We are still experimenting with a different, and more generic approach. I want to continue testing that, before moving forward here since it may replace the idea from this branch. I will let you know when I have some progress on this.

@DimedS
Copy link

DimedS commented Jan 2, 2025

@DimedS We are still experimenting with a different, and more generic approach. I want to continue testing that, before moving forward here since it may replace the idea from this branch. I will let you know when I have some progress on this.

Thanks, @humitos! Appreciate the update and your efforts!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants